From fb9066f91deb5450afaa97ac40a45077b0639011 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 25 Sep 2024 17:11:55 +0200 Subject: [PATCH 01/16] feat: cap tx size --- app/ante/ante.go | 2 + app/ante/max_tx_size.go | 28 +++++++ app/ante/max_tx_size_test.go | 84 +++++++++++++++++++ app/ante/{tx_size.go => tx_size_gas.go} | 0 .../{tx_size_test.go => tx_size_gas_test.go} | 0 pkg/appconsts/v3/app_consts.go | 1 + pkg/appconsts/versioned_consts.go | 4 + pkg/appconsts/versioned_consts_test.go | 6 ++ specs/src/ante_handler_v3.md | 31 +++++++ 9 files changed, 156 insertions(+) create mode 100644 app/ante/max_tx_size.go create mode 100644 app/ante/max_tx_size_test.go rename app/ante/{tx_size.go => tx_size_gas.go} (100%) rename app/ante/{tx_size_test.go => tx_size_gas_test.go} (100%) create mode 100644 specs/src/ante_handler_v3.md diff --git a/app/ante/ante.go b/app/ante/ante.go index 89b6c1d050..c82313b65e 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -32,6 +32,8 @@ func NewAnteHandler( // Set up the context with a gas meter. // Must be called before gas consumption occurs in any other decorator. ante.NewSetUpContextDecorator(), + // Ensure the tx is not larger than the configured threshold. + NewMaxTxSizeDecorator(), // Ensure the tx does not contain any extension options. ante.NewExtensionOptionsDecorator(nil), // Ensure the tx passes ValidateBasic. diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go new file mode 100644 index 0000000000..d7c3436df6 --- /dev/null +++ b/app/ante/max_tx_size.go @@ -0,0 +1,28 @@ +package ante + +import ( + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// MaxTxSizeDecorator ensures that a tx can not be larger than +// application's configured versioned constant. +type MaxTxSizeDecorator struct{} + +func NewMaxTxSizeDecorator() MaxTxSizeDecorator { + return MaxTxSizeDecorator{} +} + +// AnteHandle implements the AnteHandler interface. It ensures that tx size is under application's configured threshold. +func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + // This is a tx validity check, therefore it only applies to CheckTx. + // Tx size rule applies to app versions v3 and onwards. + if ctx.IsCheckTx() && ctx.BlockHeader().Version.App >= v3.Version { + if len(ctx.TxBytes()) >= appconsts.TxMaxBytes(ctx.BlockHeader().Version.App) { + return ctx, sdkerrors.ErrTxTooLarge + } + } + return next(ctx, tx, simulate) +} diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go new file mode 100644 index 0000000000..13407e7e31 --- /dev/null +++ b/app/ante/max_tx_size_test.go @@ -0,0 +1,84 @@ +package ante_test + +import ( + "testing" + + "github.com/celestiaorg/celestia-app/v3/app/ante" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + version "github.com/tendermint/tendermint/proto/tendermint/version" +) + +func TestMaxTxSizeDecorator(t *testing.T) { + decorator := ante.NewMaxTxSizeDecorator() + anteHandler := sdk.ChainAnteDecorators(decorator) + + testCases := []struct { + name string + txSize int + isCheckTx bool + expectError bool + appVersion uint64 + }{ + { + name: "good tx: under max tx bytes threshold", + txSize: v3.MaxTxBytes - 1, + isCheckTx: true, + appVersion: v3.Version, + expectError: false, + }, + { + name: "bad tx; over max tx bytes threshold", + txSize: v3.MaxTxBytes + 1, + isCheckTx: true, + appVersion: v3.Version, + expectError: true, + }, + { + name: "bad tx; equal to max tx bytes threshold", + txSize: v3.MaxTxBytes, + isCheckTx: true, + appVersion: v3.Version, + expectError: true, + }, + { + name: "bad tx; should not error when not CheckTx", + txSize: v3.MaxTxBytes, + isCheckTx: false, + appVersion: v3.Version, + expectError: false, + }, + { + name: "only applies to v3 and above", + txSize: v3.MaxTxBytes, + isCheckTx: true, + appVersion: v2.Version, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := sdk.NewContext(nil, tmproto.Header{ + Version: version.Consensus{ + App: tc.appVersion, + }, + }, tc.isCheckTx, nil) + + txBytes := make([]byte, tc.txSize) + + ctx = ctx.WithTxBytes(txBytes) + _, err := anteHandler(ctx, nil, false) + if tc.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), sdkerrors.ErrTxTooLarge.Error()) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/app/ante/tx_size.go b/app/ante/tx_size_gas.go similarity index 100% rename from app/ante/tx_size.go rename to app/ante/tx_size_gas.go diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_gas_test.go similarity index 100% rename from app/ante/tx_size_test.go rename to app/ante/tx_size_gas_test.go diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go index 3fa92f49a0..ca787102c1 100644 --- a/pkg/appconsts/v3/app_consts.go +++ b/pkg/appconsts/v3/app_consts.go @@ -6,4 +6,5 @@ const ( SubtreeRootThreshold int = 64 TxSizeCostPerByte uint64 = 10 GasPerBlobByte uint32 = 8 + MaxTxBytes int = 2097152 ) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index c5ffb952a6..38af704745 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -42,6 +42,10 @@ func GasPerBlobByte(_ uint64) uint32 { return v3.GasPerBlobByte } +func TxMaxBytes(_ uint64) int { + return v3.MaxTxBytes +} + var ( DefaultSubtreeRootThreshold = SubtreeRootThreshold(LatestVersion) DefaultSquareSizeUpperBound = SquareSizeUpperBound(LatestVersion) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index da29d71241..118d04872a 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -66,6 +66,12 @@ func TestVersionedConsts(t *testing.T) { expectedConstant: v3.GasPerBlobByte, got: appconsts.GasPerBlobByte(v3.Version), }, + { + name: "MaxTxBytes v3", + version: v3.Version, + expectedConstant: v3.MaxTxBytes, + got: appconsts.GasPerBlobByte(v3.Version), + }, } for _, tc := range testCases { diff --git a/specs/src/ante_handler_v3.md b/specs/src/ante_handler_v3.md new file mode 100644 index 0000000000..0c100a8b31 --- /dev/null +++ b/specs/src/ante_handler_v3.md @@ -0,0 +1,31 @@ +# AnteHandler v3 + +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 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. +- The tx's [memo](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L110-L113) is <= the max memo characters where [`MaxMemoCharacters = 256`](). +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the + +gas consumed based on the tx's size where [`TxSizeCostPerByte = 10`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L232). +- The tx's feepayer has enough funds to pay fees for the tx. The tx's feepayer is the feegranter (if specified) or the tx's first signer. Note the [feegrant](https://github.com/cosmos/cosmos-sdk/blob/v0.46.15/x/feegrant/README.md) module is enabled. +- The tx's gas price is >= the network minimum gas price where [`NetworkMinGasPrice = 0.000001` utia](https://github.com/celestiaorg/celestia-app/blob/8caa5807df8d15477554eba953bd056ae72d4503/pkg/appconsts/v2/app_consts.go#L9). +- The tx's count of signatures <= the max number of signatures. The max number of signatures is [`TxSigLimit = 7`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L231). +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's signatures. +- The tx's [signatures](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/types/tx/signing/signature.go#L10-L26) are valid. For each signature, ensure that the signature's sequence number (a.k.a nonce) matches the account sequence number of the signer. +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and + +`gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). +- The tx's total blob share count is <= the max blob share count. The max blob share count is derived from the maximum valid square size. The max valid square size is the minimum of: `GovMaxSquareSize` and `SquareSizeUpperBound`. +- The tx does not contain a message of type [MsgSubmitProposal](https://github.com/cosmos/cosmos-sdk/blob/d6d929843bbd331b885467475bcb3050788e30ca/proto/cosmos/gov/v1/tx.proto#L33-L43) with zero proposal messages. +- The tx is not an IBC packet or update message that has already been processed. + +In addition to the above criteria, the AnteHandler also has a number of side-effects: + +- Tx fees are deducted from the tx's feepayer and added to the fee collector module account. +- Tx priority is calculated based on the smallest denomination of gas price in the tx and set in context. +- The nonce of all tx signers is incremented by 1. From dacf9e8ad82e1d926a74d4d363aca2157c24ba99 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 25 Sep 2024 21:22:43 +0200 Subject: [PATCH 02/16] style: nits --- pkg/appconsts/v3/app_consts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go index ca787102c1..95c2139471 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 + MaxTxBytes int = 2097152 // 2MG (2 * 1024 * 1024) ) From 334e7896b0b395691e05f08909f143874933e8e4 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 25 Sep 2024 21:34:32 +0200 Subject: [PATCH 03/16] test: fix wrong test --- pkg/appconsts/versioned_consts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 118d04872a..160fd869e0 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -70,7 +70,7 @@ func TestVersionedConsts(t *testing.T) { name: "MaxTxBytes v3", version: v3.Version, expectedConstant: v3.MaxTxBytes, - got: appconsts.GasPerBlobByte(v3.Version), + got: appconsts.TxMaxBytes(v3.Version), }, } From f7b7d5963c1b325f3c7ac52a82d7efaf1c977366 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 4 Oct 2024 08:58:09 +0200 Subject: [PATCH 04/16] refactor: make it consensus critical --- app/ante/max_tx_size.go | 3 +-- app/ante/max_tx_size_test.go | 16 ++-------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index d7c3436df6..f4e560a2cb 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -17,9 +17,8 @@ func NewMaxTxSizeDecorator() MaxTxSizeDecorator { // AnteHandle implements the AnteHandler interface. It ensures that tx size is under application's configured threshold. func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - // This is a tx validity check, therefore it only applies to CheckTx. // Tx size rule applies to app versions v3 and onwards. - if ctx.IsCheckTx() && ctx.BlockHeader().Version.App >= v3.Version { + if ctx.BlockHeader().Version.App >= v3.Version { if len(ctx.TxBytes()) >= appconsts.TxMaxBytes(ctx.BlockHeader().Version.App) { return ctx, sdkerrors.ErrTxTooLarge } diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go index 13407e7e31..521d37109a 100644 --- a/app/ante/max_tx_size_test.go +++ b/app/ante/max_tx_size_test.go @@ -20,42 +20,30 @@ func TestMaxTxSizeDecorator(t *testing.T) { testCases := []struct { name string txSize int - isCheckTx bool expectError bool appVersion uint64 }{ { - name: "good tx: under max tx bytes threshold", + name: "good tx; under max tx bytes threshold", txSize: v3.MaxTxBytes - 1, - isCheckTx: true, appVersion: v3.Version, expectError: false, }, { name: "bad tx; over max tx bytes threshold", txSize: v3.MaxTxBytes + 1, - isCheckTx: true, appVersion: v3.Version, expectError: true, }, { name: "bad tx; equal to max tx bytes threshold", txSize: v3.MaxTxBytes, - isCheckTx: true, appVersion: v3.Version, expectError: true, }, - { - name: "bad tx; should not error when not CheckTx", - txSize: v3.MaxTxBytes, - isCheckTx: false, - appVersion: v3.Version, - expectError: false, - }, { name: "only applies to v3 and above", txSize: v3.MaxTxBytes, - isCheckTx: true, appVersion: v2.Version, expectError: false, }, @@ -67,7 +55,7 @@ func TestMaxTxSizeDecorator(t *testing.T) { Version: version.Consensus{ App: tc.appVersion, }, - }, tc.isCheckTx, nil) + }, false, nil) txBytes := make([]byte, tc.txSize) From a31c0b8fef40305ff4b4be22fc6b76ee625d0f5b Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 4 Oct 2024 09:11:43 +0200 Subject: [PATCH 05/16] docs: add another ante rule --- specs/src/ante_handler_v3.md | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/specs/src/ante_handler_v3.md b/specs/src/ante_handler_v3.md index 50379b12f9..f2f6087d71 100644 --- a/specs/src/ante_handler_v3.md +++ b/specs/src/ante_handler_v3.md @@ -8,17 +8,6 @@ The AnteHandler chains together several decorators to ensure the following crite - 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. - The tx's [memo](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L110-L113) is <= the max memo characters where [`MaxMemoCharacters = 256`](). -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the - -gas consumed based on the tx's size where [`TxSizeCostPerByte = 10`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L232). -- The tx's feepayer has enough funds to pay fees for the tx. The tx's feepayer is the feegranter (if specified) or the tx's first signer. Note the [feegrant](https://github.com/cosmos/cosmos-sdk/blob/v0.46.15/x/feegrant/README.md) module is enabled. -- The tx's gas price is >= the network minimum gas price where [`NetworkMinGasPrice = 0.000001` utia](https://github.com/celestiaorg/celestia-app/blob/8caa5807df8d15477554eba953bd056ae72d4503/pkg/appconsts/v2/app_consts.go#L9). -- The tx's count of signatures <= the max number of signatures. The max number of signatures is [`TxSigLimit = 7`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L231). -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's signatures. -- The tx's [signatures](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/types/tx/signing/signature.go#L10-L26) are valid. For each signature, ensure that the signature's sequence number (a.k.a nonce) matches the account sequence number of the signer. -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and - -`gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). - The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's size where [`TxSizeCostPerByte = 10`](https://github.com/celestiaorg/celestia-app/blob/32fc6903478ea08eba728ac9cd4ffedf9ef72d98/pkg/appconsts/v3/app_consts.go#L8). - The tx's feepayer has enough funds to pay fees for the tx. The tx's feepayer is the feegranter (if specified) or the tx's first signer. Note the [feegrant](https://github.com/cosmos/cosmos-sdk/blob/v0.46.15/x/feegrant/README.md) module is enabled. - The tx's gas price is >= the network minimum gas price where [`NetworkMinGasPrice = 0.000001` utia](https://github.com/celestiaorg/celestia-app/blob/32fc6903478ea08eba728ac9cd4ffedf9ef72d98/pkg/appconsts/initial_consts.go#L33). From d90623998daa36403c392b334e9126df77926866 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 4 Oct 2024 09:18:47 +0200 Subject: [PATCH 06/16] refactor: naming and docs --- app/ante/max_tx_size.go | 2 +- pkg/appconsts/v3/app_consts.go | 2 +- pkg/appconsts/versioned_consts.go | 2 +- specs/src/ante_handler_v3.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index f4e560a2cb..dd1da920f6 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -19,7 +19,7 @@ func NewMaxTxSizeDecorator() MaxTxSizeDecorator { func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { // Tx size rule applies to app versions v3 and onwards. if ctx.BlockHeader().Version.App >= v3.Version { - if len(ctx.TxBytes()) >= appconsts.TxMaxBytes(ctx.BlockHeader().Version.App) { + if len(ctx.TxBytes()) >= appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) { return ctx, sdkerrors.ErrTxTooLarge } } diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go index 95c2139471..2d16d59993 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 // 2MG (2 * 1024 * 1024) + MaxTxBytes int = 2097152 // 2 MiB in bytes ) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 38af704745..6a186afe71 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -42,7 +42,7 @@ func GasPerBlobByte(_ uint64) uint32 { return v3.GasPerBlobByte } -func TxMaxBytes(_ uint64) int { +func MaxTxBytes(_ uint64) int { return v3.MaxTxBytes } diff --git a/specs/src/ante_handler_v3.md b/specs/src/ante_handler_v3.md index f2f6087d71..f379ef2d00 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 MaxTxSize. +- The tx size is not larger than the application's configured versioned constant MaxTxBytes. - 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. From 941f55b4164ae2a558351aa804f8f0b36bc24631 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 4 Oct 2024 09:55:12 +0200 Subject: [PATCH 07/16] fix: missed rename --- pkg/appconsts/versioned_consts_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 160fd869e0..90a681b627 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -70,7 +70,7 @@ func TestVersionedConsts(t *testing.T) { name: "MaxTxBytes v3", version: v3.Version, expectedConstant: v3.MaxTxBytes, - got: appconsts.TxMaxBytes(v3.Version), + got: appconsts.MaxTxBytes(v3.Version), }, } From 567d8322dde719d95fb11e8abaa99bfb2a130e0a Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 8 Oct 2024 10:10:12 -0400 Subject: [PATCH 08/16] refactor: syntactic changes --- app/ante/max_tx_size.go | 10 ++++++---- app/ante/max_tx_size_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index dd1da920f6..7a1f99daf6 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -18,10 +18,12 @@ func NewMaxTxSizeDecorator() MaxTxSizeDecorator { // AnteHandle implements the AnteHandler interface. It ensures that tx size is under application's configured threshold. func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { // Tx size rule applies to app versions v3 and onwards. - if ctx.BlockHeader().Version.App >= v3.Version { - if len(ctx.TxBytes()) >= appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) { - return ctx, sdkerrors.ErrTxTooLarge - } + if ctx.BlockHeader().Version.App < v3.Version { + return next(ctx, tx, simulate) + } + + if len(ctx.TxBytes()) > appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) { + return ctx, sdkerrors.ErrTxTooLarge } return next(ctx, tx, simulate) } diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go index 521d37109a..4b5187c3ff 100644 --- a/app/ante/max_tx_size_test.go +++ b/app/ante/max_tx_size_test.go @@ -36,14 +36,14 @@ func TestMaxTxSizeDecorator(t *testing.T) { expectError: true, }, { - name: "bad tx; equal to max tx bytes threshold", + name: "good tx; equal to max tx bytes threshold", txSize: v3.MaxTxBytes, appVersion: v3.Version, - expectError: true, + expectError: false, }, { - name: "only applies to v3 and above", - txSize: v3.MaxTxBytes, + name: "good tx; limit only applies to v3 and above", + txSize: v3.MaxTxBytes + 10, appVersion: v2.Version, expectError: false, }, From 172e29b73d1eed062bc5b7751682d8f67bfd3231 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 12:26:23 -0400 Subject: [PATCH 09/16] feat: set ctx in prepare and process plus test it --- app/ante/max_tx_size.go | 7 ++-- app/process_proposal.go | 6 +++ app/test/prepare_proposal_test.go | 36 +++++++++++++++++- app/test/process_proposal_test.go | 41 +++++++++++++++++++++ app/validate_txs.go | 8 ++++ test/util/blobfactory/payforblob_factory.go | 2 +- 6 files changed, 94 insertions(+), 6 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index 7a1f99daf6..99989b72ba 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -1,10 +1,10 @@ package ante import ( + "fmt" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) // MaxTxSizeDecorator ensures that a tx can not be larger than @@ -22,8 +22,9 @@ func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool return next(ctx, tx, simulate) } - if len(ctx.TxBytes()) > appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) { - return ctx, sdkerrors.ErrTxTooLarge + maxTxBytes := appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) + if len(ctx.TxBytes()) > maxTxBytes { + return ctx, fmt.Errorf("tx size is larger than the application's configured threshold: %d bytes", maxTxBytes) } return next(ctx, tx, simulate) } diff --git a/app/process_proposal.go b/app/process_proposal.go index fc10bb88e2..34ba8645c9 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -90,6 +90,9 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp return reject() } + // Set the tx size on the context before calling the AnteHandler + sdkCtx = sdkCtx.WithTxBytes(tx) + // we need to increment the sequence for every transaction so that // the signature check below is accurate. this error only gets hit // if the account in question doesn't exist. @@ -115,6 +118,9 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp return reject() } + // Set the tx size on the context before calling the AnteHandler + sdkCtx = sdkCtx.WithTxBytes(tx) + // validated the PFB signature sdkCtx, err = handler(sdkCtx, sdkTx, false) if err != nil { diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 8bfcc281f8..5f9b8fd1be 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -1,21 +1,22 @@ package app_test import ( + "strings" "testing" "time" - tmrand "github.com/tendermint/tendermint/libs/rand" - "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" + tmrand "github.com/tendermint/tendermint/libs/rand" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" "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" "github.com/celestiaorg/celestia-app/v3/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v3/test/util/testfactory" @@ -39,6 +40,7 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) { accnts[:numBlobTxs], infos[:numBlobTxs], testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs), + blobfactory.DefaultTxOpts()..., ) normalTxs := testutil.SendTxsWithAccounts( @@ -96,6 +98,7 @@ 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 @@ -137,6 +140,28 @@ func TestPrepareProposalFiltering(t *testing.T) { require.NoError(t, err) noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6)) + // memo is 2MG resulting in the transaction being over limit + largeString := strings.Repeat("a", 2*1024*1024) + + // 3 transactions over MaxTxBytes 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 + 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 @@ -181,6 +206,13 @@ func TestPrepareProposalFiltering(t *testing.T) { }, prunedTxs: [][]byte{noAccountTx}, }, + { + name: "blobTxs and sendTxs that exceed MaxTxBytes limit", + txs: func() [][]byte { + return append(largeTxs, largeBlobTxs...) // All txs are over MaxTxBytes limit + }, + prunedTxs: append(largeTxs, largeBlobTxs...), + }, } for _, tt := range tests { diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index df390dcdfa..40495df97a 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -3,6 +3,7 @@ package app_test import ( "bytes" "fmt" + "strings" "testing" "time" @@ -49,6 +50,26 @@ func TestProcessProposal(t *testing.T) { testfactory.RandomBlobNamespaces(tmrand.NewRand(), 4), [][]int{{100}, {1000}, {420}, {300}}, ), + blobfactory.DefaultTxOpts()..., + ) + + largeMemo := strings.Repeat("a", appconsts.MaxTxBytes(appconsts.LatestVersion)) + + // create 2 single blobTxs that include a large memo making the transaction + // larger than the configured max tx bytes + 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 bytes 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 @@ -299,6 +320,26 @@ func TestProcessProposal(t *testing.T) { appVersion: appconsts.LatestVersion, expectedResult: abci.ResponseProcessProposal_REJECT, }, + { + name: "blob txs larger than configured max tx bytes", + 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 bytes", + 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/app/validate_txs.go b/app/validate_txs.go index 3538e221a0..d41cc872b2 100644 --- a/app/validate_txs.go +++ b/app/validate_txs.go @@ -50,6 +50,10 @@ func filterStdTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handler logger.Error("decoding already checked transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx).Hash()), "error", err) continue } + + // Set the tx size on the context before calling the AnteHandler + ctx = ctx.WithTxBytes(tx) + ctx, err = handler(ctx, sdkTx, false) // either the transaction is invalid (ie incorrect nonce) and we // simply want to remove this tx, or we're catching a panic from one @@ -83,6 +87,10 @@ func filterBlobTxs(logger log.Logger, dec sdk.TxDecoder, ctx sdk.Context, handle logger.Error("decoding already checked blob transaction", "tx", tmbytes.HexBytes(coretypes.Tx(tx.Tx).Hash()), "error", err) continue } + + // Set the tx size on the context before calling the AnteHandler + ctx = ctx.WithTxBytes(tx.Tx) + ctx, err = handler(ctx, sdkTx, false) // either the transaction is invalid (ie incorrect nonce) and we // simply want to remove this tx, or we're catching a panic from one diff --git a/test/util/blobfactory/payforblob_factory.go b/test/util/blobfactory/payforblob_factory.go index 1a4aa6d10c..f9b05359bc 100644 --- a/test/util/blobfactory/payforblob_factory.go +++ b/test/util/blobfactory/payforblob_factory.go @@ -245,10 +245,10 @@ func ManyMultiBlobTx( accounts []string, accInfos []AccountInfo, blobs [][]*share.Blob, + opts ...user.TxOption, ) [][]byte { t.Helper() txs := make([][]byte, len(accounts)) - opts := DefaultTxOpts() 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) From 156dc32342471c1f4bcfc0ca0d8c965f50ea07c5 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 12:37:39 -0400 Subject: [PATCH 10/16] fix: local test --- app/ante/max_tx_size_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go index 4b5187c3ff..9434e646d6 100644 --- a/app/ante/max_tx_size_test.go +++ b/app/ante/max_tx_size_test.go @@ -7,7 +7,6 @@ import ( v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" version "github.com/tendermint/tendermint/proto/tendermint/version" @@ -63,7 +62,7 @@ func TestMaxTxSizeDecorator(t *testing.T) { _, err := anteHandler(ctx, nil, false) if tc.expectError { require.Error(t, err) - require.Contains(t, err.Error(), sdkerrors.ErrTxTooLarge.Error()) + require.Contains(t, err.Error(), "tx size is larger than the application's configured threshold") } else { require.NoError(t, err) } From 6dd6a1facfa330371463ebc7330ec6850c709a73 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 12:38:25 -0400 Subject: [PATCH 11/16] style: lint --- app/test/process_proposal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index 40495df97a..1c0ab641b8 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -66,7 +66,7 @@ func TestProcessProposal(t *testing.T) { ), user.SetMemo(largeMemo)) - // create 1 large sendTx that includes a large memo making the + // create 1 large sendTx that includes a large memo making the // transaction over the configured max tx bytes limit largeSendTx := testutil.SendTxsWithAccounts( t, testApp, enc, kr, 1000, accounts[0], accounts[1:2], testutil.ChainID, user.SetMemo(largeMemo), From c73e687ab98cf381990a943d7b775f2894aa46cd Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 12:41:46 -0400 Subject: [PATCH 12/16] style: gofumped --- app/ante/max_tx_size.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index 99989b72ba..2c595bfb96 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -2,6 +2,7 @@ package ante import ( "fmt" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" sdk "github.com/cosmos/cosmos-sdk/types" From 12dfb019c9e641eb946a37304dd0455c15b8f067 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: Thu, 10 Oct 2024 16:47:07 -0400 Subject: [PATCH 13/16] Update app/test/prepare_proposal_test.go Co-authored-by: Rootul P --- app/test/prepare_proposal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 5f9b8fd1be..36b1a4e439 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -140,7 +140,7 @@ func TestPrepareProposalFiltering(t *testing.T) { require.NoError(t, err) noAccountTx := []byte(testutil.SendTxWithManualSequence(t, encConf.TxConfig, kr, nilAccount, accounts[0], 1000, "", 0, 6)) - // memo is 2MG resulting in the transaction being over limit + // memo is 2 MiB resulting in the transaction being over limit largeString := strings.Repeat("a", 2*1024*1024) // 3 transactions over MaxTxBytes limit From 54b2b4d1ae3874579b1617aee8da258be4bd7f4c Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 17:04:11 -0400 Subject: [PATCH 14/16] fix: version gate in process proposal --- app/ante/max_tx_size.go | 6 ++++-- app/ante/max_tx_size_test.go | 33 ++++++++++++++++++++------------- app/process_proposal.go | 6 ++++-- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/app/ante/max_tx_size.go b/app/ante/max_tx_size.go index 2c595bfb96..2936081aca 100644 --- a/app/ante/max_tx_size.go +++ b/app/ante/max_tx_size.go @@ -23,9 +23,11 @@ func (d MaxTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool return next(ctx, tx, simulate) } + currentTxSize := len(ctx.TxBytes()) maxTxBytes := appconsts.MaxTxBytes(ctx.BlockHeader().Version.App) - if len(ctx.TxBytes()) > maxTxBytes { - return ctx, fmt.Errorf("tx size is larger than the application's configured threshold: %d bytes", maxTxBytes) + 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) } return next(ctx, tx, simulate) } diff --git a/app/ante/max_tx_size_test.go b/app/ante/max_tx_size_test.go index 9434e646d6..3847a0b6a1 100644 --- a/app/ante/max_tx_size_test.go +++ b/app/ante/max_tx_size_test.go @@ -21,50 +21,57 @@ func TestMaxTxSizeDecorator(t *testing.T) { txSize int expectError bool appVersion uint64 + isCheckTx []bool }{ { name: "good tx; under max tx bytes threshold", txSize: v3.MaxTxBytes - 1, appVersion: v3.Version, expectError: false, + isCheckTx: []bool{true, false}, }, { name: "bad tx; over max tx bytes threshold", txSize: v3.MaxTxBytes + 1, appVersion: v3.Version, expectError: true, + isCheckTx: []bool{true, false}, }, { name: "good tx; equal to max tx bytes threshold", txSize: v3.MaxTxBytes, appVersion: v3.Version, expectError: false, + isCheckTx: []bool{true, false}, }, { name: "good tx; limit only applies to v3 and above", txSize: v3.MaxTxBytes + 10, appVersion: v2.Version, expectError: false, + isCheckTx: []bool{true, false}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - ctx := sdk.NewContext(nil, tmproto.Header{ - Version: version.Consensus{ - App: tc.appVersion, - }, - }, false, nil) + for _, isCheckTx := range tc.isCheckTx { - txBytes := make([]byte, tc.txSize) + ctx := sdk.NewContext(nil, tmproto.Header{ + Version: version.Consensus{ + App: tc.appVersion, + }, + }, isCheckTx, nil) - ctx = ctx.WithTxBytes(txBytes) - _, err := anteHandler(ctx, nil, false) - if tc.expectError { - require.Error(t, err) - require.Contains(t, err.Error(), "tx size is larger than the application's configured threshold") - } else { - require.NoError(t, err) + txBytes := make([]byte, tc.txSize) + + ctx = ctx.WithTxBytes(txBytes) + _, err := anteHandler(ctx, nil, false) + if tc.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } } }) } diff --git a/app/process_proposal.go b/app/process_proposal.go index 34ba8645c9..7b51634c80 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -118,8 +118,10 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp return reject() } - // Set the tx size on the context before calling the AnteHandler - sdkCtx = sdkCtx.WithTxBytes(tx) + // set the tx bytes in the context for app version v3 and greater + if sdkCtx.BlockHeader().Version.App >= 3 { + sdkCtx = sdkCtx.WithTxBytes(tx) + } // validated the PFB signature sdkCtx, err = handler(sdkCtx, sdkTx, false) From a873099ce3d6f205a1bcf5c6768c3a9926852a7f Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 10 Oct 2024 17:16:33 -0400 Subject: [PATCH 15/16] refactor: reduce repetition in process proposal --- app/process_proposal.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/process_proposal.go b/app/process_proposal.go index 7b51634c80..fa9e444b00 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -68,6 +68,11 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp } sdkTx, err := app.txConfig.TxDecoder()(tx) + // Set the tx bytes in the context for app version v3 and greater + if sdkCtx.BlockHeader().Version.App >= 3 { + sdkCtx = sdkCtx.WithTxBytes(tx) + } + if err != nil { if req.Header.Version.App == v1 { // For appVersion 1, there was no block validity rule that all @@ -90,9 +95,6 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp return reject() } - // Set the tx size on the context before calling the AnteHandler - sdkCtx = sdkCtx.WithTxBytes(tx) - // we need to increment the sequence for every transaction so that // the signature check below is accurate. this error only gets hit // if the account in question doesn't exist. @@ -118,11 +120,6 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) (resp abci.Resp return reject() } - // set the tx bytes in the context for app version v3 and greater - if sdkCtx.BlockHeader().Version.App >= 3 { - sdkCtx = sdkCtx.WithTxBytes(tx) - } - // validated the PFB signature sdkCtx, err = handler(sdkCtx, sdkTx, false) if err != nil { From f6002273be2acb7293d41bb16c93381c0dda7af9 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 11 Oct 2024 12:32:14 -0400 Subject: [PATCH 16/16] fix: failing test --- app/test/prepare_proposal_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/test/prepare_proposal_test.go b/app/test/prepare_proposal_test.go index 775fec59c1..58ebc0daae 100644 --- a/app/test/prepare_proposal_test.go +++ b/app/test/prepare_proposal_test.go @@ -41,6 +41,7 @@ func TestPrepareProposalPutsPFBsAtEnd(t *testing.T) { accnts[:numBlobTxs], infos[:numBlobTxs], testfactory.Repeat([]*share.Blob{protoBlob}, numBlobTxs), + blobfactory.DefaultTxOpts()..., ) normalTxs := testutil.SendTxsWithAccounts(