fix: check tx gas limit against block gas limit

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`.

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,43 @@ 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)
+ }
+
+ // 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,
+ })
+ 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()

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
@@ -186,9 +191,15 @@ 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
+ }
+
  var (
  selectedTxs [][]byte
  totalTxBytes int64
+ totalTxGas uint64
  )
 
  iterator := h.mempool.Select(ctx, req.Txs)
@@ -207,12 +218,31 @@ 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()
+ }
+
+ // 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 {
+ totalTxBytes += txSize
+ selectedTxs = append(selectedTxs, bz)
+ }
+ }
+
+ // Check if we've reached capacity. If so, we cannot select any more
+ // transactions.
+ if totalTxBytes >= req.MaxTxBytes || (maxBlockGas > 0 && (totalTxGas >= uint64(maxBlockGas))) {
  break
  }
  }
@@ -244,11 +274,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

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,7 @@ 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 +176,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

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"

docs/docs/building-apps/02-app-mempool.md

@@ -44,7 +44,10 @@ 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) 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
@@ -81,7 +84,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) {

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,

x/auth/ante/setup.go

@@ -40,6 +40,14 @@ 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 limit %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

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()