feat(mempool)!: respect gas wanted returned by ante handler

CHANGELOG.md

@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Breaking Changes
 
+* (mempool) [#25338](https://github.com/cosmos/cosmos-sdk/pull/25338) Respect gas wanted returned by the ante handler for block selection. Adds `InsertWithOption` to the `Mempool` interface (carries the ante-reported `GasWanted`) and changes the `SelectBy` callback to receive a `mempool.Tx` wrapper that exposes the stored value.
 * (modules) [#26421](https://github.com/cosmos/cosmos-sdk/pull/26421) Remove the `x/protocolpool` module and its API/proto surface from the SDK. Applications upgrading from v0.54 should include `protocolpool` in deleted store upgrades.
 * (genutils) [#26468](https://github.com/cosmos/cosmos-sdk/pull/26468) Consolidate ExportGenesisFileWithTime arguments to preserve consensus params.
 

baseapp/abci_test.go

@@ -1562,7 +1562,7 @@ func TestABCI_Proposal_HappyPath(t *testing.T) {
 	tx2Bytes, err := suite.txConfig.TxEncoder()(tx2)
 	require.NoError(t, err)
 
-	err = pool.Insert(sdk.Context{}, tx2)
+	err = pool.Insert(sdk.Context{}, tx2, mempool.InsertOption{})
 	require.NoError(t, err)
 
 	reqPrepareProposal := abci.RequestPrepareProposal{
@@ -1726,7 +1726,7 @@ func TestABCI_PrepareProposal_ReachedMaxBytes(t *testing.T) {
 
 	for i := range 100 {
 		tx2 := newTxCounter(t, suite.txConfig, int64(i), int64(i))
-		err := pool.Insert(sdk.Context{}, tx2)
+		err := pool.Insert(sdk.Context{}, tx2, mempool.InsertOption{})
 		require.NoError(t, err)
 	}
 
@@ -1755,7 +1755,7 @@ func TestABCI_PrepareProposal_BadEncoding(t *testing.T) {
 	require.NoError(t, err)
 
 	tx := newTxCounter(t, suite.txConfig, 0, 0)
-	err = pool.Insert(sdk.Context{}, tx)
+	err = pool.Insert(sdk.Context{}, tx, mempool.InsertOption{})
 	require.NoError(t, err)
 
 	reqPrepareProposal := abci.RequestPrepareProposal{
@@ -1793,7 +1793,7 @@ func TestABCI_PrepareProposal_OverGasUnderBytes(t *testing.T) {
 		builder.SetGasLimit(10)
 		setTxSignature(t, builder, uint64(i))
 
-		err := pool.Insert(sdk.Context{}, builder.GetTx())
+		err := pool.Insert(sdk.Context{}, builder.GetTx(), mempool.InsertOption{GasWanted: 10})
 		require.NoError(t, err)
 	}
 
@@ -1833,7 +1833,7 @@ func TestABCI_PrepareProposal_MaxGas(t *testing.T) {
 		builder.SetGasLimit(10)
 		setTxSignature(t, builder, uint64(i))
 
-		err := pool.Insert(sdk.Context{}, builder.GetTx())
+		err := pool.Insert(sdk.Context{}, builder.GetTx(), mempool.InsertOption{GasWanted: 10})
 		require.NoError(t, err)
 	}
 
@@ -1876,7 +1876,7 @@ func TestABCI_PrepareProposal_Failures(t *testing.T) {
 	failTx := newTxCounter(t, suite.txConfig, 1, 1)
 	failTx = setFailOnAnte(t, suite.txConfig, failTx, true)
 
-	err = pool.Insert(sdk.Context{}, failTx)
+	err = pool.Insert(sdk.Context{}, failTx, mempool.InsertOption{})
 	require.NoError(t, err)
 	require.Equal(t, 2, pool.CountTx())
 
@@ -2580,7 +2580,7 @@ func TestABCI_Proposal_FailReCheckTx(t *testing.T) {
 	tx2Bytes, err := suite.txConfig.TxEncoder()(tx2)
 	require.NoError(t, err)
 
-	err = pool.Insert(sdk.Context{}, tx2)
+	err = pool.Insert(sdk.Context{}, tx2, mempool.InsertOption{})
 	require.NoError(t, err)
 
 	require.Equal(t, 2, pool.CountTx())

baseapp/abci_utils.go

@@ -201,7 +201,7 @@ type (
 	// to verify a transaction.
 	ProposalTxVerifier interface {
 		PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error)
-		ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error)
+		ProcessProposalVerifyTx(txBz []byte) (msg sdk.Tx, gasWanted uint64, err error)
 		TxDecode(txBz []byte) (sdk.Tx, error)
 		TxEncode(tx sdk.Tx) ([]byte, error)
 	}
@@ -225,6 +225,21 @@ func NewDefaultProposalHandler(mp mempool.Mempool, txVerifier ProposalTxVerifier
 	}
 }
 
+// txGasForBlockAccounting returns the gas to count against MaxGas for tx.
+// Prefers the ante-reported gasWanted; falls back to the tx's declared
+// GetGas() when the ante returned 0 so a buggy/permissive ante can't let a
+// proposer slip a block past MaxGas. Used by both PrepareProposal selection
+// and ProcessProposal verification so the two paths stay in agreement.
+func txGasForBlockAccounting(tx sdk.Tx, gasWanted uint64) uint64 {
+	if gasWanted > 0 {
+		return gasWanted
+	}
+	if gasTx, ok := tx.(sdk.GasTx); ok {
+		return gasTx.GetGas()
+	}
+	return 0
+}
+
 // SetTxSelector sets the TxSelector function on the DefaultProposalHandler.
 func (h *DefaultProposalHandler) SetTxSelector(ts TxSelector) {
 	h.txSelector = ts
@@ -276,7 +291,12 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
 					return nil, err
 				}
 
-				stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz)
+				var txGasLimit uint64
+				if gasTx, ok := tx.(sdk.GasTx); ok {
+					txGasLimit = gasTx.GetGas()
+				}
+
+				stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz, txGasLimit)
 				if stop {
 					break
 				}
@@ -297,15 +317,14 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
 			selectedTxsNums        int
 			selectedTxsSignersSeqs = make(map[string]uint64)
 		)
-
-		mempool.SelectBy(ctx, h.mempool, req.Txs, func(memTx sdk.Tx) bool {
-			unorderedTx, ok := memTx.(sdk.TxWithUnordered)
+		mempool.SelectBy(ctx, h.mempool, req.Txs, func(memTx mempool.PooledTx) bool {
+			unorderedTx, ok := memTx.Tx.(sdk.TxWithUnordered)
 			isUnordered := ok && unorderedTx.GetUnordered()
 			txSignersSeqs := make(map[string]uint64)
 
 			// if the tx is unordered, we don't need to check the sequence, we just add it
 			if !isUnordered {
-				signerData, err := h.signerExtAdapter.GetSigners(memTx)
+				signerData, err := h.signerExtAdapter.GetSigners(memTx.Tx)
 				if err != nil {
 					// propagate the error to the caller
 					resError = err
@@ -340,11 +359,11 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan
 			// which calls mempool.Insert, in theory everything in the pool should be
 			// valid. But some mempool implementations may insert invalid txs, so we
 			// check again.
-			txBz, err := h.txVerifier.PrepareProposalVerifyTx(memTx)
+			txBz, err := h.txVerifier.PrepareProposalVerifyTx(memTx.Tx)
 			if err != nil {
-				invalidTxs = append(invalidTxs, invalidTx{tx: memTx, err: err})
+				invalidTxs = append(invalidTxs, invalidTx{tx: memTx.Tx, err: err})
 			} else {
-				stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, memTx, txBz)
+				stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, memTx.Tx, txBz, memTx.GasWanted)
 				if stop {
 					return false
 				}
@@ -421,17 +440,13 @@ func (h *DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHan
 		}
 
 		for _, txBytes := range req.Txs {
-			tx, err := h.txVerifier.ProcessProposalVerifyTx(txBytes)
+			tx, gasWanted, 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()
-				}
-
+				totalTxGas += txGasForBlockAccounting(tx, gasWanted)
 				if totalTxGas > uint64(maxBlockGas) {
 					return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil
 				}
@@ -489,7 +504,7 @@ type TxSelector interface {
 	// a proposal based on inclusion criteria defined by the TxSelector. It must
 	// return <true> if the caller should halt the transaction selection loop
 	// (typically over a mempool) or <false> otherwise.
-	SelectTxForProposal(ctx context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte) bool
+	SelectTxForProposal(ctx context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte, gasWanted uint64) bool
 }
 
 type defaultTxSelector struct {
@@ -514,23 +529,17 @@ func (ts *defaultTxSelector) Clear() {
 	ts.selectedTxs = nil
 }
 
-func (ts *defaultTxSelector) SelectTxForProposal(_ context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte) bool {
+func (ts *defaultTxSelector) SelectTxForProposal(_ context.Context, maxTxBytes, maxBlockGas uint64, memTx sdk.Tx, txBz []byte, gasWanted uint64) bool {
 	txSize := uint64(cmttypes.ComputeProtoSizeForTxs([]cmttypes.Tx{txBz}))
 
-	var txGasLimit uint64
-	if memTx != nil {
-		if gasTx, ok := memTx.(GasTx); ok {
-			txGasLimit = gasTx.GetGas()
-		}
-	}
-
 	// only add the transaction to the proposal if we have enough capacity
 	if (txSize + ts.totalTxBytes) <= maxTxBytes {
 		// If there is a max block gas limit, add the tx only if the limit has
 		// not been met.
 		if maxBlockGas > 0 {
-			if (txGasLimit + ts.totalTxGas) <= maxBlockGas {
-				ts.totalTxGas += txGasLimit
+			txGas := txGasForBlockAccounting(memTx, gasWanted)
+			if (txGas + ts.totalTxGas) <= maxBlockGas {
+				ts.totalTxGas += txGas
 				ts.totalTxBytes += txSize
 				ts.selectedTxs = append(ts.selectedTxs, txBz)
 			}

baseapp/abci_utils_test.go

@@ -683,7 +683,7 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 
 			for _, v := range tc.txInputs {
 				app.EXPECT().PrepareProposalVerifyTx(v.tx).Return(v.bz, nil).AnyTimes()
-				s.NoError(mp.Insert(s.ctx.WithPriority(v.priority), v.tx))
+				s.NoError(mp.Insert(s.ctx.WithPriority(v.priority), v.tx, mempool.InsertOption{}))
 				tc.req.Txs = append(tc.req.Txs, v.bz)
 			}
 
@@ -703,6 +703,75 @@ func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_PriorityNonceMempoolTxSe
 	}
 }
 
+func (s *ABCIUtilsTestSuite) buildSignedTx(declaredGas uint64) (sdk.Tx, []byte) {
+	s.T().Helper()
+	cdc := codectestutil.CodecOptions{}.NewCodec()
+	baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry())
+	txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes)
+
+	secret := []byte(s.T().Name())
+	builder := txConfig.NewTxBuilder()
+	s.Require().NoError(builder.SetMsgs(&baseapptestutil.MsgKeyValue{Value: secret}))
+	builder.SetGasLimit(declaredGas)
+	privKey := secp256k1.GenPrivKeyFromSecret(secret)
+	setTxSignatureWithSecret(s.T(), builder, signingtypes.SignatureV2{
+		PubKey:   privKey.PubKey(),
+		Sequence: 1,
+		Data:     &signingtypes.SingleSignatureData{},
+	})
+	tx := builder.GetTx()
+	bz, err := txConfig.TxEncoder()(tx)
+	s.Require().NoError(err)
+	return tx, bz
+}
+
+func (s *ABCIUtilsTestSuite) TestDefaultProposalHandler_RejectsOverGasTxs() {
+	cases := []struct {
+		name        string
+		declaredGas uint64
+		recordedGas uint64 // mempool-stored for Prepare, ante-returned for Process
+		maxBlockGas int64
+	}{
+		{"ante-beats-tx-gas", 50, 200, 100}, // declared 50 < MaxGas, ante 200 > MaxGas
+		{"zero-ante-fallback", 200, 0, 100}, // ante 0, falls back to declared 200 > MaxGas
+	}
+	for _, tc := range cases {
+		s.Run(tc.name, func() {
+			tx, bz := s.buildSignedTx(tc.declaredGas)
+			ctx := s.ctx.WithConsensusParams(cmtproto.ConsensusParams{
+				Block: &cmtproto.BlockParams{MaxGas: tc.maxBlockGas},
+			})
+
+			app := mock.NewMockProposalTxVerifier(gomock.NewController(s.T()))
+			app.EXPECT().PrepareProposalVerifyTx(tx).Return(bz, nil).AnyTimes()
+			app.EXPECT().ProcessProposalVerifyTx(bz).Return(tx, tc.recordedGas, nil).AnyTimes()
+
+			// ProcessProposalHandler short-circuits to NoOp when mempool is nil/NoOp,
+			// so use a real one. The Insert is what Prepare reads; Process ignores it.
+			mp := mempool.NewPriorityMempool(mempool.DefaultPriorityNonceMempoolConfig())
+			s.Require().NoError(mp.Insert(
+				s.ctx.WithPriority(1), tx,
+				mempool.InsertOption{GasWanted: tc.recordedGas},
+			))
+
+			ph := baseapp.NewDefaultProposalHandler(mp, app)
+
+			prepResp, err := ph.PrepareProposalHandler()(ctx,
+				&abci.RequestPrepareProposal{Txs: [][]byte{bz}, MaxTxBytes: int64(len(bz)) * 2},
+			)
+			s.Require().NoError(err)
+			s.Require().Empty(prepResp.Txs, "PrepareProposal should have excluded the over-gas tx")
+
+			procResp, err := ph.ProcessProposalHandler()(ctx,
+				&abci.RequestProcessProposal{Txs: [][]byte{bz}},
+			)
+			s.Require().NoError(err)
+			s.Require().Equal(abci.ResponseProcessProposal_REJECT, procResp.Status,
+				"ProcessProposal should have rejected the over-gas block")
+		})
+	}
+}
+
 func marshalDelimitedFn(msg proto.Message) ([]byte, error) {
 	var buf bytes.Buffer
 	if err := protoio.NewDelimitedWriter(&buf).WriteMsg(msg); err != nil {

baseapp/baseapp.go

@@ -916,7 +916,7 @@ func (app *BaseApp) RunTx(mode sdk.ExecMode, txBytes []byte, tx sdk.Tx, txIndex
 
 	switch mode {
 	case execModeCheck:
-		err = app.mempool.Insert(ctx, tx)
+		err = app.mempool.Insert(ctx, tx, mempool.InsertOption{GasWanted: gasWanted})
 		if err != nil {
 			return gInfo, nil, anteEvents, err
 		}
@@ -1125,18 +1125,18 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) {
 // ProcessProposal state internally will be discarded. <nil, err> will be
 // returned if the transaction cannot be decoded. <Tx, nil> will be returned if
 // the transaction is valid, otherwise <Tx, err> will be returned.
-func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) {
+func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (msg sdk.Tx, gasWanted uint64, err error) {
 	tx, err := app.txDecoder(txBz)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	_, _, _, err = app.RunTx(execModeProcessProposal, txBz, tx, -1, nil, nil)
+	gInfo, _, _, err := app.RunTx(execModeProcessProposal, txBz, tx, -1, nil, nil)
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
 
-	return tx, nil
+	return tx, gInfo.GasWanted, nil
 }
 
 func (app *BaseApp) TxDecode(txBytes []byte) (sdk.Tx, error) {