From b29ea13fabfaf75899dbd436d090e964237539b8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 11:40:44 -0400 Subject: [PATCH 01/18] updates --- x/auth/ante/setup.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 4b6e281ac394..d10eb8ac14f2 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -40,6 +40,12 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) + if cp := ctx.ConsensusParams(); cp.Block != nil { + if gasTx.GetGas() > uint64(cp.Block.MaxGas) { + return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) + } + } + // Decorator will catch an OutOfGasPanic caused in the next antehandler // AnteHandlers must have their own defer/recover in order for the BaseApp // to know how much gas was used! This is because the GasMeter is created in From 9816ad55abe2caf8d27b4a41419cfca716d083d3 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 11:44:05 -0400 Subject: [PATCH 02/18] updates --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 279fe498da03..49f989ab9ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Bug Fixes + +* [#16547](https://github.com/cosmos/cosmos-sdk/pull/16547) Ensure a transaction's gas limit cannot exceed the block gas limit. + ### Improvements * (all) [#16497](https://github.com/cosmos/cosmos-sdk/pull/16497) Removed all exported vestiges of `sdk.MustSortJSON` and `sdk.SortJSON`. From 4ee99d922f08616b9a1f77d5e0d6b11a2f509cde Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 11:44:36 -0400 Subject: [PATCH 03/18] updates --- x/auth/ante/setup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index d10eb8ac14f2..97900b61bf53 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -41,7 +41,7 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) if cp := ctx.ConsensusParams(); cp.Block != nil { - if gasTx.GetGas() > uint64(cp.Block.MaxGas) { + if cp.Block.MaxGas > 0 && gasTx.GetGas() > uint64(cp.Block.MaxGas) { return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) } } From b2bd73f9cd008a0158560e3d41515a5916120720 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 11:45:04 -0400 Subject: [PATCH 04/18] updates --- x/auth/ante/setup.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 97900b61bf53..562fd93bc4e4 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -41,6 +41,8 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) if cp := ctx.ConsensusParams(); cp.Block != nil { + // If there exists a maximum block gas limit, we must ensure that the tx + // does not exceed it. if cp.Block.MaxGas > 0 && gasTx.GetGas() > uint64(cp.Block.MaxGas) { return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) } From c83162f09e4d41815f77c7486ccfac787b421fb8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 11:48:48 -0400 Subject: [PATCH 05/18] updates --- x/auth/ante/setup.go | 2 +- x/auth/ante/setup_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 562fd93bc4e4..06ffc6e4ce06 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -44,7 +44,7 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate // If there exists a maximum block gas limit, we must ensure that the tx // does not exceed it. if cp.Block.MaxGas > 0 && gasTx.GetGas() > uint64(cp.Block.MaxGas) { - return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) + return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas limit %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) } } diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index 67c408218af1..70964e4075c6 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -4,6 +4,7 @@ import ( "testing" storetypes "cosmossdk.io/store/types" + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "github.com/stretchr/testify/require" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" @@ -14,6 +15,40 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/ante" ) +func TestSetupDecorator_BlockMaxGas(t *testing.T) { + suite := SetupTestSuite(t, true) + suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + feeAmount := testdata.NewTestFeeAmount() + require.NoError(t, suite.txBuilder.SetMsgs(msg)) + suite.txBuilder.SetFeeAmount(feeAmount) + suite.txBuilder.SetGasLimit(101) + + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx, err := suite.CreateTestTx(suite.ctx, privs, accNums, accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT) + require.NoError(t, err) + + sud := ante.NewSetUpContextDecorator() + antehandler := sdk.ChainAnteDecorators(sud) + + suite.ctx = suite.ctx. + WithBlockHeight(1). + WithGasMeter(storetypes.NewGasMeter(0)). + WithConsensusParams(cmtproto.ConsensusParams{ + Block: &cmtproto.BlockParams{ + MaxGas: 100, + }, + }) + + _, err = antehandler(suite.ctx, tx, false) + require.Error(t, err) +} + func TestSetup(t *testing.T) { suite := SetupTestSuite(t, true) suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() From f804f1e28a2b5f4c16ac10a4380421507c9307fb Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 14 Jun 2023 12:13:48 -0400 Subject: [PATCH 06/18] updates --- testutil/sims/app_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/sims/app_helpers.go b/testutil/sims/app_helpers.go index 6fdbec1f5b71..cbc22ad2a52d 100644 --- a/testutil/sims/app_helpers.go +++ b/testutil/sims/app_helpers.go @@ -34,7 +34,7 @@ const DefaultGenTxGas = 10000000 var DefaultConsensusParams = &cmtproto.ConsensusParams{ Block: &cmtproto.BlockParams{ MaxBytes: 200000, - MaxGas: 2000000, + MaxGas: 100_000_000, }, Evidence: &cmtproto.EvidenceParams{ MaxAgeNumBlocks: 302400, From 20fc493adcd0f0bcf3f9f8d7a5c123d15cd9f27a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 15 Jun 2023 09:57:34 -0400 Subject: [PATCH 07/18] updates --- baseapp/abci_utils.go | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 465dfa516416..4b2a6426f209 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -186,9 +186,19 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand return &abci.ResponsePrepareProposal{Txs: req.Txs}, nil } + var maxBlockGas int64 + if b := ctx.ConsensusParams().Block; b != nil { + maxBlockGas = b.MaxGas + } + + type GasTx interface { + GetGas() uint64 + } + var ( selectedTxs [][]byte totalTxBytes int64 + totalTxGas uint64 ) iterator := h.mempool.Select(ctx, req.Txs) @@ -207,12 +217,28 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand panic(err) } } else { + var txGasLimit uint64 txSize := int64(len(bz)) - if totalTxBytes += txSize; totalTxBytes <= req.MaxTxBytes { - selectedTxs = append(selectedTxs, bz) - } else { - // We've reached capacity per req.MaxTxBytes so we cannot select any - // more transactions. + + gasTx, ok := memTx.(GasTx) + if ok { + txGasLimit = gasTx.GetGas() + } + + if maxBlockGas > 0 { + // Only add the transaction to the proposal if we have enough gas capacity + // and space. + if (txGasLimit+totalTxGas) < uint64(maxBlockGas) && (txSize+totalTxBytes) < req.MaxTxBytes { + totalTxGas += txGasLimit + totalTxBytes += txSize + + selectedTxs = append(selectedTxs, bz) + } + } + + // Check if we've reached capacity. If so, we cannot select any more + // transactions. + if totalTxBytes >= req.MaxTxBytes || totalTxGas >= uint64(maxBlockGas) { break } } From bbcdecefa8c32aef1508fe15d2c4fcc826a756b7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 15 Jun 2023 09:59:13 -0400 Subject: [PATCH 08/18] updates --- baseapp/abci_utils.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 4b2a6426f209..720b90963323 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -232,6 +232,12 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand totalTxGas += txGasLimit totalTxBytes += txSize + selectedTxs = append(selectedTxs, bz) + } + } else { + if (txSize + totalTxBytes) < req.MaxTxBytes { + totalTxBytes += txSize + selectedTxs = append(selectedTxs, bz) } } From ac7286982f2c4ce9ea9c3510f6b4de742928a56f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 15 Jun 2023 10:00:16 -0400 Subject: [PATCH 09/18] updates --- baseapp/abci_utils.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 720b90963323..018ff9c67a43 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -226,8 +226,8 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand } if maxBlockGas > 0 { - // Only add the transaction to the proposal if we have enough gas capacity - // and space. + // Only add the transaction to the proposal if we have enough gas + // capacity AND space. if (txGasLimit+totalTxGas) < uint64(maxBlockGas) && (txSize+totalTxBytes) < req.MaxTxBytes { totalTxGas += txGasLimit totalTxBytes += txSize @@ -235,6 +235,7 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand selectedTxs = append(selectedTxs, bz) } } else { + // Only add the transaction to the proposal if we have enough space. if (txSize + totalTxBytes) < req.MaxTxBytes { totalTxBytes += txSize From e8837b72f41759de4ebe2870732f0ab448d1ecca Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 15 Jun 2023 14:58:14 -0400 Subject: [PATCH 10/18] updates --- baseapp/abci_utils.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 018ff9c67a43..4014d4884ed8 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -40,6 +40,11 @@ type ( GetValidatorByConsAddr(sdk.Context, cryptotypes.Address) (Validator, error) TotalBondedTokens(ctx sdk.Context) math.Int } + + // GasTx defines the contract that a transaction with a gas limit must implement. + GasTx interface { + GetGas() uint64 + } ) // ValidateVoteExtensions defines a helper function for verifying vote extension @@ -191,10 +196,6 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand maxBlockGas = b.MaxGas } - type GasTx interface { - GetGas() uint64 - } - var ( selectedTxs [][]byte totalTxBytes int64 @@ -277,11 +278,29 @@ func (h DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHand } return func(ctx sdk.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) { + var totalTxGas uint64 + + var maxBlockGas int64 + if b := ctx.ConsensusParams().Block; b != nil { + maxBlockGas = b.MaxGas + } + for _, txBytes := range req.Txs { - _, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) + tx, err := h.txVerifier.ProcessProposalVerifyTx(txBytes) if err != nil { return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil } + + if maxBlockGas > 0 { + gasTx, ok := tx.(GasTx) + if ok { + totalTxGas += gasTx.GetGas() + } + + if totalTxGas > uint64(maxBlockGas) { + return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil + } + } } return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}, nil From 667765b6dcc097117b14c8650a8a1a24fc620a90 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 16 Jun 2023 11:57:59 -0400 Subject: [PATCH 11/18] updates --- baseapp/abci_utils.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 4014d4884ed8..6efa2886fa9e 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -226,20 +226,16 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand txGasLimit = gasTx.GetGas() } - if maxBlockGas > 0 { - // Only add the transaction to the proposal if we have enough gas - // capacity AND space. - if (txGasLimit+totalTxGas) < uint64(maxBlockGas) && (txSize+totalTxBytes) < req.MaxTxBytes { + // only add the transaction to the proposal if we have enough capacity + if (txSize + totalTxBytes) < req.MaxTxBytes { + // If there is a max block gas limit, add the tx only if the limit has + // not been met. + if maxBlockGas > 0 && (txGasLimit+totalTxGas) < uint64(maxBlockGas) { totalTxGas += txGasLimit totalTxBytes += txSize - selectedTxs = append(selectedTxs, bz) - } - } else { - // Only add the transaction to the proposal if we have enough space. - if (txSize + totalTxBytes) < req.MaxTxBytes { + } else { totalTxBytes += txSize - selectedTxs = append(selectedTxs, bz) } } From fde15473864bacf9f3225c0c564fc79962633341 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 16 Jun 2023 12:28:24 -0400 Subject: [PATCH 12/18] docs++ --- docs/docs/building-apps/02-app-mempool.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/docs/building-apps/02-app-mempool.md b/docs/docs/building-apps/02-app-mempool.md index a8083f76f42b..ee047a5e20e1 100644 --- a/docs/docs/building-apps/02-app-mempool.md +++ b/docs/docs/building-apps/02-app-mempool.md @@ -44,7 +44,9 @@ Allowing the application to handle ordering enables the application to define ho it would like the block constructed. The Cosmos SDK defines the `DefaultProposalHandler` type, which provides applications with -`PrepareProposal` and `ProcessProposal` handlers. +`PrepareProposal` and `ProcessProposal` handlers. If you decide to implement your +own `PrepareProposal` handler, you must be sure to ensure that the transactions +selected DO NOT exceed the maximum block gas (if set). ```go reference https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/baseapp/baseapp.go#L868-L916 @@ -81,7 +83,10 @@ Here is the implementation of the default implementation: https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/baseapp/baseapp.go#L927-L942 ``` -Like `PrepareProposal` this implementation is the default and can be modified by the application developer in [`app.go`](./01-app-go-v2.md): +Like `PrepareProposal` this implementation is the default and can be modified by +the application developer in [`app.go`](./01-app-go-v2.md). If you decide to implement +your own `ProcessProposal` handler, you must be sure to ensure that the transactions +provided in the proposal DO NOT exceed the maximum block gas (if set). ```go processOpt := func(app *baseapp.BaseApp) { From 44f55f140eb762ba45bf2e05a716a20ed73af8c4 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 20 Jun 2023 07:07:43 -0400 Subject: [PATCH 13/18] updates --- docs/docs/building-apps/02-app-mempool.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/docs/building-apps/02-app-mempool.md b/docs/docs/building-apps/02-app-mempool.md index ee047a5e20e1..9701df1d23f0 100644 --- a/docs/docs/building-apps/02-app-mempool.md +++ b/docs/docs/building-apps/02-app-mempool.md @@ -46,7 +46,8 @@ it would like the block constructed. The Cosmos SDK defines the `DefaultProposalHandler` type, which provides applications with `PrepareProposal` and `ProcessProposal` handlers. If you decide to implement your own `PrepareProposal` handler, you must be sure to ensure that the transactions -selected DO NOT exceed the maximum block gas (if set). +selected DO NOT exceed the maximum block gas (if set) and the maximum bytes provided +by `req.MaxBytes`. ```go reference https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/baseapp/baseapp.go#L868-L916 From de4da6b12c4802ed72fcb11215bdbd327bf4d311 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 20 Jun 2023 07:25:54 -0400 Subject: [PATCH 14/18] updates --- baseapp/abci_test.go | 47 +++++++++++++++++++++++++++++++++++++------ baseapp/abci_utils.go | 2 +- baseapp/utils_test.go | 1 + 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index f4ccaec78ef1..f6f3b1bdec02 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -5,23 +5,22 @@ import ( "context" "errors" "fmt" + "strconv" "strings" "testing" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" - + pruningtypes "cosmossdk.io/store/pruning/types" + "cosmossdk.io/store/snapshots" + snapshottypes "cosmossdk.io/store/snapshots/types" + storetypes "cosmossdk.io/store/types" abci "github.com/cometbft/cometbft/abci/types" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/jsonpb" "github.com/stretchr/testify/require" - pruningtypes "cosmossdk.io/store/pruning/types" - "cosmossdk.io/store/snapshots" - snapshottypes "cosmossdk.io/store/snapshots/types" - storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/baseapp" baseapptestutil "github.com/cosmos/cosmos-sdk/baseapp/testutil" "github.com/cosmos/cosmos-sdk/testutil" @@ -1286,6 +1285,42 @@ func TestABCI_PrepareProposal_BadEncoding(t *testing.T) { require.Equal(t, 1, len(resPrepareProposal.Txs)) } +func TestABCI_PrepareProposal_MaxGas(t *testing.T) { + pool := mempool.NewSenderNonceMempool() + suite := NewBaseAppSuite(t, baseapp.SetMempool(pool)) + baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), NoopCounterServerImpl{}) + + // set max block gas limit to 100 + suite.baseApp.InitChain(&abci.RequestInitChain{ + ConsensusParams: &cmtproto.ConsensusParams{ + Block: &cmtproto.BlockParams{MaxGas: 100}, + }, + }) + + // insert 100 txs, each with a gas limit of 10 + _, _, addr := testdata.KeyTestPubAddr() + for i := int64(0); i < 100; i++ { + msg := &baseapptestutil.MsgCounter{Counter: i, FailOnHandler: false, Signer: addr.String()} + msgs := []sdk.Msg{msg} + + builder := suite.txConfig.NewTxBuilder() + builder.SetMsgs(msgs...) + builder.SetMemo("counter=" + strconv.FormatInt(i, 10) + "&failOnAnte=false") + builder.SetGasLimit(10) + setTxSignature(t, builder, uint64(i)) + + err := pool.Insert(sdk.Context{}, builder.GetTx()) + require.NoError(t, err) + } + + res, err := suite.baseApp.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 1_000_000, // large enough to ignore restriction + Height: 1, + }) + require.NoError(t, err) + require.Len(t, res.Txs, 10, "invalid number of transactions returned") +} + func TestABCI_PrepareProposal_Failures(t *testing.T) { anteKey := []byte("ante-key") pool := mempool.NewSenderNonceMempool() diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 6efa2886fa9e..edc0babd86f3 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -230,7 +230,7 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand if (txSize + totalTxBytes) < req.MaxTxBytes { // If there is a max block gas limit, add the tx only if the limit has // not been met. - if maxBlockGas > 0 && (txGasLimit+totalTxGas) < uint64(maxBlockGas) { + if maxBlockGas > 0 && (txGasLimit+totalTxGas) <= uint64(maxBlockGas) { totalTxGas += txGasLimit totalTxBytes += txSize selectedTxs = append(selectedTxs, bz) diff --git a/baseapp/utils_test.go b/baseapp/utils_test.go index e436b47ad73e..154a73e84bda 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -25,6 +25,7 @@ import ( "cosmossdk.io/core/appconfig" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/testutil/testdata" "github.com/cosmos/cosmos-sdk/baseapp" From f8ffe1c7c5aabdfdf5095bbf52d2034eead8c4bc Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 20 Jun 2023 07:26:20 -0400 Subject: [PATCH 15/18] updates --- baseapp/abci_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index f6f3b1bdec02..a67c5d1fb8bf 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -1313,6 +1313,7 @@ func TestABCI_PrepareProposal_MaxGas(t *testing.T) { require.NoError(t, err) } + // ensure we only select transactions that fit within the block gas limit res, err := suite.baseApp.PrepareProposal(&abci.RequestPrepareProposal{ MaxTxBytes: 1_000_000, // large enough to ignore restriction Height: 1, From ff0fd01f2f5f510f0e31f4b88646e8a49af9879d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 20 Jun 2023 11:34:17 -0400 Subject: [PATCH 16/18] updates --- baseapp/abci_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index edc0babd86f3..930be5cd8197 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -242,7 +242,7 @@ func (h DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHand // Check if we've reached capacity. If so, we cannot select any more // transactions. - if totalTxBytes >= req.MaxTxBytes || totalTxGas >= uint64(maxBlockGas) { + if totalTxBytes >= req.MaxTxBytes || (maxBlockGas > 0 && (totalTxGas >= uint64(maxBlockGas))) { break } } From 5360e0f313783b0be612eaeae5d55e5c8e4bda83 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 20 Jun 2023 17:39:44 +0200 Subject: [PATCH 17/18] fix tests, might need to add an extra test that shows that if the tx gas limit is higher than maxblockgas it panics --- baseapp/block_gas_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/baseapp/block_gas_test.go b/baseapp/block_gas_test.go index 518f796fa5f1..413be7c59dad 100644 --- a/baseapp/block_gas_test.go +++ b/baseapp/block_gas_test.go @@ -64,7 +64,7 @@ func TestBaseApp_BlockGas(t *testing.T) { {"less than block gas meter", 10, false, false}, {"more than block gas meter", blockMaxGas, false, true}, {"more than block gas meter", uint64(float64(blockMaxGas) * 1.2), false, true}, - {"consume MaxUint64", math.MaxUint64, false, true}, + {"consume MaxUint64", math.MaxUint64, true, true}, {"consume MaxGasWanted", txtypes.MaxGasWanted, false, true}, {"consume block gas when panicked", 10, true, true}, } @@ -150,7 +150,8 @@ func TestBaseApp_BlockGas(t *testing.T) { require.NoError(t, txBuilder.SetMsgs(msg)) txBuilder.SetFeeAmount(feeAmount) - txBuilder.SetGasLimit(txtypes.MaxGasWanted) // tx validation checks that gasLimit can't be bigger than this + + txBuilder.SetGasLimit(uint64(simtestutil.DefaultConsensusParams.Block.MaxGas)) senderAccountNumber := accountKeeper.GetAccount(ctx, addr1).GetAccountNumber() privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{senderAccountNumber}, []uint64{0} @@ -176,11 +177,11 @@ func TestBaseApp_BlockGas(t *testing.T) { require.Equal(t, []byte("ok"), okValue) } // check block gas is always consumed - baseGas := uint64(57554) // baseGas is the gas consumed before tx msg + baseGas := uint64(57504) // baseGas is the gas consumed before tx msg expGasConsumed := addUint64Saturating(tc.gasToConsume, baseGas) - if expGasConsumed > txtypes.MaxGasWanted { + if expGasConsumed > uint64(simtestutil.DefaultConsensusParams.Block.MaxGas) { // capped by gasLimit - expGasConsumed = txtypes.MaxGasWanted + expGasConsumed = uint64(simtestutil.DefaultConsensusParams.Block.MaxGas) } require.Equal(t, int(expGasConsumed), int(ctx.BlockGasMeter().GasConsumed())) // tx fee is always deducted From 4ea9b70c61b7921f60f8542c84e9a9fa7ed9dba7 Mon Sep 17 00:00:00 2001 From: Facundo Medica Date: Tue, 20 Jun 2023 17:40:41 +0200 Subject: [PATCH 18/18] remove extra line --- baseapp/block_gas_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/baseapp/block_gas_test.go b/baseapp/block_gas_test.go index 413be7c59dad..0374246c2713 100644 --- a/baseapp/block_gas_test.go +++ b/baseapp/block_gas_test.go @@ -150,7 +150,6 @@ func TestBaseApp_BlockGas(t *testing.T) { require.NoError(t, txBuilder.SetMsgs(msg)) txBuilder.SetFeeAmount(feeAmount) - txBuilder.SetGasLimit(uint64(simtestutil.DefaultConsensusParams.Block.MaxGas)) senderAccountNumber := accountKeeper.GetAccount(ctx, addr1).GetAccountNumber()