Repository: celestiaorg/celestia-app

Issue:
Set max tx size cap in ProcessProposal
## Description

In v4, we should enforce max tx size cap in ProcessProposal.

## Acceptance Criteria

- [ ] The check is enforced and thoroughly tested in integration tests with expected outputs and logs
- [ ] Reconsider the current max tx size ante handler

Discussion:
@rootulp: > Reconsider the current max tx size ante handler

To expand on this, the `MaxTxSizeDecorator` ante handler doesn't work as intended because it does not receive the full blob tx (including blobs). It only receives the inner sdk tx. Therefore, it can't reject blob txs where the blobs + sdk tx exceed 2 MiB.

We worked around this limitation in https://github.com/celestiaorg/celestia-app/pull/4084 but we should deprecate and remove the `MaxTxSizeDecorator`.

Update: if we keep `MaxTxSizeDecorator` then we should add a unit test for blob txs of various sizes (less than 2 MiB and larger than 2 MiB)

Solution patch:
diff --git a/app/check_tx.go b/app/check_tx.go
index f76b3788..68ff288c 100644
--- a/app/check_tx.go
+++ b/app/check_tx.go
@@ -3,6 +3,9 @@ package app
 import (
 	"fmt"
 
+	"cosmossdk.io/errors"
+
+	apperr "github.com/celestiaorg/celestia-app/v3/app/errors"
 	"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
 	blobtypes "github.com/celestiaorg/celestia-app/v3/x/blob/types"
 	blobtx "github.com/celestiaorg/go-square/v2/tx"
@@ -15,6 +18,14 @@ import (
 // transactions that contain blobs.
 func (app *App) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 	tx := req.Tx
+
+	// all txs must be less than or equal to the max tx size limit
+	maxTxSize := appconsts.MaxTxSize(app.AppVersion())
+	currentTxSize := len(tx)
+	if currentTxSize > maxTxSize {
+		return sdkerrors.ResponseCheckTxWithEvents(errors.Wrapf(apperr.ErrTxExceedsMaxSize, "tx size %d bytes is larger than the application's configured MaxTxSize of %d bytes for version %d", currentTxSize, maxTxSize, app.AppVersion()), 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/errors/errors.go b/app/errors/errors.go
new file mode 100644
index 00000000..9bbff18b
--- /dev/null
+++ b/app/errors/errors.go
@@ -0,0 +1,12 @@
+package errors
+
+import (
+	"cosmossdk.io/errors"
+)
+
+const AppErrorsCodespace = "app"
+
+// general application errors
+var (
+	ErrTxExceedsMaxSize = errors.Register(AppErrorsCodespace, 11142, "exceeds max tx size limit")
+)



Test patch:
diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go
index cede1ae6..bbce4730 100644
--- a/app/test/big_blob_test.go
+++ b/app/test/big_blob_test.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/celestiaorg/celestia-app/v3/app"
 	"github.com/celestiaorg/celestia-app/v3/app/encoding"
+	apperrors "github.com/celestiaorg/celestia-app/v3/app/errors"
 	"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
 	"github.com/celestiaorg/celestia-app/v3/pkg/user"
 	"github.com/celestiaorg/celestia-app/v3/test/util/testfactory"
@@ -55,7 +56,7 @@ func (s *BigBlobSuite) SetupSuite() {
 	require.NoError(t, cctx.WaitForNextBlock())
 }
 
-// TestErrBlobsTooLarge verifies that submitting a 2 MiB blob hits ErrBlobsTooLarge.
+// TestErrBlobsTooLarge verifies that submitting a ~1.9 MiB blob hits ErrBlobsTooLarge.
 func (s *BigBlobSuite) TestErrBlobsTooLarge() {
 	t := s.T()
 
@@ -67,8 +68,8 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() {
 	}
 	testCases := []testCase{
 		{
-			name: "2 mebibyte blob",
-			blob: newBlobWithSize(2 * mebibyte),
+			name: "~ 1.9 MiB blob",
+			blob: newBlobWithSize(2_000_000),
 			want: blobtypes.ErrBlobsTooLarge.ABCICode(),
 		},
 	}
@@ -88,3 +89,38 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() {
 		})
 	}
 }
+
+// TestBlobExceedsMaxTxSize verifies that submitting a 2 MiB blob hits ErrTxExceedsMaxSize.
+func (s *BigBlobSuite) TestBlobExceedsMaxTxSize() {
+	t := s.T()
+
+	type testCase struct {
+		name         string
+		blob         *share.Blob
+		expectedCode uint32
+		expectedErr  string
+	}
+	testCases := []testCase{
+		{
+			name:         "2 MiB blob",
+			blob:         newBlobWithSize(2097152),
+			expectedCode: apperrors.ErrTxExceedsMaxSize.ABCICode(),
+			expectedErr:  apperrors.ErrTxExceedsMaxSize.Error(),
+		},
+	}
+
+	txClient, err := testnode.NewTxClientFromContext(s.cctx)
+	require.NoError(t, err)
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), 30*time.Second)
+			defer cancel()
+			res, err := txClient.SubmitPayForBlob(subCtx, []*share.Blob{tc.blob}, user.SetGasLimitAndGasPrice(1e9, appconsts.DefaultMinGasPrice))
+			require.Error(t, err)
+			require.Nil(t, res)
+			code := err.(*user.BroadcastTxError).Code
+			require.Equal(t, tc.expectedCode, code, err.Error(), tc.expectedErr)
+		})
+	}
+}
diff --git a/app/test/check_tx_test.go b/app/test/check_tx_test.go
index aba77205..87c7cc9e 100644
--- a/app/test/check_tx_test.go
+++ b/app/test/check_tx_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/celestiaorg/celestia-app/v3/app"
 	"github.com/celestiaorg/celestia-app/v3/app/encoding"
