Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4cd29a5
feat(mempool): respect gas wanted returned by ante handler
Jun 26, 2024
b54349d
fix mocks
mmsqe Sep 11, 2025
7be0362
Update CHANGELOG.md
mmsqe Sep 12, 2025
8937edb
Merge branch 'main' into fix_mempool
aljo242 Sep 12, 2025
0936ce1
Apply suggestions from code review
mmsqe Sep 12, 2025
99707c4
Apply suggestions from code review
mmsqe Sep 23, 2025
a4d9cfd
Merge remote-tracking branch 'origin/main' into fix_mempool
mmsqe Sep 23, 2025
c434a76
InsertWithOption
mmsqe Sep 24, 2025
010dfe1
mv GasTx
mmsqe Sep 24, 2025
24b265d
Merge branch 'main' into fix_mempool
aljo242 Sep 25, 2025
d4980fa
Merge branch 'main' into fix_mempool
aljo242 Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (baseapp) [#24655](https://github.com/cosmos/cosmos-sdk/pull/24655) Add mutex locks for `state` and make `lastCommitInfo` atomic to prevent race conditions between `Commit` and `CreateQueryContext`.
* (proto) [#24161](https://github.com/cosmos/cosmos-sdk/pull/24161) Remove unnecessary annotations from `x/staking` authz proto.
* (x/bank) [#24660](https://github.com/cosmos/cosmos-sdk/pull/24660) Improve performance of the `GetAllBalances` and `GetAccountsBalances` keeper methods.
* (mempool) [#25338](https://github.com/cosmos/cosmos-sdk/pull/25338) Respect gas wanted returned by ante handler for mempool.

### Bug Fixes

Expand Down
44 changes: 19 additions & 25 deletions baseapp/abci_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
// to verify a transaction.
ProposalTxVerifier interface {
PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error)
ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error)
ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, uint64, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint64 is a bit ambiguous, maybe a named return or comment to help the developer understand the variable

TxDecode(txBz []byte) (sdk.Tx, error)
TxEncode(tx sdk.Tx) ([]byte, error)
}
Expand Down Expand Up @@ -276,7 +276,12 @@
return nil, err
}

stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz)
var txGasLimit uint64
if gasTx, ok := tx.(mempool.GasTx); ok {
txGasLimit = gasTx.GetGas()
}

stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz, txGasLimit)
if stop {
break
}
Expand All @@ -291,14 +296,14 @@
selectedTxsNums int
invalidTxs []sdk.Tx // invalid txs to be removed out of the loop to avoid dead lock
)
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.Tx) 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
Expand Down Expand Up @@ -332,12 +337,12 @@
// NOTE: Since transaction verification was already executed in CheckTx,
// 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.

Check failure

Code scanning / gosec

integer overflow conversion uint64 -> uint32 Error

integer overflow conversion int64 -> uint64
txBz, err := h.txVerifier.PrepareProposalVerifyTx(memTx)
txBz, err := h.txVerifier.PrepareProposalVerifyTx(memTx.Tx)
if err != nil {
invalidTxs = append(invalidTxs, memTx)
invalidTxs = append(invalidTxs, memTx.Tx)
} 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
}
Expand Down Expand Up @@ -409,17 +414,13 @@
}

