From 8ba82c1b872b7f5686d9bb91b93a0442223d7bb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Fri, 11 Oct 2024 14:03:25 -0400 Subject: [PATCH] refactor: cap tx size flups (#3967) ## Overview - `MaxTxBytes` to `MaxTxSize` in order to better distinguish between the mempool and the application param - Removed the logic setting `ctx.WithTxBytes` for the transaction twice in one function due to two prs working on similar changes --- app/ante/max_tx_size.go | 8 ++++---- app/ante/max_tx_size_test.go | 14 +++++++------- app/test/prepare_proposal_test.go | 8 ++++---- app/test/process_proposal_test.go | 10 +++++----- app/validate_txs.go | 2 -- pkg/appconsts/v3/app_consts.go | 2 +- pkg/appconsts/versioned_consts.go | 4 ++-- pkg/appconsts/versioned_consts_test.go | 6 +++--- specs/src/ante_handler_v3.md | 2 +- 9 files changed, 27 insertions(+), 29 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index 2936081aca..a9525777e2 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -24,10 +24,10 @@ func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool } currentTxSize := len(ctx.TxBytes()) - maxTxBytes := appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) - if currentTxSize > maxTxBytes { - bytesOverLimit := currentTxSize - maxTxBytes - return ctx, fmt.Errorf("tx size %d bytes is larger than the application's configured threshold of %d bytes. Please reduce the size by %d bytes", currentTxSize, maxTxBytes, bytesOverLimit) + maxTxSize := appconsts.MaxTxSize(ctx.BlockHeader().Version.App) + if currentTxSize > maxTxSize { + bytesOverLimit := currentTxSize - maxTxSize + return ctx, fmt.Errorf("tx size %d bytes is larger than the application's configured threshold of %d bytes. Please reduce the size by %d bytes", currentTxSize, maxTxSize, bytesOverLimit) } return next(ctx, tx, simulate) } diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go index 3847a0b6a1..b52ceeb084 100644 --- a/app/ante/max_tx_size_test.go +++ b/app/ante/max_tx_size_test.go @@ -24,29 +24,29 @@ func TestMaxTxSizeDecorator(t *testing.T) { isCheckTx []bool }{ { - name: "good tx; under max tx bytes threshold", - txSize: v3.MaxTxBytes - 1, + name: "good tx; under max tx size threshold", + txSize: v3.MaxTxSize - 1, appVersion: v3.Version, expectError: false, isCheckTx: []bool{true, false}, }, { - name: "bad tx; over max tx bytes threshold", - txSize: v3.MaxTxBytes + 1, + name: "bad tx; over max tx size threshold", + txSize: v3.MaxTxSize + 1, appVersion: v3.Version, expectError: true, isCheckTx: []bool{true, false}, }, { - name: "good tx; equal to max tx bytes threshold", - txSize: v3.MaxTxBytes, + name: "good tx; equal to max tx size threshold", + txSize: v3.MaxTxSize, appVersion: v3.Version, expectError: false, isCheckTx: []bool{true, false}, }, { name: "good tx; limit only applies to v3 and above", - txSize: v3.MaxTxBytes + 10, + txSize: v3.MaxTxSize + 10, appVersion: v2.Version, expectError: false, isCheckTx: []bool{true, false}, diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 58ebc0daae..64e105178d 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -160,10 +160,10 @@ func TestPrepareProposalFiltering(t *testing.T) { // memo is 2 MiB resulting in the transaction being over limit largeString := strings.Repeat("a", 2*1024*1024) - // 3 transactions over MaxTxBytes limit + // 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 MaxTxBytes limit + // 3 blobTxs over MaxTxSize limit largeBlobTxs := blobfactory.ManyMultiBlobTx( t, encConf.TxConfig, @@ -231,9 +231,9 @@ func TestPrepareProposalFiltering(t *testing.T) { prunedTxs: [][]byte{tooManyShareBtx}, }, { - name: "blobTxs and sendTxs that exceed MaxTxBytes limit", + name: "blobTxs and sendTxs that exceed MaxTxSize limit", txs: func() [][]byte { - return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxBytes limit + return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxSize limit }, prunedTxs: append(largeTxs, largeBlobTxs...), }, diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index 3ec0756b67..69f18b8bd4 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -54,10 +54,10 @@ func TestProcessProposal(t *testing.T) { blobfactory.DefaultTxOpts()..., ) - largeMemo := strings.Repeat("a", appconsts.MaxTxBytes(appconsts.LatestVersion)) + 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 bytes + // larger than the configured max tx size largeBlobTxs := blobfactory.ManyMultiBlobTx( t, enc, kr, testutil.ChainID, accounts[3:], infos[3:], blobfactory.NestedBlobs( @@ -68,7 +68,7 @@ func TestProcessProposal(t *testing.T) { user.SetMemo(largeMemo)) // create 1 large sendTx that includes a large memo making the - // transaction over the configured max tx bytes limit + // 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), ) @@ -349,7 +349,7 @@ func TestProcessProposal(t *testing.T) { expectedResult: abci.ResponseProcessProposal_REJECT, }, { - name: "blob txs larger than configured max tx bytes", + name: "blob txs larger than configured max tx size", input: validData(), mutator: func(d *tmproto.Data) { d.Txs = append(d.Txs, largeBlobTxs...) @@ -359,7 +359,7 @@ func TestProcessProposal(t *testing.T) { expectedResult: abci.ResponseProcessProposal_REJECT, }, { - name: "send tx larger than configured max tx bytes", + 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...) diff --git a/app/validate_txs.go b/app/validate_txs.go index d8f9c76611..d41cc872b2 100644 --- a/app/validate_txs.go +++ b/app/validate_txs.go @@ -45,7 +45,6 @@ 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) @@ -83,7 +82,6 @@ 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 []*tx.BlobTx) ([]*tx.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) diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go index 2d16d59993..4b65941de1 100644 --- a/pkg/appconsts/v3/app_consts.go +++ b/pkg/appconsts/v3/app_consts.go @@ -6,5 +6,5 @@ const ( SubtreeRootThreshold int = 64 TxSizeCostPerByte uint64 = 10 GasPerBlobByte uint32 = 8 - MaxTxBytes int = 2097152 // 2 MiB in bytes + MaxTxSize int = 2097152 // 2 MiB in bytes ) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 6a186afe71..1eb40deb83 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -42,8 +42,8 @@ func GasPerBlobByte(_ uint64) uint32 { return v3.GasPerBlobByte } -func MaxTxBytes(_ uint64) int { - return v3.MaxTxBytes +func MaxTxSize(_ uint64) int { + return v3.MaxTxSize } var ( diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 90a681b627..f621c0199e 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -67,10 +67,10 @@ func TestVersionedConsts(t *testing.T) { got: appconsts.GasPerBlobByte(v3.Version), }, { - name: "MaxTxBytes v3", + name: "MaxTxSize v3", version: v3.Version, - expectedConstant: v3.MaxTxBytes, - got: appconsts.MaxTxBytes(v3.Version), + expectedConstant: v3.MaxTxSize, + got: appconsts.MaxTxSize(v3.Version), }, } diff --git a/specs/src/ante_handler_v3.md b/specs/src/ante_handler_v3.md index f379ef2d00..f2f6087d71 100644 --- a/specs/src/ante_handler_v3.md +++ b/specs/src/ante_handler_v3.md @@ -3,7 +3,7 @@ The AnteHandler chains together several decorators to ensure the following criteria are met for app version 3: - The tx does not contain any messages that are unsupported by the current app version. See `MsgVersioningGateKeeper`. -- The tx size is not larger than the application's configured versioned constant MaxTxBytes. +- The tx size is not larger than the application's configured versioned constant MaxTxSize. - The tx does not contain any [extension options](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L119-L122). - The tx passes `ValidateBasic()`. - The tx's [timeout_height](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L115-L117) has not been reached if one is specified.