From 0ef1d37c896361683c78f48c1984d365df40d1c3 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Mon, 24 Aug 2026 10:32:57 +0800 Subject: [PATCH] fix: harden mempool proposal gas accounting --- sei-tendermint/internal/mempool/mempool.go | 59 ++++++---- .../internal/mempool/mempool_test.go | 58 ++++++++++ .../internal/mempool/reactor/reactor_test.go | 3 + sei-tendermint/internal/mempool/tx.go | 56 ++++++--- sei-tendermint/internal/mempool/tx_test.go | 107 ++++++++++++++---- sei-tendermint/internal/mempool/types.go | 4 + sei-tendermint/internal/state/tx_filter.go | 3 +- .../internal/state/tx_filter_test.go | 2 + 8 files changed, 233 insertions(+), 59 deletions(-) diff --git a/sei-tendermint/internal/mempool/mempool.go b/sei-tendermint/internal/mempool/mempool.go index 5242f3060c..d8fe1a11e0 100644 --- a/sei-tendermint/internal/mempool/mempool.go +++ b/sei-tendermint/internal/mempool/mempool.go @@ -33,6 +33,26 @@ const ( MinGasEVMTx = 21000 ) +type checkedTxMetadata struct { + priority int64 + gasWanted int64 + estimatedGas int64 +} + +// metadataFromCheckTx normalizes application metadata identically for initial checks and +// rechecks. Reusing this step prevents proposal selection from retaining stale gas values. +func metadataFromCheckTx(res *abci.ResponseCheckTxV2) checkedTxMetadata { + estimatedGas := res.GasEstimated + if estimatedGas < MinGasEVMTx || estimatedGas > res.GasWanted { + estimatedGas = res.GasWanted + } + return checkedTxMetadata{ + priority: res.Priority, + gasWanted: res.GasWanted, + estimatedGas: estimatedGas, + } +} + type Config struct { // Maximum number of transactions in the mempool Size int @@ -353,18 +373,14 @@ func (txmp *TxMempool) CheckTx(ctx context.Context, tx types.Tx) (*abci.Response Global.NumberOfSuccessfulCheckTxsAt().Add(1) Global.observeCheckTxPriorityDistribution(res.Priority, false, "", false) - // Normalize the estimate. - estimatedGas := res.GasEstimated - if estimatedGas < MinGasEVMTx || estimatedGas > res.GasWanted { - estimatedGas = res.GasWanted - } + metadata := metadataFromCheckTx(res) wtx := &WrappedTx{ hashedTx: hTx, timestamp: time.Now().UTC(), height: txmp.height, - priority: res.Priority, - estimatedGas: estimatedGas, - gasWanted: res.GasWanted, + priority: metadata.priority, + estimatedGas: metadata.estimatedGas, + gasWanted: metadata.gasWanted, } if res.IsEVM { wtx.evm = utils.Some(evmTx{ @@ -415,11 +431,13 @@ func (txmp *TxMempool) Flush() { // The returned list starts with EVM transactions (in priority order), // followed by non-EVM transactions (in priority order). // There are 4 types of constraints. -// 1. maxBytes - stops pulling txs from mempool once maxBytes is hit. -// 2. maxGasWanted - stops pulling txs from mempool once total gas wanted exceeds maxGasWanted. +// 1. maxTxs - limits the number of transactions returned from the mempool. +// 2. maxBytes - limits the total transaction bytes returned from the mempool. +// 3. maxGasWanted - limits total gas wanted; candidates that do not fit are skipped. // Can be set to -1 to be ignored. -// 3. maxGasEstimated - similar to maxGasWanted but will use the estimated gas used for EVM txs -// while still using gas wanted for cosmos txs. Can be set to -1 to be ignored. +// 4. maxGasEstimated - similar to maxGasWanted but will use the estimated gas used for EVM txs +// while still using gas wanted for cosmos txs. Candidates that do not fit are skipped. Can be +// set to -1 to be ignored. // // NOTE: Transactions are removed from the mempool iff remove == true. // Either way, the transactions stay in the LRU cache. @@ -461,7 +479,7 @@ func (txmp *TxMempool) Update( for i, tx := range blockTxs { txResults[tx.Hash()] = execTxResult[i].Code == abci.CodeTypeOK } - newPriorities := map[types.TxHash]int64{} + recheckedTxs := map[types.TxHash]checkedTxMetadata{} invalidTxs := map[types.TxHash]bool{} if recheck { for _, wtx := range txmp.txStore.ReadyTxs() { @@ -476,18 +494,17 @@ func (txmp *TxMempool) Update( if err != nil || !res.IsOK() { invalidTxs[wtx.Hash()] = true } else { - // If succeeds, we just care about the new priority. - newPriorities[wtx.Hash()] = res.Priority + recheckedTxs[wtx.Hash()] = metadataFromCheckTx(res) } } } txmp.txStore.Update(updateSpec{ - Now: time.Now(), - Height: blockHeight, - TxResults: txResults, - NewPriorities: newPriorities, - InvalidTxs: invalidTxs, - Constraints: txConstraints, + Now: time.Now(), + Height: blockHeight, + TxResults: txResults, + RecheckedTxs: recheckedTxs, + InvalidTxs: invalidTxs, + Constraints: txConstraints, }) txmp.notifyTxsAvailable() Global.SizeAt().Set(int64(txmp.NumTxsNotPending())) diff --git a/sei-tendermint/internal/mempool/mempool_test.go b/sei-tendermint/internal/mempool/mempool_test.go index d5af52969f..3150cdc512 100644 --- a/sei-tendermint/internal/mempool/mempool_test.go +++ b/sei-tendermint/internal/mempool/mempool_test.go @@ -454,6 +454,64 @@ func TestTxMempool_ReapMaxBytesMaxGas_FallbackToGasWanted(t *testing.T) { wg.Wait() } +func TestTxMempool_RejectsGasWantedAboveProposalLimit(t *testing.T) { + gasWanted := int64(11) + client := &application{Application: kvstore.NewApplication(), gasWanted: &gasWanted} + constraints := NopTxConstraints() + constraints.MaxGasWanted = 10 + txmp := setup(TestConfig(), proxy.New(client), func() (TxConstraints, error) { + return constraints, nil + }) + + _, err := txmp.CheckTx(t.Context(), types.Tx("sender=key=1")) + require.Error(t, err) + require.Zero(t, txmp.Size()) +} + +func TestTxMempool_ZeroProposalGasLimitDoesNotRejectAtAdmission(t *testing.T) { + gasWanted := int64(1) + client := &application{Application: kvstore.NewApplication(), gasWanted: &gasWanted} + constraints := NopTxConstraints() + constraints.MaxGasWanted = 0 + txmp := setup(TestConfig(), proxy.New(client), func() (TxConstraints, error) { + return constraints, nil + }) + + _, err := txmp.CheckTx(t.Context(), types.Tx("sender=key=1")) + require.NoError(t, err) + require.Equal(t, 1, txmp.Size()) + reaped, _ := txmp.ReapTxs(ReapLimits{MaxGasWanted: utils.Some(int64(0))}, false) + require.Empty(t, reaped) +} + +func TestTxMempool_RecheckRefreshesGasMetadata(t *testing.T) { + gasWanted := int64(1) + gasEstimated := int64(1) + client := &application{ + Application: kvstore.NewApplication(), + gasWanted: &gasWanted, + gasEstimated: &gasEstimated, + } + txmp := setup(TestConfig(), proxy.New(client), NopTxConstraintsFetcher) + tx := types.Tx("sender=key=1") + + _, err := txmp.CheckTx(t.Context(), tx) + require.NoError(t, err) + reaped, _ := txmp.ReapTxs(ReapLimits{MaxGasWanted: utils.Some(int64(1))}, false) + require.Equal(t, types.Txs{tx}, reaped) + + gasWanted = 9 + gasEstimated = 9 + txmp.Lock() + require.NoError(t, txmp.Update(t.Context(), 1, nil, nil, NopTxConstraints(), true)) + txmp.Unlock() + + reaped, _ = txmp.ReapTxs(ReapLimits{MaxGasWanted: utils.Some(int64(8))}, false) + require.Empty(t, reaped) + reaped, _ = txmp.ReapTxs(ReapLimits{MaxGasWanted: utils.Some(int64(9))}, false) + require.Equal(t, types.Txs{tx}, reaped) +} + func TestTxMempool_ReapMaxTxs(t *testing.T) { ctx := t.Context() diff --git a/sei-tendermint/internal/mempool/reactor/reactor_test.go b/sei-tendermint/internal/mempool/reactor/reactor_test.go index f587cf919e..84cfac75bf 100644 --- a/sei-tendermint/internal/mempool/reactor/reactor_test.go +++ b/sei-tendermint/internal/mempool/reactor/reactor_test.go @@ -224,6 +224,7 @@ func TestReactorFailedCheckTxCountEvictsPeer(t *testing.T) { return mempool.TxConstraints{ MaxDataBytes: 10, MaxGas: -1, + MaxGasWanted: -1, }, nil }) t.Cleanup(leaktest.Check(t)) @@ -275,6 +276,7 @@ func TestReactorPeerDownClearsFailedCheckTxCount(t *testing.T) { return mempool.TxConstraints{ MaxDataBytes: 10, MaxGas: -1, + MaxGasWanted: -1, }, nil }, ) @@ -314,6 +316,7 @@ func TestReactorMissingFailedCheckTxCountIsNotRecreated(t *testing.T) { return mempool.TxConstraints{ MaxDataBytes: 10, MaxGas: -1, + MaxGasWanted: -1, }, nil }, ) diff --git a/sei-tendermint/internal/mempool/tx.go b/sei-tendermint/internal/mempool/tx.go index 5a728bde6e..d6ea12c5ab 100644 --- a/sei-tendermint/internal/mempool/tx.go +++ b/sei-tendermint/internal/mempool/tx.go @@ -66,6 +66,9 @@ func (wtx *WrappedTx) check(c TxConstraints) error { if c.MaxGas >= 0 && wtx.gasWanted > c.MaxGas { return fmt.Errorf("gas wanted exceeds max gas: gas wanted %d is greater than max gas %d", wtx.gasWanted, c.MaxGas) } + if c.MaxGasWanted > 0 && wtx.gasWanted > c.MaxGasWanted { + return fmt.Errorf("gas wanted exceeds max gas wanted: gas wanted %d is greater than max gas wanted %d", wtx.gasWanted, c.MaxGasWanted) + } return nil } @@ -550,12 +553,12 @@ func (s *txStore) compact(inner *txStoreInner, clearAccounts bool) { } type updateSpec struct { - Now time.Time - Height int64 - TxResults map[types.TxHash]bool // true - success, false - failed, missing - not executed - Constraints TxConstraints - NewPriorities map[types.TxHash]int64 - InvalidTxs map[types.TxHash]bool + Now time.Time + Height int64 + TxResults map[types.TxHash]bool // true - success, false - failed, missing - not executed + Constraints TxConstraints + RecheckedTxs map[types.TxHash]checkedTxMetadata + InvalidTxs map[types.TxHash]bool } func (s *txStore) Update(spec updateSpec) { @@ -592,6 +595,11 @@ func (s *txStore) Update(spec updateSpec) { if expired { Global.ExpiredTxsAt().Add(1) } + if metadata, ok := spec.RecheckedTxs[wtx.Hash()]; ok { + wtx.priority = metadata.priority + wtx.gasWanted = metadata.gasWanted + wtx.estimatedGas = metadata.estimatedGas + } invalid := spec.InvalidTxs[wtx.Hash()] || wtx.check(spec.Constraints) != nil _, executed := spec.TxResults[wtx.Hash()] remove := invalid || executed || (expired && (s.config.RemoveExpiredTxsFromQueue || !inner.isReady(wtx))) @@ -606,8 +614,6 @@ func (s *txStore) Update(spec updateSpec) { if el, ok := wtx.readyEl.Get(); ok { s.readyTxs.Remove(el) } - } else if newPriority, ok := spec.NewPriorities[wtx.Hash()]; ok { - wtx.priority = newPriority } } start := time.Now() @@ -624,6 +630,14 @@ type ReapLimits struct { MaxGasEstimated utils.Option[int64] } +// fitsReapCapacity reports whether a transaction fits every remaining proposal budget. +// The budgets are independent, so a candidate must fit all three to be selectable. +func (wtx *WrappedTx) fitsReapCapacity(remainingBytes, remainingGasWanted, remainingGasEstimated int64) bool { + return wtx.protoSize <= remainingBytes && + wtx.gasWanted <= remainingGasWanted && + wtx.estimatedGas <= remainingGasEstimated +} + // Reap returns a list of transactions within the provided tx, // byte, and gas constraints together with the total estimated gas for the // returned transactions. Reaped txs are removed iff remove == true. @@ -649,18 +663,32 @@ func (s *txStore) Reap(l ReapLimits, remove bool) (types.Txs, int64) { var wtxs []*WrappedTx for inner := range s.inner.Lock() { if uint64(inner.state.Load().ready.count) >= s.config.TxNotifyThreshold { //nolint:gosec // count is non-negative + blockedEVMAccounts := map[common.Address]struct{}{} for _, wtx := range inner.inInclusionOrder() { - if uint64(len(wtxs)) >= maxTxs || !inner.isReady(wtx) { + if uint64(len(wtxs)) >= maxTxs { break } - if maxBytes-totalSize < wtx.protoSize { + if !inner.isReady(wtx) { break } - if maxGasWanted-totalGasWanted < wtx.gasWanted { - break + evm, isEVM := wtx.evm.Get() + // Non-EVM ordering assumes transactions come from different accounts. If that + // assumption is violated, skipping sequence N can select N+1 and fail delivery. + if isEVM { + if _, blocked := blockedEVMAccounts[evm.address]; blocked { + continue + } } - if maxGasEstimated-totalGasEstimated < wtx.estimatedGas { - break + if !wtx.fitsReapCapacity( + maxBytes-totalSize, + maxGasWanted-totalGasWanted, + maxGasEstimated-totalGasEstimated, + ) { + if isEVM { + // A later nonce cannot be selected when this account's current nonce is skipped. + blockedEVMAccounts[evm.address] = struct{}{} + } + continue } // include tx and update totals totalSize += wtx.protoSize diff --git a/sei-tendermint/internal/mempool/tx_test.go b/sei-tendermint/internal/mempool/tx_test.go index 79d7c87bd6..07a0eb0259 100644 --- a/sei-tendermint/internal/mempool/tx_test.go +++ b/sei-tendermint/internal/mempool/tx_test.go @@ -291,6 +291,67 @@ func TestTxStore_Size(t *testing.T) { require.Equal(t, numTxs, txStore.State().total.count) } +func TestTxStore_ReapSkipsUnfittableTransaction(t *testing.T) { + txStore := newTxStoreForTest() + tooLarge := &WrappedTx{ + hashedTx: newHashedTx(types.Tx("too-large")), + priority: 3, + gasWanted: 11, + estimatedGas: 11, + timestamp: time.Now(), + } + firstFit := &WrappedTx{ + hashedTx: newHashedTx(types.Tx("first-fit")), + priority: 2, + gasWanted: 6, + estimatedGas: 6, + timestamp: time.Now(), + } + secondFit := &WrappedTx{ + hashedTx: newHashedTx(types.Tx("second-fit")), + priority: 1, + gasWanted: 4, + estimatedGas: 4, + timestamp: time.Now(), + } + for _, tx := range []*WrappedTx{tooLarge, firstFit, secondFit} { + require.NoError(t, txStore.Insert(tx)) + } + + reaped, _ := txStore.Reap(ReapLimits{ + MaxGasWanted: utils.Some(int64(10)), + MaxGasEstimated: utils.Some(int64(10)), + }, false) + require.Equal(t, types.Txs{firstFit.Tx(), secondFit.Tx()}, reaped) +} + +func TestTxStore_ReapDoesNotSkipEVMNonceDependency(t *testing.T) { + rng := utils.TestRng() + app := newEVMNonceApp() + txStore := NewTxStore(TestConfig(), proxy.New(app)) + blockedAddress := genEvmAddress(rng) + selectedAddress := genEvmAddress(rng) + app.setBalance(blockedAddress, 1) + app.setBalance(selectedAddress, 1) + + blockedHead := makeEvmTxForTest(rng, blockedAddress, 0, 3, 0) + blockedHead.gasWanted = 11 + blockedHead.estimatedGas = 11 + blockedTail := makeEvmTxForTest(rng, blockedAddress, 1, 2, 0) + selected := makeEvmTxForTest(rng, selectedAddress, 0, 1, 0) + selected.gasWanted = 5 + selected.estimatedGas = 5 + for _, tx := range []*WrappedTx{blockedHead, blockedTail, selected} { + require.NoError(t, txStore.Insert(tx)) + } + + reaped, _ := txStore.Reap(ReapLimits{ + MaxGasWanted: utils.Some(int64(10)), + MaxGasEstimated: utils.Some(int64(10)), + }, false) + require.Equal(t, types.Txs{selected.Tx()}, reaped) +} + func TestTxStore_RejectsAndEvictsTransactionsBelowAccountNonce(t *testing.T) { rng := utils.TestRng() app := newEVMNonceApp() @@ -353,11 +414,11 @@ func TestTxStore_RejectsAndEvictsTransactionsBelowAccountNonce(t *testing.T) { } txStore.Update(updateSpec{ - Now: time.Now(), - Height: height + 1, - TxResults: map[types.TxHash]bool{}, - Constraints: NopTxConstraints(), - NewPriorities: map[types.TxHash]int64{}, + Now: time.Now(), + Height: height + 1, + TxResults: map[types.TxHash]bool{}, + Constraints: NopTxConstraints(), + RecheckedTxs: map[types.TxHash]checkedTxMetadata{}, }) for txHash, wtx := range env.byHash { @@ -420,9 +481,9 @@ func testTxStoreUpdateExpiresTransactions(t *testing.T, removeExpiredTxsFromQueu env.markReadyTxs() updates := []updateSpec{ - {Now: baseTime.Add(16 * time.Second), Height: 14, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), NewPriorities: map[types.TxHash]int64{}}, - {Now: baseTime.Add(24 * time.Second), Height: 22, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), NewPriorities: map[types.TxHash]int64{}}, - {Now: baseTime.Add(36 * time.Second), Height: 34, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), NewPriorities: map[types.TxHash]int64{}}, + {Now: baseTime.Add(16 * time.Second), Height: 14, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), RecheckedTxs: map[types.TxHash]checkedTxMetadata{}}, + {Now: baseTime.Add(24 * time.Second), Height: 22, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), RecheckedTxs: map[types.TxHash]checkedTxMetadata{}}, + {Now: baseTime.Add(36 * time.Second), Height: 34, TxResults: map[types.TxHash]bool{}, Constraints: NopTxConstraints(), RecheckedTxs: map[types.TxHash]checkedTxMetadata{}}, } for _, update := range updates { @@ -565,11 +626,11 @@ func TestTxStore_ExpiredTxCacheBehavior(t *testing.T) { require.NoError(t, txStore.Insert(pending)) txStore.Update(updateSpec{ - Now: time.Unix(102, 0), - Height: 1, - TxResults: map[types.TxHash]bool{}, - Constraints: NopTxConstraints(), - NewPriorities: map[types.TxHash]int64{}, + Now: time.Unix(102, 0), + Height: 1, + TxResults: map[types.TxHash]bool{}, + Constraints: NopTxConstraints(), + RecheckedTxs: map[types.TxHash]checkedTxMetadata{}, }) _, readyPresent := txStore.ByHash(ready.Hash()) @@ -601,11 +662,11 @@ func TestTxStore_NoncePrunedTxsRejectedAsOldNonce(t *testing.T) { env.app.setNonce(address, 9) txStore.Update(updateSpec{ - Now: time.Now(), - Height: 1, - TxResults: map[types.TxHash]bool{}, - Constraints: NopTxConstraints(), - NewPriorities: map[types.TxHash]int64{}, + Now: time.Now(), + Height: 1, + TxResults: map[types.TxHash]bool{}, + Constraints: NopTxConstraints(), + RecheckedTxs: map[types.TxHash]checkedTxMetadata{}, }) _, readyPresent := txStore.ByHash(prunedReady.Hash()) @@ -706,11 +767,11 @@ func TestTxStore_ReplacesReadyThenPendingTxByHigherPriority(t *testing.T) { env.app.setBalance(address, 50) env.txStore.Update(updateSpec{ - Now: time.Now(), - Height: 1, - TxResults: map[types.TxHash]bool{}, - Constraints: NopTxConstraints(), - NewPriorities: map[types.TxHash]int64{}, + Now: time.Now(), + Height: 1, + TxResults: map[types.TxHash]bool{}, + Constraints: NopTxConstraints(), + RecheckedTxs: map[types.TxHash]checkedTxMetadata{}, }) env.assertState(t) diff --git a/sei-tendermint/internal/mempool/types.go b/sei-tendermint/internal/mempool/types.go index 8cc85d6dcb..e1a58e47bb 100644 --- a/sei-tendermint/internal/mempool/types.go +++ b/sei-tendermint/internal/mempool/types.go @@ -7,6 +7,9 @@ import "math" type TxConstraints struct { MaxDataBytes int64 MaxGas int64 + // MaxGasWanted uses -1 for unlimited. Zero remains accepted for backward + // compatibility and yields empty proposals. + MaxGasWanted int64 } // TxConstraintsFetcher returns the precomputed consensus-derived mempool limits for the current @@ -17,6 +20,7 @@ func NopTxConstraints() TxConstraints { return TxConstraints{ MaxDataBytes: math.MaxInt64, MaxGas: -1, + MaxGasWanted: -1, } } diff --git a/sei-tendermint/internal/state/tx_filter.go b/sei-tendermint/internal/state/tx_filter.go index 01ee35d411..4ca49d017b 100644 --- a/sei-tendermint/internal/state/tx_filter.go +++ b/sei-tendermint/internal/state/tx_filter.go @@ -58,6 +58,7 @@ func TxConstraintsForState(state State) mempool.TxConstraints { state.ConsensusParams.Block.MaxBytes, state.Validators.Size(), ), - MaxGas: state.ConsensusParams.Block.MaxGas, + MaxGas: state.ConsensusParams.Block.MaxGas, + MaxGasWanted: state.ConsensusParams.Block.MaxGasWanted, } } diff --git a/sei-tendermint/internal/state/tx_filter_test.go b/sei-tendermint/internal/state/tx_filter_test.go index 9a37d0fc3a..b843176cb5 100644 --- a/sei-tendermint/internal/state/tx_filter_test.go +++ b/sei-tendermint/internal/state/tx_filter_test.go @@ -14,6 +14,7 @@ import ( func TestTxFilter(t *testing.T) { genDoc := randomGenesisDoc() genDoc.ConsensusParams.Block.MaxBytes = 3000 + genDoc.ConsensusParams.Block.MaxGasWanted = 1234 genDoc.ConsensusParams.Evidence.MaxBytes = 1500 // Max size of Txs is much smaller than size of block, @@ -33,6 +34,7 @@ func TestTxFilter(t *testing.T) { constraints := sm.TxConstraintsForState(state) require.NoError(t, err) + assert.Equal(t, int64(1234), constraints.MaxGasWanted) txSize := types.ComputeProtoSizeForTxs([]types.Tx{tc.tx}) if tc.isErr { assert.Greater(t, txSize, constraints.MaxDataBytes, "#%v", i)