From 4cd29a5c9cecae7a40e9614fab61b08bf8e0ea1c Mon Sep 17 00:00:00 2001 From: yihuang Date: Wed, 26 Jun 2024 12:00:48 +0800 Subject: [PATCH 1/7] feat(mempool): respect gas wanted returned by ante handler * Problem: mempool don't respect gas wanted returned by ante handler Solution: - support custom gas wanted in mempool * Update CHANGELOG.md Signed-off-by: yihuang * cleanup * cleanup * fix priorityIndex * fix process proposal * fix lint * fix lint --------- Signed-off-by: yihuang Problem: mempool.GasTx interface is not reused (#536) * Problem: redundant mutex for InsertWithGasWanted cfg of PriorityNonceMempool remains unchanged once assigned, so no lock is required * make mocks * cleanup * keep order of check MaxTx --- CHANGELOG.md | 1 + baseapp/abci_utils.go | 44 +++++++++------------ baseapp/baseapp.go | 12 +++--- baseapp/testutil/mock/mocks.go | 15 +++---- types/mempool/mempool.go | 25 ++++++++++-- types/mempool/mempool_test.go | 2 +- types/mempool/noop.go | 11 +++--- types/mempool/priority_nonce.go | 29 +++++++++----- types/mempool/priority_nonce_test.go | 10 ++--- types/mempool/sender_nonce.go | 25 ++++++++---- types/mempool/sender_nonce_property_test.go | 2 +- 11 files changed, 107 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be9d8730d5da..22383a197a2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index 9c53f0f6550b..f1ac070b5334 100644 --- a/baseapp/abci_utils.go +++ b/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) (sdk.Tx, uint64, error) TxDecode(txBz []byte) (sdk.Tx, error) TxEncode(tx sdk.Tx) ([]byte, error) } @@ -276,7 +276,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.(mempool.GasTx); ok { + txGasLimit = gasTx.GetGas() + } + + stop := h.txSelector.SelectTxForProposal(ctx, uint64(req.MaxTxBytes), maxBlockGas, tx, txBz, txGasLimit) if stop { break } @@ -291,14 +296,14 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan 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 @@ -333,11 +338,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, 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 } @@ -409,17 +414,13 @@ func (h *DefaultProposalHandler) ProcessProposalHandler() sdk.ProcessProposalHan } 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 } @@ -477,7 +478,7 @@ type TxSelector interface { // a proposal based on inclusion criteria defined by the TxSelector. It must // return if the caller should halt the transaction selection loop // (typically over a mempool) or 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 { @@ -502,23 +503,16 @@ 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 + if (gasWanted + ts.totalTxGas) <= maxBlockGas { + ts.totalTxGas += gasWanted ts.totalTxBytes += txSize ts.selectedTxs = append(ts.selectedTxs, txBz) } diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 91ef3e1c8f9d..ff21d3891b76 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -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 } @@ -1067,18 +1067,18 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { // ProcessProposal state internally will be discarded. will be // returned if the transaction cannot be decoded. will be returned if // the transaction is valid, otherwise will be returned. -func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, error) { +func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, uint64, error) { 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) { diff --git a/baseapp/testutil/mock/mocks.go b/baseapp/testutil/mock/mocks.go index 51de61fa7156..7ed7c6fe515c 100644 --- a/baseapp/testutil/mock/mocks.go +++ b/baseapp/testutil/mock/mocks.go @@ -135,12 +135,13 @@ func (mr *MockProposalTxVerifierMockRecorder) PrepareProposalVerifyTx(tx any) *g } // ProcessProposalVerifyTx mocks base method. -func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types.Tx, error) { +func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types.Tx,uint64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ProcessProposalVerifyTx", txBz) ret0, _ := ret[0].(types.Tx) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(uint64) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // ProcessProposalVerifyTx indicates an expected call of ProcessProposalVerifyTx. @@ -216,17 +217,17 @@ func (mr *MockTxSelectorMockRecorder) Clear() *gomock.Call { } // SelectTxForProposal mocks base method. -func (m *MockTxSelector) SelectTxForProposal(ctx context.Context, maxTxBytes, maxBlockGas uint64, memTx types.Tx, txBz []byte) bool { +func (m *MockTxSelector) SelectTxForProposal(ctx context.Context, maxTxBytes, maxBlockGas uint64, memTx types.Tx, txBz []byte, gasWanted uint64) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SelectTxForProposal", ctx, maxTxBytes, maxBlockGas, memTx, txBz) + ret := m.ctrl.Call(m, "SelectTxForProposal", ctx, maxTxBytes, maxBlockGas, memTx, txBz, gasWanted) ret0, _ := ret[0].(bool) return ret0 } // SelectTxForProposal indicates an expected call of SelectTxForProposal. -func (mr *MockTxSelectorMockRecorder) SelectTxForProposal(ctx, maxTxBytes, maxBlockGas, memTx, txBz any) *gomock.Call { +func (mr *MockTxSelectorMockRecorder) SelectTxForProposal(ctx, maxTxBytes, maxBlockGas, memTx, txBz, gasWanted any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SelectTxForProposal", reflect.TypeOf((*MockTxSelector)(nil).SelectTxForProposal), ctx, maxTxBytes, maxBlockGas, memTx, txBz) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SelectTxForProposal", reflect.TypeOf((*MockTxSelector)(nil).SelectTxForProposal), ctx, maxTxBytes, maxBlockGas, memTx, txBz, gasWanted) } // SelectedTxs mocks base method. diff --git a/types/mempool/mempool.go b/types/mempool/mempool.go index 291e0f981f93..73f105f790b6 100644 --- a/types/mempool/mempool.go +++ b/types/mempool/mempool.go @@ -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 +} + 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 + // 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 @@ -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 @@ -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 } var ( @@ -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 diff --git a/types/mempool/mempool_test.go b/types/mempool/mempool_test.go index d84baaebaa68..be18cf329d4e 100644 --- a/types/mempool/mempool_test.go +++ b/types/mempool/mempool_test.go @@ -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 } diff --git a/types/mempool/noop.go b/types/mempool/noop.go index 2dd806b8598e..bcc96a11c273 100644 --- a/types/mempool/noop.go +++ b/types/mempool/noop.go @@ -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 } diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 6fcc6a22290e..621c9c0648b5 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -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. // @@ -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 { @@ -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 @@ -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, ) } @@ -270,7 +272,7 @@ 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) @@ -278,6 +280,15 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error 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 { @@ -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 @@ -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() diff --git a/types/mempool/priority_nonce_test.go b/types/mempool/priority_nonce_test.go index fc515e99cfa5..eb5a0311ea09 100644 --- a/types/mempool/priority_nonce_test.go +++ b/types/mempool/priority_nonce_test.go @@ -388,7 +388,7 @@ func (s *MempoolTestSuite) TestIterator() { // iterate through txs iterator := pool.Select(ctx, nil) for iterator != nil { - tx := iterator.Tx().(testTx) + tx := iterator.Tx().Tx.(testTx) require.Equal(t, tt.txs[tx.id].p, int(tx.priority)) require.Equal(t, tt.txs[tx.id].n, int(tx.nonce)) require.Equal(t, tt.txs[tx.id].a, tx.address) @@ -464,8 +464,8 @@ func (s *MempoolTestSuite) TestIteratorConcurrency() { }() var i int - pool.SelectBy(ctx, nil, func(memTx sdk.Tx) bool { - tx := memTx.(testTx) + pool.SelectBy(ctx, nil, func(memTx mempool.Tx) bool { + tx := memTx.Tx.(testTx) if tx.id < len(tt.txs) { require.Equal(t, tt.txs[tx.id].p, int(tx.priority)) require.Equal(t, tt.txs[tx.id].n, int(tx.nonce)) @@ -935,7 +935,7 @@ func TestNextSenderTx_TxReplacement(t *testing.T) { require.Equal(t, 1, mp.CountTx()) iter := mp.Select(ctx, nil) - require.Equal(t, tx, iter.Tx()) + require.Equal(t, tx, iter.Tx().Tx) } // test Priority with TxReplacement @@ -970,7 +970,7 @@ func TestNextSenderTx_TxReplacement(t *testing.T) { require.Equal(t, 1, mp.CountTx()) iter := mp.Select(ctx, nil) - require.Equal(t, txs[3], iter.Tx()) + require.Equal(t, txs[3], iter.Tx().Tx) } func TestPriorityNonceMempool_UnorderedTx_FailsForSequence(t *testing.T) { diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index b56f698dd142..cead35e84114 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -113,12 +113,12 @@ func (snm *SenderNonceMempool) NextSenderTx(sender string) sdk.Tx { } cursor := senderIndex.Front() - return cursor.Value.(sdk.Tx) + return cursor.Value.(Tx).Tx } -// Insert adds a tx to the mempool. It returns an error if the tx does not have +// InsertWithGasWanted adds a tx to the mempool. It returns an error if the tx does not have // at least one signer. Note, priority is ignored. -func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { +func (snm *SenderNonceMempool) InsertWithGasWanted(_ context.Context, tx sdk.Tx, gasLimit uint64) error { snm.mtx.Lock() defer snm.mtx.Unlock() if snm.maxTx > 0 && len(snm.existingTx) >= snm.maxTx { @@ -128,6 +128,8 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { return nil } + memTx := NewMempoolTx(tx, gasLimit) + sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() if err != nil { return err @@ -149,7 +151,7 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { snm.senders[sender] = senderTxs } - senderTxs.Set(nonce, tx) + senderTxs.Set(nonce, memTx) key := txKey{nonce: nonce, address: sender} snm.existingTx[key] = true @@ -157,6 +159,15 @@ func (snm *SenderNonceMempool) Insert(_ context.Context, tx sdk.Tx) error { return nil } +func (snm *SenderNonceMempool) Insert(ctx context.Context, tx sdk.Tx) error { + var gasLimit uint64 + if gasTx, ok := tx.(GasTx); ok { + gasLimit = gasTx.GetGas() + } + + return snm.InsertWithGasWanted(ctx, tx, gasLimit) +} + // Select returns an iterator ordering transactions in the mempool with the lowest // nonce of a randomly selected sender first. // @@ -197,7 +208,7 @@ func (snm *SenderNonceMempool) doSelect(_ context.Context, _ [][]byte) Iterator } // SelectBy will hold the mutex during the iteration, callback returns if continue. -func (snm *SenderNonceMempool) SelectBy(ctx context.Context, txs [][]byte, callback func(sdk.Tx) bool) { +func (snm *SenderNonceMempool) SelectBy(ctx context.Context, txs [][]byte, callback func(Tx) bool) { snm.mtx.Lock() defer snm.mtx.Unlock() @@ -290,8 +301,8 @@ func (i *senderNonceMempoolIterator) Next() Iterator { return nil } -func (i *senderNonceMempoolIterator) Tx() sdk.Tx { - return i.currentTx.Value.(sdk.Tx) +func (i *senderNonceMempoolIterator) Tx() Tx { + return i.currentTx.Value.(Tx) } func removeAtIndex[T any](slice []T, index int) []T { diff --git a/types/mempool/sender_nonce_property_test.go b/types/mempool/sender_nonce_property_test.go index 58994d48e1f1..892ca5bb312a 100644 --- a/types/mempool/sender_nonce_property_test.go +++ b/types/mempool/sender_nonce_property_test.go @@ -96,7 +96,7 @@ func fetchAllTxs(iterator mempool.Iterator) []testTx { var txs []testTx for iterator != nil { tx := iterator.Tx() - txs = append(txs, tx.(testTx)) + txs = append(txs, tx.Tx.(testTx)) i := iterator.Next() iterator = i } From b54349d2a2d78e68538c86737599d59678b454a0 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 11 Sep 2025 09:35:02 +0800 Subject: [PATCH 2/7] fix mocks --- baseapp/testutil/mock/mocks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/testutil/mock/mocks.go b/baseapp/testutil/mock/mocks.go index 7ed7c6fe515c..1ac2102a0584 100644 --- a/baseapp/testutil/mock/mocks.go +++ b/baseapp/testutil/mock/mocks.go @@ -135,7 +135,7 @@ func (mr *MockProposalTxVerifierMockRecorder) PrepareProposalVerifyTx(tx any) *g } // ProcessProposalVerifyTx mocks base method. -func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types.Tx,uint64, error) { +func (m *MockProposalTxVerifier) ProcessProposalVerifyTx(txBz []byte) (types.Tx, uint64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ProcessProposalVerifyTx", txBz) ret0, _ := ret[0].(types.Tx) From 7be0362bac06259e5b3dd87f0582b631802ebec6 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 12 Sep 2025 08:30:23 +0800 Subject: [PATCH 3/7] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22383a197a2f..3f3940523676 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,7 +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. +* (mempool) [#25338](https://github.com/cosmos/cosmos-sdk/pull/25338) Respect gas wanted returned by ante handler for mempool. ### Bug Fixes From 0936ce1754fbf7c16a5cfa53e75b227f964e210c Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 12 Sep 2025 23:53:04 +0800 Subject: [PATCH 4/7] Apply suggestions from code review --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f3940523676..070f67ab34ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * `x/crisis` * (crypto) [#24414](https://github.com/cosmos/cosmos-sdk/pull/24414) Remove sr25519 support, since it was removed in CometBFT v1.x (see: CometBFT [#3646](https://github.com/cometbft/cometbft/pull/3646)). +### API-BREAKING + +* (mempool) [#25338](https://github.com/cosmos/cosmos-sdk/pull/25338) Respect gas wanted returned by ante handler for mempool. + ### Features * [#24872](https://github.com/cosmos/cosmos-sdk/pull/24872) Support BLS 12-381 for cli `init`, `gentx`, `collect-gentx` @@ -62,7 +66,6 @@ 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 From 99707c4bca20ffda274a5e7fc835301d4c9b56a5 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 23 Sep 2025 09:30:12 +0800 Subject: [PATCH 5/7] Apply suggestions from code review --- baseapp/abci_utils.go | 2 +- baseapp/baseapp.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index f1ac070b5334..c08a27bea3c8 100644 --- a/baseapp/abci_utils.go +++ b/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, uint64, error) + ProcessProposalVerifyTx(txBz []byte) (msg sdk.Tx, gasWanted uint64, err error) TxDecode(txBz []byte) (sdk.Tx, error) TxEncode(tx sdk.Tx) ([]byte, error) } diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index ff21d3891b76..c81fb4e4b107 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -1067,7 +1067,7 @@ func (app *BaseApp) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { // ProcessProposal state internally will be discarded. will be // returned if the transaction cannot be decoded. will be returned if // the transaction is valid, otherwise will be returned. -func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (sdk.Tx, uint64, error) { +func (app *BaseApp) ProcessProposalVerifyTx(txBz []byte) (msg sdk.Tx, gasWanted uint64, err error) { tx, err := app.txDecoder(txBz) if err != nil { return nil, 0, err From c434a7640a7aafa5b747eab23af5d36a91dad2ba Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 24 Sep 2025 09:00:07 +0800 Subject: [PATCH 6/7] InsertWithOption --- baseapp/baseapp.go | 2 +- types/mempool/mempool.go | 8 ++++++-- types/mempool/noop.go | 12 ++++++------ types/mempool/priority_nonce.go | 9 ++++----- types/mempool/sender_nonce.go | 8 ++++---- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index c81fb4e4b107..ec828e5c5a7d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -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.InsertWithGasWanted(ctx, tx, gasWanted) + err = app.mempool.InsertWithOption(ctx, tx, mempool.InsertOption{GasWanted: gasWanted}) if err != nil { return gInfo, nil, anteEvents, err } diff --git a/types/mempool/mempool.go b/types/mempool/mempool.go index 73f105f790b6..3692f970568b 100644 --- a/types/mempool/mempool.go +++ b/types/mempool/mempool.go @@ -23,13 +23,17 @@ type GasTx interface { GetGas() uint64 } +type InsertOption struct { + GasWanted uint64 +} + 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 + // InsertWithOption with a custom option, e.g. gas wanted. + InsertWithOption(context.Context, sdk.Tx, InsertOption) error // 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. diff --git a/types/mempool/noop.go b/types/mempool/noop.go index bcc96a11c273..ff7f100c9210 100644 --- a/types/mempool/noop.go +++ b/types/mempool/noop.go @@ -16,9 +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) 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 } +func (NoOpMempool) Insert(context.Context, sdk.Tx) error { return nil } +func (NoOpMempool) InsertWithOption(context.Context, sdk.Tx, InsertOption) 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 } diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 621c9c0648b5..646a87e01faf 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -192,7 +192,7 @@ func (mp *PriorityNonceMempool[C]) NextSenderTx(sender string) sdk.Tx { return cursor.Value.(Tx).Tx } -// InsertWithGasWanted attempts to insert a Tx into the app-side mempool in O(log n) time, +// InsertWithOption 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. // @@ -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]) InsertWithGasWanted(ctx context.Context, tx sdk.Tx, gasWanted uint64) error { +func (mp *PriorityNonceMempool[C]) InsertWithOption(ctx context.Context, tx sdk.Tx, option InsertOption) error { mp.mtx.Lock() defer mp.mtx.Unlock() if mp.cfg.MaxTx > 0 && mp.priorityIndex.Len() >= mp.cfg.MaxTx { @@ -209,8 +209,7 @@ func (mp *PriorityNonceMempool[C]) InsertWithGasWanted(ctx context.Context, tx s } else if mp.cfg.MaxTx < 0 { return nil } - - memTx := NewMempoolTx(tx, gasWanted) + memTx := NewMempoolTx(tx, option.GasWanted) sigs, err := mp.cfg.SignerExtractor.GetSigners(tx) if err != nil { @@ -286,7 +285,7 @@ func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error gasLimit = gasTx.GetGas() } - return mp.InsertWithGasWanted(ctx, tx, gasLimit) + return mp.InsertWithOption(ctx, tx, InsertOption{GasWanted: gasLimit}) } func (i *PriorityNonceIterator[C]) iteratePriority() Iterator { diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index cead35e84114..0f7b960778a5 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -116,9 +116,9 @@ func (snm *SenderNonceMempool) NextSenderTx(sender string) sdk.Tx { return cursor.Value.(Tx).Tx } -// InsertWithGasWanted adds a tx to the mempool. It returns an error if the tx does not have +// InsertWithOption adds a tx to the mempool. It returns an error if the tx does not have // at least one signer. Note, priority is ignored. -func (snm *SenderNonceMempool) InsertWithGasWanted(_ context.Context, tx sdk.Tx, gasLimit uint64) error { +func (snm *SenderNonceMempool) InsertWithOption(_ context.Context, tx sdk.Tx, option InsertOption) error { snm.mtx.Lock() defer snm.mtx.Unlock() if snm.maxTx > 0 && len(snm.existingTx) >= snm.maxTx { @@ -128,7 +128,7 @@ func (snm *SenderNonceMempool) InsertWithGasWanted(_ context.Context, tx sdk.Tx, return nil } - memTx := NewMempoolTx(tx, gasLimit) + memTx := NewMempoolTx(tx, option.GasWanted) sigs, err := tx.(signing.SigVerifiableTx).GetSignaturesV2() if err != nil { @@ -165,7 +165,7 @@ func (snm *SenderNonceMempool) Insert(ctx context.Context, tx sdk.Tx) error { gasLimit = gasTx.GetGas() } - return snm.InsertWithGasWanted(ctx, tx, gasLimit) + return snm.InsertWithOption(ctx, tx, InsertOption{GasWanted: gasLimit}) } // Select returns an iterator ordering transactions in the mempool with the lowest From 010dfe183c9db82a629f1b4315e62dfb54a46602 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 24 Sep 2025 09:00:20 +0800 Subject: [PATCH 7/7] mv GasTx --- baseapp/abci_utils.go | 2 +- types/mempool/mempool.go | 4 ---- types/mempool/priority_nonce.go | 2 +- types/mempool/sender_nonce.go | 2 +- types/tx_msg.go | 5 +++++ 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/baseapp/abci_utils.go b/baseapp/abci_utils.go index c08a27bea3c8..7c6f4d2dbba8 100644 --- a/baseapp/abci_utils.go +++ b/baseapp/abci_utils.go @@ -277,7 +277,7 @@ func (h *DefaultProposalHandler) PrepareProposalHandler() sdk.PrepareProposalHan } var txGasLimit uint64 - if gasTx, ok := tx.(mempool.GasTx); ok { + if gasTx, ok := tx.(sdk.GasTx); ok { txGasLimit = gasTx.GetGas() } diff --git a/types/mempool/mempool.go b/types/mempool/mempool.go index 3692f970568b..6a3e1ba975a7 100644 --- a/types/mempool/mempool.go +++ b/types/mempool/mempool.go @@ -19,10 +19,6 @@ func NewMempoolTx(tx sdk.Tx, gasWanted uint64) Tx { } } -type GasTx interface { - GetGas() uint64 -} - type InsertOption struct { GasWanted uint64 } diff --git a/types/mempool/priority_nonce.go b/types/mempool/priority_nonce.go index 646a87e01faf..4deb69156e57 100644 --- a/types/mempool/priority_nonce.go +++ b/types/mempool/priority_nonce.go @@ -281,7 +281,7 @@ func (mp *PriorityNonceMempool[C]) InsertWithOption(ctx context.Context, tx sdk. func (mp *PriorityNonceMempool[C]) Insert(ctx context.Context, tx sdk.Tx) error { var gasLimit uint64 - if gasTx, ok := tx.(GasTx); ok { + if gasTx, ok := tx.(sdk.GasTx); ok { gasLimit = gasTx.GetGas() } diff --git a/types/mempool/sender_nonce.go b/types/mempool/sender_nonce.go index 0f7b960778a5..33efba0a0d16 100644 --- a/types/mempool/sender_nonce.go +++ b/types/mempool/sender_nonce.go @@ -161,7 +161,7 @@ func (snm *SenderNonceMempool) InsertWithOption(_ context.Context, tx sdk.Tx, op func (snm *SenderNonceMempool) Insert(ctx context.Context, tx sdk.Tx) error { var gasLimit uint64 - if gasTx, ok := tx.(GasTx); ok { + if gasTx, ok := tx.(sdk.GasTx); ok { gasLimit = gasTx.GetGas() } diff --git a/types/tx_msg.go b/types/tx_msg.go index c32c23b1bfb2..94b30e60bd7e 100644 --- a/types/tx_msg.go +++ b/types/tx_msg.go @@ -106,6 +106,11 @@ type ( // doesn't require access to any other information. ValidateBasic() error } + + // GasTx defines an interface for a transaction that contains gas wanted for mempool + GasTx interface { + GetGas() uint64 + } ) // TxDecoder unmarshals transaction bytes