chore: optimize checkTx (backport #3954)

app/process_proposal.go

@@ -61,6 +61,9 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp
  tx = blobTx.Tx
  }
 
+ // todo: uncomment once we're sure this isn't consensus breaking
+ // sdkCtx = sdkCtx.WithTxBytes(tx)
+
  sdkTx, err := app.txConfig.TxDecoder()(tx)
  if err != nil {
  if req.Header.Version.App == v1 {

app/test/prepare_proposal_test.go

@@ -138,6 +138,22 @@ func TestPrepareProposalFiltering(t *testing.T) {
  require.NoError(t, err)
  noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6))
 
+ // create a tx that can't be included in a 64 x 64 when accounting for the
+ // pfb along with the shares
+ tooManyShareBtx := blobfactory.ManyMultiBlobTx(
+ t,
+ encConf.TxConfig,
+ kr,
+ testutil.ChainID,
+ accounts[3:4],
+ infos[3:4],
+ blobfactory.NestedBlobs(
+ t,
+ testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4000),
+ [][]int{repeat(4000, 1)},
+ ),
+ )[0]
+
  type test struct {
  name string
  txs func() [][]byte
@@ -182,6 +198,13 @@ func TestPrepareProposalFiltering(t *testing.T) {
  },
  prunedTxs: [][]byte{noAccountTx},
  },
+ {
+ name: "blob tx with too many shares",
+ txs: func() [][]byte {
+ return [][]byte{tooManyShareBtx}
+ },
+ prunedTxs: [][]byte{tooManyShareBtx},
+ },
  }
 
  for _, tt := range tests {
@@ -217,3 +240,12 @@ func queryAccountInfo(capp *app.App, accs []string, kr keyring.Keyring) []blobfa
  }
  return infos
 }
+
+// repeat returns a slice of length n with each element set to val.
+func repeat[T any](n int, val T) []T {
+ result := make([]T, n)
+ for i := range result {
+ result[i] = val
+ }
+ return result
+}

app/test/process_proposal_test.go

@@ -14,6 +14,7 @@
  "github.com/tendermint/tendermint/proto/tendermint/version"
  coretypes "github.com/tendermint/tendermint/types"
 
+<<<<<<< HEAD
  "github.com/celestiaorg/celestia-app/v2/app"
  "github.com/celestiaorg/celestia-app/v2/app/encoding"
  "github.com/celestiaorg/celestia-app/v2/pkg/appconsts"
@@ -28,6 +29,24 @@
  appns "github.com/celestiaorg/go-square/namespace"
  "github.com/celestiaorg/go-square/shares"
  "github.com/celestiaorg/go-square/square"
+=======
+ "github.com/celestiaorg/celestia-app/v3/app"
+ "github.com/celestiaorg/celestia-app/v3/app/encoding"
+ "github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
+ v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1"
+ v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
+ v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
+ "github.com/celestiaorg/celestia-app/v3/pkg/da"
+ "github.com/celestiaorg/celestia-app/v3/pkg/user"
+ testutil "github.com/celestiaorg/celestia-app/v3/test/util"
+ "github.com/celestiaorg/celestia-app/v3/test/util/blobfactory"
+ "github.com/celestiaorg/celestia-app/v3/test/util/testfactory"
+ "github.com/celestiaorg/celestia-app/v3/test/util/testnode"
+ blobtypes "github.com/celestiaorg/celestia-app/v3/x/blob/types"
+ "github.com/celestiaorg/go-square/v2"
+ "github.com/celestiaorg/go-square/v2/share"
+ "github.com/celestiaorg/go-square/v2/tx"
+>>>>>>> ca222a86 (chore: optimize checkTx (#3954))
 )
 
 func TestProcessProposal(t *testing.T) {
@@ -58,7 +77,7 @@
  // block with all blobs included
  validData := func() *tmproto.Data {
  return &tmproto.Data{
  Txs: blobTxs[:3],
  }
  }
 
@@ -81,11 +100,25 @@
  assert.Error(t, err)
  data := bytes.Repeat([]byte{1}, 13)
 
+ tooManyShareBtx := blobfactory.ManyMultiBlobTx(
+ t,
+ enc,
+ kr,
+ testutil.ChainID,
+ accounts[3:4],
+ infos[3:4],
+ blobfactory.NestedBlobs(
+ t,
+ testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4000),
+ [][]int{repeat(4000, 1)},
+ ),
+ )[0]
+
  type test struct {
  name string
  input *tmproto.Data
  mutator func(*tmproto.Data)
  appVersion uint64
  expectedResult abci.ResponseProcessProposal_Result
  }
 
@@ -330,6 +363,59 @@
  appVersion: appconsts.LatestVersion,
  expectedResult: abci.ResponseProcessProposal_REJECT,
  },