for _, txBytes := range req.Txs {
tx, err := h.txVerifier.ProcessProposalVerifyTx(txBytes)
_, 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 += gasWanted
if totalTxGas > uint64(maxBlockGas) {
return &abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}, nil
}
Expand Down Expand Up @@ -477,7 +478,7 @@
// 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 {
Expand All @@ -502,23 +503,16 @@
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
if (gasWanted + ts.totalTxGas) <= maxBlockGas {
ts.totalTxGas += gasWanted
ts.totalTxBytes += txSize
ts.selectedTxs = append(ts.selectedTxs, txBz)
}
Expand Down
12 changes: 6 additions & 6 deletions baseapp/baseapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ func (app *BaseApp) runTx(mode sdk.ExecMode, txBytes []byte, tx sdk.Tx) (gInfo s

switch mode {
case execModeCheck:
err = app.mempool.Insert(ctx, tx)
err = app.mempool.InsertWithGasWanted(ctx, tx, gasWanted)
if err != nil {
return gInfo, nil, anteEvents, err
}
Expand Down Expand Up @@ -1067,18 +1067,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) (sdk.Tx, uint64, error) {
Copy link
Contributor

@technicallyty technicallyty Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here and comments need to updated above too

tx, err := app.txDecoder(txBz)
if err != nil {
return nil, err
return nil, 0, err
}

_, _, _, err = app.runTx(execModeProcessProposal, txBz, tx)
gInfo, _, _, err := app.runTx(execModeProcessProposal, txBz, tx)
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) {
Expand Down
15 changes: 8 additions & 7 deletions baseapp/testutil/mock/mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 22 additions & 3 deletions types/mempool/mempool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,30 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

type Tx struct {
Tx sdk.Tx
GasWanted uint64
}

func NewMempoolTx(tx sdk.Tx, gasWanted uint64) Tx {
return Tx{
Tx: tx,
GasWanted: gasWanted,
}
}

type GasTx interface {
GetGas() uint64
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be defined with the other tx interfaces: types/tx_msg.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this interface is mainly used in mempool

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think even if it is only used here, i would prefer to house all Tx interfaces in one place


type Mempool interface {
// Insert attempts to insert a Tx into the app-side mempool returning
// an error upon failure.
Insert(context.Context, sdk.Tx) error

// Insert with a custom gas wanted value
InsertWithGasWanted(context.Context, sdk.Tx, uint64) error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain why this extra method is needed? i think the reason is here:

cosmos-sdk/baseapp/baseapp.go

Line 876 in 4cd29a5

err = app.mempool.InsertWithGasWanted(ctx, tx, gasWanted)
but im not sure i fully understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we need to fill mempoolTx with gasWanted from gas meter when runTx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference between the tx.Gas() vs the gasWanted from the meter here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think of removing this method and instead adding an argument to Insert such as:

type InsertOption struct {
   GasWanted uint64
}

type Mempool interface {
    // ....
    Insert(context.Context, sdk.Tx, InsertOption) error
}

this way, if more is needed in the future, it will be nonbreaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are cases that tx.Gas() is different from gasWanted returned from context GasMeter() and we need to override with it. I think we keep Insert mainly for the tests.

// Select returns an Iterator over the app-side mempool. If txs are specified,
// then they shall be incorporated into the Iterator. The Iterator is not thread-safe to use.
Select(context.Context, [][]byte) Iterator
Expand All @@ -31,7 +50,7 @@ type ExtMempool interface {
Mempool

// SelectBy use callback to iterate over the mempool, it's thread-safe to use.
SelectBy(context.Context, [][]byte, func(sdk.Tx) bool)
SelectBy(context.Context, [][]byte, func(Tx) bool)
}

// Iterator defines an app-side mempool iterator interface that is as minimal as
Expand All @@ -43,7 +62,7 @@ type Iterator interface {
Next() Iterator

// Tx returns the transaction at the current position of the iterator.
Tx() sdk.Tx
Tx() Tx
}
Comment on lines +65 to 66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Iterator.Tx now returns wrapper Tx — breaking change: verify all usages are updated.

Audit for code still asserting sdk.Tx directly from Iterator.Tx().

Run:

Expected: call sites use iter.Tx().Tx (or equivalent) after this change.

🏁 Script executed:

#!/bin/bash
# Find Iterator.Tx() usages and spot unwraps expecting sdk.Tx.
rg -nP --type=go -C3 '\.Tx\(\)\s*[\),]'  # method call contexts
rg -nP --type=go -C3 '\.Tx\(\)\.'        # chained member access implying wrapper usage
rg -nP --type=go -C3 '=\s*iter\.Tx\(\)'  # direct assignment patterns

Length of output: 3411

Iterator.Tx now returns wrapper Tx — update all call sites that still treat it as sdk.Tx

Call sites still passing/assigning iter.Tx() must unwrap (.Tx) or accept the wrapper:

  • types/mempool/mempool.go:83 — for iter != nil && callback(iter.Tx()) {
  • types/mempool/sender_nonce.go:216 — for iter != nil && callback(iter.Tx()) {
  • types/mempool/priority_nonce.go:395 — for iter != nil && callback(iter.Tx()) {

Tests already access the inner Tx in several places (e.g., types/mempool/mempool_test.go:142; types/mempool/priority_nonce_test.go:391, 938, 973) — ensure expectations/ assertions match the wrapper change.

🤖 Prompt for AI Agents 
In types/mempool/mempool.go around lines 65-66 the interface method Tx() now
returns the wrapper type Tx instead of sdk.Tx; update all call sites that still
treat iter.Tx() as sdk.Tx to either unwrap the inner sdk.Tx via iter.Tx.Tx or
change the consumer to accept the wrapper type. Specifically, modify the three
call sites (types/mempool/mempool.go:83, types/mempool/sender_nonce.go:216,
types/mempool/priority_nonce.go:395) to call callback(iter.Tx().Tx) if callback
expects sdk.Tx, or change the callback signature to accept the wrapper; also
update tests that inspect the inner transaction (e.g.,
types/mempool/mempool_test.go:142; types/mempool/priority_nonce_test.go:391,
938, 973) to access the inner sdk.Tx through the wrapper before making
assertions.


var (
Expand All @@ -53,7 +72,7 @@ var (

// SelectBy is compatible with old interface to avoid breaking api.
// In v0.52+, this function is removed and SelectBy is merged into Mempool interface.
func SelectBy(ctx context.Context, mempool Mempool, txs [][]byte, callback func(sdk.Tx) bool) {
func SelectBy(ctx context.Context, mempool Mempool, txs [][]byte, callback func(Tx) bool) {
if ext, ok := mempool.(ExtMempool); ok {
ext.SelectBy(ctx, txs, callback)
return
Expand Down
2 changes: 1 addition & 1 deletion types/mempool/mempool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func fetchTxs(iterator mempool.Iterator, maxBytes int64) []sdk.Tx {
if numBytes += txSize; numBytes > maxBytes {
break
}
txs = append(txs, iterator.Tx())
txs = append(txs, iterator.Tx().Tx)
i := iterator.Next()
iterator = i
}
Expand Down
11 changes: 6 additions & 5 deletions types/mempool/noop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ var _ ExtMempool = (*NoOpMempool)(nil)
// is FIFO-ordered by default.
type NoOpMempool struct{}

func (NoOpMempool) Insert(context.Context, sdk.Tx) error { return nil }
func (NoOpMempool) Select(context.Context, [][]byte) Iterator { return nil }
func (NoOpMempool) SelectBy(context.Context, [][]byte, func(sdk.Tx) bool) {}
func (NoOpMempool) CountTx() int { return 0 }
func (NoOpMempool) Remove(sdk.Tx) error { return nil }
func (NoOpMempool) Insert(context.Context, sdk.Tx) error { return nil }
func (NoOpMempool) InsertWithGasWanted(context.Context, sdk.Tx, uint64) error { return nil }
func (NoOpMempool) Select(context.Context, [][]byte) Iterator { return nil }
func (NoOpMempool) SelectBy(context.Context, [][]byte, func(Tx) bool) {}
func (NoOpMempool) CountTx() int { return 0 }
func (NoOpMempool) Remove(sdk.Tx) error { return nil }
29 changes: 20 additions & 9 deletions types/mempool/priority_nonce.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,10 +189,10 @@ func (mp *PriorityNonceMempool[C]) NextSenderTx(sender string) sdk.Tx {
}

cursor := senderIndex.Front()
return cursor.Value.(sdk.Tx)
return cursor.Value.(Tx).Tx
}

// Insert attempts to insert a Tx into the app-side mempool in O(log n) time,
// InsertWithGasWanted attempts to insert a Tx into the app-side mempool in O(log n) time,
// returning an error if unsuccessful. Sender and nonce are derived from the
// transaction's first signature.
//
Expand All @@ -201,7 +201,7 @@ func (mp *PriorityNonceMempool[C]) NextSenderTx(sender string) sdk.Tx {
//
// Inserting a duplicate tx with a different priority overwrites the existing tx,
// changing the total order of the mempool.
func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error {
func (mp *PriorityNonceMempool[C]) InsertWithGasWanted(ctx context.Context, tx sdk.Tx, gasWanted uint64) error {
mp.mtx.Lock()
defer mp.mtx.Unlock()
if mp.cfg.MaxTx > 0 && mp.priorityIndex.Len() >= mp.cfg.MaxTx {
Expand All @@ -210,6 +210,8 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error
return nil
}

memTx := NewMempoolTx(tx, gasWanted)

sigs, err := mp.cfg.SignerExtractor.GetSigners(tx)
if err != nil {
return err
Expand Down Expand Up @@ -247,12 +249,12 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error
// changes.
sk := txMeta[C]{nonce: nonce, sender: sender}
if oldScore, txExists := mp.scores[sk]; txExists {
if mp.cfg.TxReplacement != nil && !mp.cfg.TxReplacement(oldScore.priority, priority, senderIndex.Get(key).Value.(sdk.Tx), tx) {
if mp.cfg.TxReplacement != nil && !mp.cfg.TxReplacement(oldScore.priority, priority, senderIndex.Get(key).Value.(Tx).Tx, tx) {
return fmt.Errorf(
"tx doesn't fit the replacement rule, oldPriority: %v, newPriority: %v, oldTx: %v, newTx: %v",
oldScore.priority,
priority,
senderIndex.Get(key).Value.(sdk.Tx),
senderIndex.Get(key).Value.(Tx).Tx,
tx,
)
}
Expand All @@ -270,14 +272,23 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error

// Since senderIndex is scored by nonce, a changed priority will overwrite the
// existing key.
key.senderElement = senderIndex.Set(key, tx)
key.senderElement = senderIndex.Set(key, memTx)

mp.scores[sk] = txMeta[C]{priority: priority}
mp.priorityIndex.Set(key, tx)

return nil
}

func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error {
var gasLimit uint64
if gasTx, ok := tx.(GasTx); ok {
gasLimit = gasTx.GetGas()
}

return mp.InsertWithGasWanted(ctx, tx, gasLimit)
}

func (i *PriorityNonceIterator[C]) iteratePriority() Iterator {
// beginning of priority iteration
if i.priorityNode == nil {
Expand Down Expand Up @@ -341,8 +352,8 @@ func (i *PriorityNonceIterator[C]) Next() Iterator {
return i
}

func (i *PriorityNonceIterator[C]) Tx() sdk.Tx {
return i.senderCursors[i.sender].Value.(sdk.Tx)
func (i *PriorityNonceIterator[C]) Tx() Tx {
return i.senderCursors[i.sender].Value.(Tx)
}

// Select returns a set of transactions from the mempool, ordered by priority
Expand Down Expand Up @@ -376,7 +387,7 @@ func (mp *PriorityNonceMempool[C]) doSelect(_ context.Context, _ [][]byte) Itera
}

// SelectBy will hold the mutex during the iteration, callback returns if continue.
func (mp *PriorityNonceMempool[C]) SelectBy(ctx context.Context, txs [][]byte, callback func(sdk.Tx) bool) {
func (mp *PriorityNonceMempool[C]) SelectBy(ctx context.Context, txs [][]byte, callback func(Tx) bool) {
mp.mtx.Lock()
defer mp.mtx.Unlock()

Expand Down
Loading
Loading