+	apperr "github.com/celestiaorg/celestia-app/v3/app/errors"
 	"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
 	"github.com/celestiaorg/celestia-app/v3/pkg/user"
 	testutil "github.com/celestiaorg/celestia-app/v3/test/util"
@@ -32,7 +33,7 @@ func TestCheckTx(t *testing.T) {
 	ns1, err := share.NewV0Namespace(bytes.Repeat([]byte{1}, share.NamespaceVersionZeroIDSize))
 	require.NoError(t, err)
 
-	accs := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"}
+	accs := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m"}
 
 	testApp, kr := testutil.SetupTestAppWithGenesisValSet(app.DefaultConsensusParams(), accs...)
 	testApp.Commit()
@@ -173,11 +174,11 @@ func TestCheckTx(t *testing.T) {
 			expectedABCICode: abci.CodeTypeOK,
 		},
 		{
-			name:      "10,000,000 byte blob",
+			name:      "2,000,000 byte blob",
 			checkType: abci.CheckTxType_New,
 			getTx: func() []byte {
 				signer := createSigner(t, kr, accs[9], encCfg.TxConfig, 10)
-				_, blobs := blobfactory.RandMsgPayForBlobsWithSigner(tmrand.NewRand(), signer.Account(accs[9]).Address().String(), 10_000_000, 1)
+				_, blobs := blobfactory.RandMsgPayForBlobsWithSigner(tmrand.NewRand(), signer.Account(accs[9]).Address().String(), 2_000_000, 1)
 				tx, _, err := signer.CreatePayForBlobs(accs[9], blobs, opts...)
 				require.NoError(t, err)
 				return tx
@@ -217,6 +218,32 @@ 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[11], encCfg.TxConfig, 12)
+				blob, err := share.NewV1Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2097152), signer.Account(accs[11]).Address())
+				require.NoError(t, err)
+				blobTx, _, err := signer.CreatePayForBlobs(accs[11], []*share.Blob{blob}, opts...)
+				require.NoError(t, err)
+				return blobTx
+			},
+			expectedABCICode: apperr.ErrTxExceedsMaxSize.ABCICode(),
+		},
+		{
+			name:      "v0 blob over 2MiB",
+			checkType: abci.CheckTxType_New,
+			getTx: func() []byte {
+				signer := createSigner(t, kr, accs[12], encCfg.TxConfig, 13)
+				blob, err := share.NewV0Blob(share.RandomBlobNamespace(), bytes.Repeat([]byte{1}, 2097152))
+				require.NoError(t, err)
+				blobTx, _, err := signer.CreatePayForBlobs(accs[12], []*share.Blob{blob}, opts...)
+				require.NoError(t, err)
+				return blobTx
+			},
+			expectedABCICode: apperr.ErrTxExceedsMaxSize.ABCICode(),
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/app/test/consistent_apphash_test.go b/app/test/consistent_apphash_test.go
index 9a108e4d..2acc1030 100644
--- a/app/test/consistent_apphash_test.go
+++ b/app/test/consistent_apphash_test.go
@@ -62,6 +62,10 @@ type appHashTest struct {
 	expectedAppHash    []byte
 }
 
+func DefaultTxOpts() []user.TxOption {
+	return blobfactory.FeeTxOpts(10_000_000)
+}
+
 // TestConsistentAppHash executes all state machine messages on all app versions, generates an app hash,
 // and compares it against a previously generated hash from the same set of transactions.
 // App hashes across different commits should be consistent.
@@ -377,7 +381,7 @@ func createEncodedBlobTx(t *testing.T, signer *user.Signer, accountAddresses []s
 	blobTx := blobTx{
 		author:    senderAcc.Name(),
 		blobs:     []*share.Blob{blob},
-		txOptions: blobfactory.DefaultTxOpts(),
+		txOptions: DefaultTxOpts(),
 	}
 	encodedBlobTx, _, err := signer.CreatePayForBlobs(blobTx.author, blobTx.blobs, blobTx.txOptions...)
 	require.NoError(t, err)
@@ -429,7 +433,7 @@ func deterministicKeyRing(cdc codec.Codec) (keyring.Keyring, []types.PubKey) {
 func processSdkMessages(signer *user.Signer, sdkMessages []sdk.Msg) ([][]byte, error) {
 	encodedTxs := make([][]byte, 0, len(sdkMessages))
 	for _, msg := range sdkMessages {
-		encodedTx, err := signer.CreateTx([]sdk.Msg{msg}, blobfactory.DefaultTxOpts()...)
+		encodedTx, err := signer.CreateTx([]sdk.Msg{msg}, DefaultTxOpts()...)
 		if err != nil {
 			return nil, err
 		}
diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go
index f274d9da..b167419a 100644
--- a/app/test/prepare_proposal_test.go
+++ b/app/test/prepare_proposal_test.go
@@ -51,7 +51,6 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) {
 		accnts[:numBlobTxs],
 		infos[:numBlobTxs],
 		testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs),
-		blobfactory.DefaultTxOpts()...,
 	)
 
 	normalTxs := testutil.SendTxsWithAccounts(
@@ -109,7 +108,6 @@ func TestPrepareProposalFiltering(t *testing.T) {
 			testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
 			[][]int{{100}, {1000}, {420}},
 		),
-		blobfactory.DefaultTxOpts()...,
 	)
 
 	// create 3 MsgSend transactions that are signed with valid account numbers
@@ -173,22 +171,6 @@ func TestPrepareProposalFiltering(t *testing.T) {
 	// 3 transactions over MaxTxSize limit
 	largeTxs := coretypes.Txs(testutil.SendTxsWithAccounts(t, testApp, encConf.TxConfig, kr, 1000, accounts[0], accounts[:3], testutil.ChainID, user.SetMemo(largeString))).ToSliceOfBytes()
 
-	// 3 blobTxs over MaxTxSize limit
-	largeBlobTxs := blobfactory.ManyMultiBlobTx(
-		t,
-		encConf.TxConfig,
-		kr,
-		testutil.ChainID,
-		accounts[:3],
-		infos[:3],
-		blobfactory.NestedBlobs(
-			t,
-			testfactory.RandomBlobNamespaces(tmrand.NewRand(), 3),
-			[][]int{{100}, {1000}, {420}},
-		),
-		user.SetMemo(largeString),
-	)
-
 	type test struct {
 		name      string
 		txs       func() [][]byte
@@ -243,9 +225,9 @@ func TestPrepareProposalFiltering(t *testing.T) {
 		{
 			name: "blobTxs and sendTxs that exceed MaxTxSize limit",
 			txs: func() [][]byte {
-				return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxSize limit
+				return largeTxs // All txs are over MaxTxSize limit
 			},
-			prunedTxs: append(largeTxs, largeBlobTxs...),
+			prunedTxs: largeTxs,
 		},
 	}
 
diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go
index 33b1d406..093116a8 100644
--- a/app/test/process_proposal_test.go
+++ b/app/test/process_proposal_test.go
@@ -3,7 +3,6 @@ package app_test
 import (
 	"bytes"
 	"fmt"
-	"strings"
 	"testing"
 	"time"
 
@@ -51,26 +50,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))
-
-	// create 2 single blobTxs that include a large memo making the transaction
-	// larger than the configured max tx size
-	largeBlobTxs := blobfactory.ManyMultiBlobTx(
-		t, enc, kr, testutil.ChainID, accounts[3:], infos[3:],
-		blobfactory.NestedBlobs(
-			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
-	largeSendTx := testutil.SendTxsWithAccounts(
-		t, testApp, enc, kr, 1000, accounts[0], accounts[1:2], testutil.ChainID, user.SetMemo(largeMemo),
 	)
 
 	// create 3 MsgSend transactions that are signed with valid account numbers
@@ -349,26 +328,6 @@ func TestProcessProposal(t *testing.T) {
 			appVersion:     v3.Version,
 			expectedResult: abci.ResponseProcessProposal_REJECT,
 		},
-		{
-			name:  "blob txs larger than configured max tx size",
-			input: validData(),
-			mutator: func(d *tmproto.Data) {
-				d.Txs = append(d.Txs, largeBlobTxs...)
-				d.Hash = calculateNewDataHash(t, d.Txs)
-			},
-			appVersion:     appconsts.LatestVersion,
-			expectedResult: abci.ResponseProcessProposal_REJECT,
-		},
-		{
-			name:  "send tx larger than configured max tx size",
-			input: validData(),
-			mutator: func(d *tmproto.Data) {
-				d.Txs = append(coretypes.Txs(largeSendTx).ToSliceOfBytes(), d.Txs...)
-				d.Hash = calculateNewDataHash(t, d.Txs)
-			},
-			appVersion:     appconsts.LatestVersion,
-			expectedResult: abci.ResponseProcessProposal_REJECT,
-		},
 	}
 
 	for _, tt := range tests {
diff --git a/test/util/blobfactory/payforblob_factory.go b/test/util/blobfactory/payforblob_factory.go
index f9b05359..d15e7ca0 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
diff --git a/test/util/blobfactory/test_util.go b/test/util/blobfactory/test_util.go
index 0496ba17..90a0697d 100644
--- a/test/util/blobfactory/test_util.go
+++ b/test/util/blobfactory/test_util.go
@@ -11,7 +11,7 @@ import (
 )
 
 func DefaultTxOpts() []user.TxOption {
-	return FeeTxOpts(10_000_000)
+	return FeeTxOpts(10_000_000_000)
 }
 
 func FeeTxOpts(gas uint64) []user.TxOption {