+<<<<<<< HEAD
+=======
+ {
+ name: "valid v1 authored blob",
+ input: validData(),
+ mutator: func(d *tmproto.Data) {
+ addr := signer.Account(accounts[0]).Address()
+ blob, err := share.NewV1Blob(ns1, data, addr)
+ require.NoError(t, err)
+ rawTx, _, err := signer.CreatePayForBlobs(accounts[0], []*share.Blob{blob}, user.SetGasLimit(100000), user.SetFee(100000))
+ require.NoError(t, err)
+ d.Txs[0] = rawTx
+ d.Hash = calculateNewDataHash(t, d.Txs)
+ },
+ appVersion: appconsts.LatestVersion,
+ expectedResult: abci.ResponseProcessProposal_ACCEPT,
+ },
+ {
+ name: "v1 authored blob with invalid signer",
+ input: validData(),
+ mutator: func(d *tmproto.Data) {
+ addr := signer.Account(accounts[0]).Address()
+ falseAddr := testnode.RandomAddress().(sdk.AccAddress)
+ blob, err := share.NewV1Blob(ns1, data, falseAddr)
+ require.NoError(t, err)
+ msg, err := blobtypes.NewMsgPayForBlobs(addr.String(), appconsts.LatestVersion, blob)
+ require.NoError(t, err)
+
+ rawTx, err := signer.CreateTx([]sdk.Msg{msg}, user.SetGasLimit(100000), user.SetFee(100000))
+ require.NoError(t, err)
+
+ blobTxBytes, err := tx.MarshalBlobTx(rawTx, blob)
+ require.NoError(t, err)
+ d.Txs[0] = blobTxBytes
+ d.Hash = calculateNewDataHash(t, d.Txs)
+ },
+ appVersion: appconsts.LatestVersion,
+ expectedResult: abci.ResponseProcessProposal_REJECT,
+ },
+ {
+ name: "blob tx that takes up too many shares",
+ input: &tmproto.Data{
+ Txs: [][]byte{},
+ },
+ mutator: func(d *tmproto.Data) {
+ // this tx will get filtered out by prepare proposal before this
+ // so we add it here
+ d.Txs = append(d.Txs, tooManyShareBtx)
+ },
+ appVersion: v3.Version,
+ expectedResult: abci.ResponseProcessProposal_REJECT,
+ },
+>>>>>>> ca222a86 (chore: optimize checkTx (#3954))
  }
 
  for _, tt := range tests {

app/validate_txs.go

@@ -42,6 +42,7 @@ func FilterTxs(logger log.Logger, ctx sdk.Context, handler sdk.AnteHandler, txCo
 func filterStdTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs [][]byte) ([][]byte, sdk.Context) {
  n := 0
  for _, tx := range txs {
+ ctx = ctx.WithTxBytes(tx)
  sdkTx, err := dec(tx)
  if err != nil {
  logger.Error("decoding already checked transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx).Hash()), "error", err)
@@ -75,6 +76,7 @@ func filterStdTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler
 func filterBlobTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler sdk.AnteHandler, txs []*blob.BlobTx) ([]*blob.BlobTx, sdk.Context) {
  n := 0
  for _, tx := range txs {
+ ctx = ctx.WithTxBytes(tx.Tx)
  sdkTx, err := dec(tx.Tx)
  if err != nil {
  logger.Error("decoding already checked blob transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx.Tx).Hash()), "error", err)

x/blob/ante/blob_share_decorator.go

@@ -36,7 +36,7 @@ func (d BlobShareDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool
  maxBlobShares := d.getMaxBlobShares(ctx)
  for _, m := range tx.GetMsgs() {
  if pfb, ok := m.(*blobtypes.MsgPayForBlobs); ok {
- if sharesNeeded := getSharesNeeded(pfb.BlobSizes); sharesNeeded > maxBlobShares {
+ if sharesNeeded := getSharesNeeded(uint32(len(ctx.TxBytes())), pfb.BlobSizes); sharesNeeded > maxBlobShares {
  return ctx, errors.Wrapf(blobtypes.ErrBlobsTooLarge, "the number of shares occupied by blobs in this MsgPayForBlobs %d exceeds the max number of shares available for blob data %d", sharesNeeded, maxBlobShares)
  }
  }
@@ -49,10 +49,8 @@ func (d BlobShareDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool
 func (d BlobShareDecorator) getMaxBlobShares(ctx sdk.Context) int {
  squareSize := d.getMaxSquareSize(ctx)
  totalShares := squareSize * squareSize
- // The PFB tx share must occupy at least one share so the number of blob shares
- // is at most one less than totalShares.
- blobShares := totalShares - 1
- return blobShares
+ // the shares used up by the tx are calculated in `getSharesNeeded`
+ return totalShares
 }
 
 // getMaxSquareSize returns the maximum square size based on the current values
@@ -74,8 +72,9 @@ func (d BlobShareDecorator) getMaxSquareSize(ctx sdk.Context) int {
 }
 
 // getSharesNeeded returns the total number of shares needed to represent all of
-// the blobs described by blobSizes.
-func getSharesNeeded(blobSizes []uint32) (sum int) {
+// the blobs described by blobSizes along with the shares used by the tx
+func getSharesNeeded(txSize uint32, blobSizes []uint32) (sum int) {
+ sum = share.CompactSharesNeeded(txSize)
  for _, blobSize := range blobSizes {
  sum += shares.SparseSharesNeeded(blobSize)
  }