diff --git a/app/check_tx.go b/app/check_tx.go index f76b37880f..7b610e00c3 100644 --- a/app/check_tx.go +++ b/app/check_tx.go @@ -15,6 +15,15 @@ import ( // transactions that contain blobs. func (app *App) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { tx := req.Tx + + // all txs should be under the max tx size limit + maxTxSize := appconsts.MaxTxSize(app.AppVersion()) + currentTxSize := len(tx) + if currentTxSize > appconsts.MaxTxSize(app.AppVersion()) { + err := fmt.Errorf("tx size %d bytes is larger than the application's configured threshold of %d bytes", currentTxSize, maxTxSize) + return sdkerrors.ResponseCheckTxWithEvents(err, 0, 0, []abci.Event{}, false) + } + // check if the transaction contains blobs btx, isBlob, err := blobtx.UnmarshalBlobTx(tx) if isBlob && err != nil { diff --git a/app/test/check_tx_test.go b/app/test/check_tx_test.go index aba7720547..5918a0ca68 100644 --- a/app/test/check_tx_test.go +++ b/app/test/check_tx_test.go @@ -217,6 +217,31 @@ func TestCheckTx(t *testing.T) { }, expectedABCICode: abci.CodeTypeOK, }, + { + name: "v1 blob over 2MiB", + checkType: abci.CheckTxType_New, + getTx: func() []byte { + signer := createSigner(t, kr, accs[10], encCfg.TxConfig, 11) + blob, err := share.NewV1Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2_000_000), signer.Account(accs[10]).Address()) + require.NoError(t, err) + blobTx, _, err := signer.CreatePayForBlobs(accs[10], []*share.Blob{blob}, opts...) + require.NoError(t, err) + return blobTx + }, + expectedABCICode: blobtypes.ErrBlobsTooLarge.ABCICode(), + }, + { + name: "v0 blob over 2MiB", + checkType: abci.CheckTxType_New, + getTx: func() []byte { + signer := createSigner(t, kr, accs[10], encCfg.TxConfig, 11) + blob, err := share.NewV0Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2_000_000)) + require.NoError(t, err) + blobTx, _, err := signer.CreatePayForBlobs(accs[10], []*share.Blob{blob}, opts...) + require.NoError(t, err) + return blobTx + }, + }, } for _, tt := range tests { diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index f274d9da6a..b7d3ce2513 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -2,6 +2,7 @@ package app_test import ( "crypto/rand" + "fmt" "strings" "testing" "time" @@ -51,7 +52,6 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) { accnts[:numBlobTxs], infos[:numBlobTxs], testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs), - blobfactory.DefaultTxOpts()..., ) normalTxs := testutil.SendTxsWithAccounts( @@ -97,75 +97,74 @@ func TestPrepareProposalFiltering(t *testing.T) { // create 3 single blob blobTxs that are signed with valid account numbers // and sequences - blobTxs := blobfactory.ManyMultiBlobTx( - t, - encConf.TxConfig, - kr, - testutil.ChainID, - accounts[:3], - infos[:3], - blobfactory.NestedBlobs( - t, - testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3), - [][]int{{100}, {1000}, {420}}, - ), - blobfactory.DefaultTxOpts()..., - ) + // blobTxs := blobfactory.ManyMultiBlobTx( + // t, + // encConf.TxConfig, + // kr, + // testutil.ChainID, + // accounts[:3], + // infos[:3], + // blobfactory.NestedBlobs( + // t, + // testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3), + // [][]int{{100}, {1000}, {420}}, + // ), + // ) // create 3 MsgSend transactions that are signed with valid account numbers // and sequences - sendTxs := coretypes.Txs(testutil.SendTxsWithAccounts( - t, - testApp, - encConf.TxConfig, - kr, - 1000, - accounts[0], - accounts[len(accounts)-3:], - testutil.ChainID, - )).ToSliceOfBytes() - - validTxs := func() [][]byte { - txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)) - txs = append(txs, blobTxs...) - txs = append(txs, sendTxs...) - return txs - } + // sendTxs := coretypes.Txs(testutil.SendTxsWithAccounts( + // t, + // testApp, + // encConf.TxConfig, + // kr, + // 1000, + // accounts[0], + // accounts[len(accounts)-3:], + // testutil.ChainID, + // )).ToSliceOfBytes() + + // validTxs := func() [][]byte { + // txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)) + // txs = append(txs, blobTxs...) + // txs = append(txs, sendTxs...) + // return txs + // } // create 3 MsgSend transactions that are using the same sequence as the // first three blob transactions above - duplicateSeqSendTxs := coretypes.Txs(testutil.SendTxsWithAccounts( - t, - testApp, - encConf.TxConfig, - kr, - 1000, - accounts[0], - accounts[:3], - testutil.ChainID, - )).ToSliceOfBytes() + // duplicateSeqSendTxs := coretypes.Txs(testutil.SendTxsWithAccounts( + // t, + // testApp, + // encConf.TxConfig, + // kr, + // 1000, + // accounts[0], + // accounts[:3], + // testutil.ChainID, + // )).ToSliceOfBytes() // create a transaction with an account that doesn't exist. This will cause the increment nonce nilAccount := "carmon san diego" _, _, err := kr.NewMnemonic(nilAccount, keyring.English, "", "", hd.Secp256k1) require.NoError(t, err) - noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6)) + // 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] + // 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] // memo is 2 MiB resulting in the transaction being over limit largeString := strings.Repeat("a", 2*1024*1024) @@ -184,11 +183,25 @@ func TestPrepareProposalFiltering(t *testing.T) { blobfactory.NestedBlobs( t, testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3), - [][]int{{100}, {1000}, {420}}, + [][]int{{699050}, {699050}, {699050}}, // over 2MiB, equal to 2 MiB, and under 2 MiB ), - user.SetMemo(largeString), ) + // 3 blobTxs where one is over MaxTxSize limit + // mixedSizeBlobTxs := blobfactory.ManyMultiBlobTx( + // t, + // encConf.TxConfig, + // kr, + // testutil.ChainID, + // accounts[:2], + // infos[:2], + // blobfactory.NestedBlobs( + // t, + // testfactory.RandomBlobNamespaces(tmrand.NewRand(), 2), + // [][]int{{2}, {2800}, {2097152}}, + // ), + // ) + type test struct { name string txs func() [][]byte @@ -196,50 +209,50 @@ func TestPrepareProposalFiltering(t *testing.T) { } tests := []test{ - { - name: "all valid txs, none are pruned", - txs: func() [][]byte { return validTxs() }, - prunedTxs: [][]byte{}, - }, - { - // even though duplicateSeqSendTxs are getting appended to the end of the - // block, and we do not check the signatures of the standard txs, - // the blob txs still get pruned because we are separating the - // normal and blob txs, and checking/executing the normal txs first. - name: "duplicate sequence appended to the end of the block", - txs: func() [][]byte { - return append(validTxs(), duplicateSeqSendTxs...) - }, - prunedTxs: blobTxs, - }, - { - name: "duplicate sequence txs", - txs: func() [][]byte { - txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)+len(duplicateSeqSendTxs)) - // these should increment the nonce of the accounts that are - // signing the blobtxs, which should make those signatures - // invalid. - txs = append(txs, duplicateSeqSendTxs...) - txs = append(txs, blobTxs...) - txs = append(txs, sendTxs...) - return txs - }, - prunedTxs: blobTxs, - }, - { - name: "nil account panic catch", - txs: func() [][]byte { - return [][]byte{noAccountTx} - }, - prunedTxs: [][]byte{noAccountTx}, - }, - { - name: "blob tx with too many shares", - txs: func() [][]byte { - return [][]byte{tooManyShareBtx} - }, - prunedTxs: [][]byte{tooManyShareBtx}, - }, + // { + // name: "all valid txs, none are pruned", + // txs: func() [][]byte { return validTxs() }, + // prunedTxs: [][]byte{}, + // }, + // { + // // even though duplicateSeqSendTxs are getting appended to the end of the + // // block, and we do not check the signatures of the standard txs, + // // the blob txs still get pruned because we are separating the + // // normal and blob txs, and checking/executing the normal txs first. + // name: "duplicate sequence appended to the end of the block", + // txs: func() [][]byte { + // return append(validTxs(), duplicateSeqSendTxs...) + // }, + // prunedTxs: blobTxs, + // }, + // { + // name: "duplicate sequence txs", + // txs: func() [][]byte { + // txs := make([][]byte, 0, len(sendTxs)+len(blobTxs)+len(duplicateSeqSendTxs)) + // // these should increment the nonce of the accounts that are + // // signing the blobtxs, which should make those signatures + // // invalid. + // txs = append(txs, duplicateSeqSendTxs...) + // txs = append(txs, blobTxs...) + // txs = append(txs, sendTxs...) + // return txs + // }, + // prunedTxs: blobTxs, + // }, + // { + // name: "nil account panic catch", + // txs: func() [][]byte { + // return [][]byte{noAccountTx} + // }, + // prunedTxs: [][]byte{noAccountTx}, + // }, + // { + // name: "blob tx with too many shares", + // txs: func() [][]byte { + // return [][]byte{tooManyShareBtx} + // }, + // prunedTxs: [][]byte{tooManyShareBtx}, + // }, { name: "blobTxs and sendTxs that exceed MaxTxSize limit", txs: func() [][]byte { @@ -260,10 +273,16 @@ func TestPrepareProposalFiltering(t *testing.T) { Height: height, Time: blockTime, }) + fmt.Println( + "EXPECTED TXS", len(tt.txs())-len(tt.prunedTxs), + "RESPONSE TXS", len(resp.BlockData.Txs), + ) // check that we have the expected number of transactions require.Equal(t, len(tt.txs())-len(tt.prunedTxs), len(resp.BlockData.Txs)) // check that the expected txs were removed for _, ptx := range tt.prunedTxs { + fmt.Println("PTX: ", len(ptx)) + fmt.Println(len(tt.prunedTxs), "PRUNED TXS") require.NotContains(t, resp.BlockData.Txs, ptx) } }) diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index 33b1d406c5..34e79c8fe2 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -51,7 +51,6 @@ func TestProcessProposal(t *testing.T) { testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4), [][]int{{100}, {1000}, {420}, {300}}, ), - blobfactory.DefaultTxOpts()..., ) largeMemo := strings.Repeat("a", appconsts.MaxTxSize(appconsts.LatestVersion)) @@ -64,8 +63,7 @@ func TestProcessProposal(t *testing.T) { t, testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4), [][]int{{100}, {1000}, {420}, {300}}, - ), - user.SetMemo(largeMemo)) + )) // create 1 large sendTx that includes a large memo making the // transaction over the configured max tx size limit diff --git a/app/validate_txs.go b/app/validate_txs.go index ee3edfbb6f..90ff84dfbc 100644 --- a/app/validate_txs.go +++ b/app/validate_txs.go @@ -1,6 +1,8 @@ package app import ( + "fmt" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/go-square/v2/tx" "github.com/cosmos/cosmos-sdk/client" @@ -34,7 +36,19 @@ func separateTxs(_ client.TxConfig, rawTxs [][]byte) ([][]byte, []*tx.BlobTx) { // // Side-effect: arranges all normal transactions before all blob transactions. func FilterTxs(logger log.Logger, ctx sdk.Context, handler sdk.AnteHandler, txConfig client.TxConfig, txs [][]byte) [][]byte { - normalTxs, blobTxs := separateTxs(txConfig, txs) + // all transactions should be below the max tx size + maxTxSize := appconsts.MaxTxSize(ctx.BlockHeader().Version.App) + var txsBelowLimit [][]byte + for _, tx := range txs { + if len(tx) > maxTxSize { + err := fmt.Sprintf("tx size %d bytes is larger than the application's configured threshold of %d bytes", len(tx), maxTxSize) + logger.Error(err) + continue + } + txsBelowLimit = append(txsBelowLimit, tx) + } + + normalTxs, blobTxs := separateTxs(txConfig, txsBelowLimit) normalTxs, ctx = filterStdTxs(logger, txConfig.TxDecoder(), ctx, handler, normalTxs) blobTxs, _ = filterBlobTxs(logger, txConfig.TxDecoder(), ctx, handler, blobTxs) return append(normalTxs, encodeBlobTxs(blobTxs)...) diff --git a/test/util/blobfactory/payforblob_factory.go b/test/util/blobfactory/payforblob_factory.go index f9b05359bc..d15e7ca0fe 100644 --- a/test/util/blobfactory/payforblob_factory.go +++ b/test/util/blobfactory/payforblob_factory.go @@ -245,14 +245,13 @@ func ManyMultiBlobTx( accounts []string, accInfos []AccountInfo, blobs [][]*share.Blob, - opts ...user.TxOption, ) [][]byte { t.Helper() txs := make([][]byte, len(accounts)) for i, acc := range accounts { signer, err := user.NewSigner(kr, enc, chainid, appconsts.LatestVersion, user.NewAccount(acc, accInfos[i].AccountNum, accInfos[i].Sequence)) require.NoError(t, err) - txs[i], _, err = signer.CreatePayForBlobs(acc, blobs[i], opts...) + txs[i], _, err = signer.CreatePayForBlobs(acc, blobs[i], DefaultTxOpts()...) require.NoError(t, err) } return txs