From a490e35a9994ca23821b499f3702b6f52147630c Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 13 Jul 2026 15:54:57 +0800 Subject: [PATCH 01/22] fix: prevent cosmos mempool proposal starvation under backlog * carry rechecked cosmos snapshot across heights instead of reset in each block * guard re-adds with a committed-nonce watermark pruned synchronously at FinalizeBlock * serve last completed snapshot when recheck loop falls behind a proposal --- CHANGELOG.md | 1 + mempool/internal/heightsync/heightsync.go | 53 +++++- .../internal/heightsync/heightsync_test.go | 54 ++++++ mempool/mempool.go | 9 +- mempool/recheck_pool.go | 25 ++- mempool/recheck_pool_test.go | 61 +++++++ mempool/tx_store.go | 164 ++++++++++++++++-- mempool/tx_store_test.go | 122 +++++++++++++ server/server_app_options.go | 4 +- 9 files changed, 466 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 203076e27..23148fcd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve last completed snapshot when recheck loop falls behind a proposal. ## v0.6.0 diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index 7e83b5fc1..012cd51d4 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -116,6 +116,13 @@ type HeightSync[Store any] struct { // fields of the Store itself mu sync.RWMutex + // staleFallback makes GetStore return the most recent completed Store + // (instead of nil) when it times out while still behind the target height. + // Only enable it for stores that stay free of state a committed block + // invalidated (see the cosmos pool's committed-nonce watermark), otherwise + // a stale Store could serve already-committed txs. + staleFallback bool + logger log.Logger } @@ -134,9 +141,26 @@ func New[Store any](startHeight *big.Int, reset func(logger log.Logger) *Store, return hs } +// WithStaleFallback enables the stale fallback (see the staleFallback field) +// and returns hs for chaining at construction. +func (hs *HeightSync[Store]) WithStaleFallback() *HeightSync[Store] { + hs.staleFallback = true + return hs +} + // StartNewHeight resets the HeightSync for a new height, overwriting the // previous Store with a fresh Store via the reset fn. func (hs *HeightSync[Store]) StartNewHeight(height *big.Int) { + hs.StartNewHeightFrom(height, nil) +} + +// StartNewHeightFrom starts a new height whose Store is derived from the +// previous height's Store via carry (nil carry means a fresh Store, as in +// StartNewHeight). Carrying lets a producer keep validated state across +// heights, so a pass cancelled before completion does not discard everything +// the previous height validated. carry runs while the HeightSync write lock is +// held and must not call back into the HeightSync. +func (hs *HeightSync[Store]) StartNewHeightFrom(height *big.Int, carry func(prev *Store) *Store) { hs.mu.Lock() defer hs.mu.Unlock() @@ -146,9 +170,13 @@ func (hs *HeightSync[Store]) StartNewHeight(height *big.Int) { panic(fmt.Errorf("height %s not ended before starting new height %s", hs.currentHeight.String(), height.String())) } - // create new Store for this height + // create the Store for this height, either fresh or carried forward hs.currentHeight = new(big.Int).Set(height) - hs.store = hs.reset(hs.logger) + if carry != nil { + hs.store = carry(hs.store) + } else { + hs.store = hs.reset(hs.logger) + } // close old channel and create new one to wake up consumers oldChan := hs.heightChanged @@ -237,8 +265,7 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto } // current height is behind target, we cannot return the Store at the - // current height, so we must wait for the height to advance to the - // callers target height + // target height yet, so we wait for the height to advance. heightChangedChan := hs.heightChanged hs.mu.RUnlock() @@ -250,10 +277,22 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto // height changed, loop back to check if we've reached target continue case <-ctx.Done(): - // caller is done waiting, but we still do not have a Store at the - // correct height to return, return nil instead + // The caller is done waiting and the height sync never reached the + // target height. hsTimeout.Add(ctx, 1) - return nil + if !hs.staleFallback { + // caller opted out of stale results, return nil + return nil + } + // Rather than starve the caller (e.g. an empty block proposal), + // fall back to the most recent completed Store. It is at a height + // <= target, so it is safe to serve as long as producers keep it + // free of state that a since-committed block invalidated (the + // cosmos pool does this via its committed-nonce watermark). + hs.mu.RLock() + value := hs.store + hs.mu.RUnlock() + return value } } } diff --git a/mempool/internal/heightsync/heightsync_test.go b/mempool/internal/heightsync/heightsync_test.go index ff747c70c..84e851d5d 100644 --- a/mempool/internal/heightsync/heightsync_test.go +++ b/mempool/internal/heightsync/heightsync_test.go @@ -190,6 +190,60 @@ func TestStartNewHeightResetsValue(t *testing.T) { require.Empty(t, result.get()) } +func TestStartNewHeightFromCarriesStore(t *testing.T) { + hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()) + + hv.StartNewHeight(big.NewInt(1)) + hv.Do(func(s *testStore) { s.add("carried") }) + hv.EndCurrentHeight() + + // advance to height 2 carrying the previous store forward + hv.StartNewHeightFrom(big.NewInt(2), func(prev *testStore) *testStore { + return &testStore{items: prev.get()} + }) + hv.Do(func(s *testStore) { s.add("fresh") }) + hv.EndCurrentHeight() + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + result := hv.GetStore(ctx, big.NewInt(2)) + require.NotNil(t, result) + require.Equal(t, []string{"carried", "fresh"}, result.get()) +} + +// With the stale-fallback option, GetStore returns the most recent completed +// store instead of nil when it times out behind the target height. +func TestStaleFallbackReturnsLastStore(t *testing.T) { + hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()).WithStaleFallback() + + hv.StartNewHeight(big.NewInt(1)) + hv.Do(func(s *testStore) { s.add("h1") }) + hv.EndCurrentHeight() + + // request height 2 but never advance to it: times out on the height-behind path + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + value := hv.GetStore(ctx, big.NewInt(2)) + require.NotNil(t, value) + require.Equal(t, []string{"h1"}, value.get()) +} + +// Without the option, the same height-behind timeout returns nil (the default). +func TestNoStaleFallbackReturnsNil(t *testing.T) { + hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()) + + hv.StartNewHeight(big.NewInt(1)) + hv.Do(func(s *testStore) { s.add("h1") }) + hv.EndCurrentHeight() + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + require.Nil(t, hv.GetStore(ctx, big.NewInt(2))) +} + func TestConcurrentDo(t *testing.T) { hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()) diff --git a/mempool/mempool.go b/mempool/mempool.go index 336342e56..2d38334ff 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -200,11 +200,14 @@ func NewMempool( panic("tx pool should contain only legacypool") } + // Stale fallback: when recheck loop falls behind consensus, serve last completed + // snapshot rather than an empty proposal — since store's committed-nonce + // watermark keeps already-committed txs out. heightSync := heightsync.New( blockchain.CurrentBlock().Number, NewCosmosTxStore, logger.With("pool", "cosmos_recheck_mempool"), - ) + ).WithStaleFallback() reservationHandle := reservationTracker.NewHandle(cosmosReserverHandlerID, reserver.WithRefCounter()) @@ -447,6 +450,10 @@ func (m *Mempool) removeCosmosTx(tx sdk.Tx, reason sdkmempool.RemoveReason) erro if reason.Caller == sdkmempool.CallerRunTxFinalize { m.recordNonceAdvances(tx) + // Prune committed tx from recheck snapshot synchronously. Snapshot is carried + // across heights, so without this a just-committed tx could be served + // into next proposal before async recheck pass drops it. + m.recheckCosmosPool.PruneCommitted(tx) } if err := m.recheckCosmosPool.Remove(tx); err != nil { diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 5a38319a0..a4f9704d9 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -429,7 +429,13 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header m.mu.Lock() defer m.mu.Unlock() - m.recheckedTxs.StartNewHeight(newHead.Number) + // Carry the validated set forward instead of resetting to an empty store, + // a pass cancelled by next block does not discard all progress and starve proposals. + // The pass prunes whatever became invalid, committed txs are kept out by + // the store's watermark (see CosmosTxStore.PruneCommitted). + m.recheckedTxs.StartNewHeightFrom(newHead.Number, func(prev *CosmosTxStore) *CosmosTxStore { + return prev.Clone() + }) defer m.recheckedTxs.EndCurrentHeight() latestCtx, err := m.blockchain.GetLatestContext() @@ -527,6 +533,9 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header continue } m.reapList.DropCosmosTx(txn) + // Drop from the carried-forward snapshot too; otherwise a tx that just + // failed recheck would linger in the store from the previous height. + m.markTxRemoved(txn) if err := m.unreserveTx(txn); err != nil { m.logger.Error("failed to release reservations", "err", err) @@ -541,6 +550,20 @@ func (m *RecheckMempool) markTxRechecked(txn sdk.Tx) { m.recheckedTxs.Do(func(store *CosmosTxStore) { store.AddTx(txn) }) } +// markTxRemoved drops a tx from the height synced cosmos tx store. +func (m *RecheckMempool) markTxRemoved(txn sdk.Tx) { + m.recheckedTxs.Do(func(store *CosmosTxStore) { store.RemoveTx(txn) }) +} + +// PruneCommitted records that a block being finalized consumed tx's +// signer/nonces and drops tx (and any lower-nonced sibling) from current snapshot. +// It runs synchronously during FinalizeBlock so carried-forward store +// can never feed an already-committed tx into a later proposal, +// even before next recheck pass runs. +func (m *RecheckMempool) PruneCommitted(txn sdk.Tx) { + m.recheckedTxs.Do(func(store *CosmosTxStore) { store.PruneCommitted(txn) }) +} + // markTxInserted conservatively updates the current height snapshot for live inserts. // If the inserted tx replaces an existing tx, any other txs from the same sender with // a higher nonce is dropped and rebuilt by the next recheck. diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index d57233455..3cdd6da36 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -726,6 +726,67 @@ func TestRecheckMempool_RecheckedTxs(t *testing.T) { } } +// TestRecheckMempool_CarryForwardSurvivesCancellation verifies the fix for the +// cosmos-pool proposal starvation: a recheck pass carries the previous height's +// validated set forward, so a pass that is cancelled (or merely still running) +// before it validates anything does not present an empty snapshot to proposals. +// Before the fix, StartNewHeight reset the store to empty each height, so a +// pass that had not yet re-added txs exposed a zero-length snapshot. +func TestRecheckMempool_CarryForwardSurvivesCancellation(t *testing.T) { + tracker := reserver.NewReservationTracker() + handle := tracker.NewHandle(1) + ctx := newRecheckTestContext() + bc := newTestBlockchain(t, ctx) + + const numTxs = 5 + + var blockPass atomic.Bool + ready := make(chan struct{}) + gate := make(chan struct{}) + anteHandler := func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + if blockPass.Load() { + ready <- struct{}{} + <-gate + } + return ctx, nil + } + + rc := newMockRechecker(ctx, anteHandler) + mp := mempool.NewRecheckMempool( + nil, 0, handle, rc, + newTestRecheckedTxs(), newTestReapList(), bc, log.NewNopLogger(), + ) + mp.Start(testHeader(0)) + defer mp.Close() + + // Insert and validate a set of txs at height 1. + for range numTxs { + key, _ := crypto.GenerateKey() + require.NoError(t, mp.Insert(ctx, newRecheckTestTx(t, key))) + } + mp.TriggerRecheckSync(testHeader(1)) + require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(1))), numTxs) + + // Start a height-2 pass that blocks on the very first ante call, before it + // has re-added any tx to the height-2 snapshot. + blockPass.Store(true) + mp.TriggerRecheck(testHeader(2)) + <-ready // the pass is now stalled having added nothing itself + + // The height-2 snapshot must already expose the carried-forward set. Use a + // short timeout since the (stalled) pass will not call EndCurrentHeight. + getCtx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + carried := collectIteratorTxs(mp.RecheckedTxs(getCtx, big.NewInt(2))) + require.Len(t, carried, numTxs, "carried-forward snapshot must not be empty mid-pass") + + // Let the stalled pass finish. blockPass is cleared before releasing the + // gate, so only the already-stalled ante was waiting: the remaining txs + // pass through without signalling ready again. + blockPass.Store(false) + close(gate) +} + func TestRecheckMempool_RecheckedTxsBlocksUntilComplete(t *testing.T) { acc := newRecheckTestAccount(t) tracker := reserver.NewReservationTracker() diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 07724ec63..4d7168538 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -16,8 +16,17 @@ import ( // CosmosTxStore is a set of cosmos transactions that can be added to or // removed from. type CosmosTxStore struct { - txs map[string]cosmosTxBucket - nextUnkeyed uint64 + txs map[string]cosmosTxBucket + nextUnkeyed uint64 + + // consumed is a per-signer high-water mark of nonces consumed by committed + // blocks: AddTx rejects and PruneCommitted drops txs at or below it. It + // exists because the store is carried across heights (see Clone) — without + // it, a recheck pass or an Insert racing FinalizeBlock could re-add a + // just-committed tx and feed it back into a proposal. Holds one entry per + // signer that has ever committed (grows with active accounts, not traffic). + consumed map[string]uint64 + logger log.Logger signerExtractor sdkmempool.SignerExtractionAdapter mu sync.RWMutex @@ -40,17 +49,55 @@ type cosmosTxWithMetadata struct { func NewCosmosTxStore(l log.Logger) *CosmosTxStore { return &CosmosTxStore{ txs: make(map[string]cosmosTxBucket), + consumed: make(map[string]uint64), logger: l, signerExtractor: sdkmempool.NewDefaultSignerExtractionAdapter(), } } +// Clone returns a deep-enough copy of store for carrying the validated set forward +// into next height. The tx values are shared (immutable), but the +// bucket/index/consumed maps are copied so mutations on clone do not affect source. +func (s *CosmosTxStore) Clone() *CosmosTxStore { + s.mu.RLock() + defer s.mu.RUnlock() + + clone := &CosmosTxStore{ + txs: make(map[string]cosmosTxBucket, len(s.txs)), + consumed: make(map[string]uint64, len(s.consumed)), + nextUnkeyed: s.nextUnkeyed, + logger: s.logger, + signerExtractor: s.signerExtractor, + } + for signerKey, bucket := range s.txs { + txs := make([]cosmosTxWithMetadata, len(bucket.txs)) + copy(txs, bucket.txs) + signers := make(map[string]struct{}, len(bucket.signers)) + for signer := range bucket.signers { + signers[signer] = struct{}{} + } + clone.txs[signerKey] = cosmosTxBucket{txs: txs, signers: signers} + } + for signer, nonce := range s.consumed { + clone.consumed[signer] = nonce + } + return clone +} + // AddTx adds a single tx to the store while constructing a validated snapshot. func (s *CosmosTxStore) AddTx(tx sdk.Tx) { s.mu.Lock() defer s.mu.Unlock() storedTx := s.newCosmosTxWithMetadata(tx) + + // Reject txs whose nonce a committed block already consumed. This guards the + // carried-forward store from re-admitting an already-committed tx via a + // recheck pass or an Insert that races FinalizeBlock. + if s.isConsumedLocked(storedTx.nonceMap) { + return + } + if storedTx.signerKey == "" { storedTx.signerKey = unkeyedSignerKey } @@ -59,10 +106,13 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { } bucket := s.txs[storedTx.signerKey] - for _, existing := range bucket.txs { + for i, existing := range bucket.txs { if existing.txKey == storedTx.txKey { - // this should never happen. panicking for safety - s.logger.Warn("attempted to add duplicate tx to CosmosTxStore", "key", storedTx.txKey) + // The slot is already occupied — expected with a carried-forward + // store, where each recheck pass re-adds still-valid txs. Overwrite: + // the pool admits one tx per signer/nonce, so the newest wins. + bucket.txs[i] = storedTx + s.txs[storedTx.signerKey] = bucket return } } @@ -104,25 +154,105 @@ func (s *CosmosTxStore) InvalidateFrom(tx sdk.Tx) int { if !bucketContainsAnySigner(existingBucket, storedTx.nonceMap) { continue } + removed += s.filterBucketLocked(signerKey, existingBucket, func(t cosmosTxWithMetadata) bool { + return invalidatesCosmosTx(t, storedTx.nonceMap) + }) + } + + return removed +} + +// RemoveTx removes a single tx from the store if present. It is the counterpart +// to AddTx used when a recheck pass drops a tx that became invalid: with a +// carried-forward store the tx would otherwise linger from the previous height. +// Returns true if a tx was removed. +func (s *CosmosTxStore) RemoveTx(tx sdk.Tx) bool { + s.mu.Lock() + defer s.mu.Unlock() - next := existingBucket.txs[:0] - for _, existing := range existingBucket.txs { - if invalidatesCosmosTx(existing, storedTx.nonceMap) { - removed++ - continue - } - next = append(next, existing) + storedTx := s.newCosmosTxWithMetadata(tx) + if storedTx.signerKey == "" || storedTx.txKey == "" { + // unkeyed txs are not addressable for targeted removal + return false + } + + bucket, ok := s.txs[storedTx.signerKey] + if !ok { + return false + } + return s.filterBucketLocked(storedTx.signerKey, bucket, func(t cosmosTxWithMetadata) bool { + return t.txKey == storedTx.txKey + }) > 0 +} + +// PruneCommitted records that a committed block consumed the given tx's +// signer/nonces and drops any stored tx at or below a consumed nonce. It is +// called synchronously as a block is finalized so the carried-forward store can +// never feed an already-committed tx into a later proposal, even before the +// next recheck pass runs. Returns the number of stored txs pruned. +func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { + s.mu.Lock() + defer s.mu.Unlock() + + nonceMap, ok := s.cosmosTxNonceMap(tx) + if !ok { + return 0 + } + + // bump the per-signer high-water mark + for signer, nonce := range nonceMap { + if cur, exists := s.consumed[signer]; !exists || nonce > cur { + s.consumed[signer] = nonce + } + } + + // drop any stored tx now under a watermark: a tx is invalid if ANY of its + // signers has a consumed nonce, since every signer must be executable + removed := 0 + for signerKey, bucket := range s.txs { + removed += s.filterBucketLocked(signerKey, bucket, func(t cosmosTxWithMetadata) bool { + return s.isConsumedLocked(t.nonceMap) + }) + } + + return removed +} + +// isConsumedLocked reports whether any signer of the given nonceMap sits at or +// below the committed high-water mark. Callers must hold s.mu. +func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { + for signer, nonce := range nonceMap { + if mark, ok := s.consumed[signer]; ok && nonce <= mark { + return true } + } + return false +} - clear(existingBucket.txs[len(next):]) - if len(next) == 0 { - delete(s.txs, signerKey) +// filterBucketLocked removes every tx in the bucket at signerKey for which +// match returns true, deleting the bucket if it empties. Callers must hold +// s.mu. Returns the number of txs removed. +func (s *CosmosTxStore) filterBucketLocked(signerKey string, bucket cosmosTxBucket, match func(cosmosTxWithMetadata) bool) int { + next := bucket.txs[:0] + removed := 0 + for _, existing := range bucket.txs { + if match(existing) { + removed++ continue } - existingBucket.txs = next - s.txs[signerKey] = existingBucket + next = append(next, existing) + } + if removed == 0 { + return 0 } + clear(bucket.txs[len(next):]) + if len(next) == 0 { + delete(s.txs, signerKey) + return removed + } + bucket.txs = next + s.txs[signerKey] = bucket return removed } diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index f76510dcd..19a836ad0 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -252,6 +252,128 @@ func TestCosmosTxStoreIteratorSnapshotIsolation(t *testing.T) { require.Equal(t, 2, count) } +func newPubKeyBytes(t *testing.T) []byte { + t.Helper() + key, err := crypto.GenerateKey() + require.NoError(t, err) + return crypto.CompressPubkey(&key.PublicKey) +} + +func TestCosmosTxStoreRemoveTx(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + tx0 := newKeyedMockTxWithPubKey(signer, 0) + tx1 := newKeyedMockTxWithPubKey(signer, 1) + + store.AddTx(tx0) + store.AddTx(tx1) + require.Equal(t, 2, store.Len()) + + require.True(t, store.RemoveTx(tx0)) + require.Equal(t, 1, store.Len()) + + // removing again is a no-op + require.False(t, store.RemoveTx(tx0)) + require.Equal(t, 1, store.Len()) + + // the remaining tx is the one we did not remove + require.True(t, store.RemoveTx(tx1)) + require.Equal(t, 0, store.Len()) +} + +func TestCosmosTxStoreCloneIsIndependent(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) + store.AddTx(newKeyedMockTxWithPubKey(signer, 2)) + // carry a committed watermark forward too: drops nonce 0, leaving 1 and 2 + store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 2, store.Len()) + + clone := store.Clone() + require.Equal(t, store.Len(), clone.Len()) + + // mutating the clone must not affect the source + clone.AddTx(newKeyedMockTxWithPubKey(signer, 3)) + require.Equal(t, 2, store.Len()) + require.Equal(t, 3, clone.Len()) + + // mutating the source must not affect the clone + require.True(t, store.RemoveTx(newKeyedMockTxWithPubKey(signer, 1))) + require.Equal(t, 1, store.Len()) + require.Equal(t, 3, clone.Len()) + + // the committed watermark is carried: the clone still rejects the consumed nonce + clone.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 3, clone.Len()) +} + +func TestCosmosTxStorePruneCommitted(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) + store.AddTx(newKeyedMockTxWithPubKey(signer, 2)) + require.Equal(t, 3, store.Len()) + + // committing nonce 0 drops nonce 0, keeps 1 and 2 + require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0))) + require.Equal(t, 2, store.Len()) + + // a re-add of the committed nonce is rejected by the watermark + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 2, store.Len()) + + // committing nonce 1 drops nonce 1, keeps 2 + require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 1))) + require.Equal(t, 1, store.Len()) + store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) + require.Equal(t, 1, store.Len()) +} + +// A committed single-signer tx must evict a pooled multi-signer tx that shares +// that signer/nonce — the exact case the deferred-removal comment warns about. +func TestCosmosTxStorePruneCommittedMultiSigner(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signerA := newPubKeyBytes(t) + signerB := newPubKeyBytes(t) + + // a tx signed by both A@0 and B@0 + multi := newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{0, 0}) + store.AddTx(multi) + require.Equal(t, 1, store.Len()) + + // committing A@0 (single signer) must drop the multi-signer tx + require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signerA, 0))) + require.Equal(t, 0, store.Len()) + + // and it stays out even if a recheck tries to re-add it + store.AddTx(multi) + require.Equal(t, 0, store.Len()) +} + +// AddTx overwrites the tx occupying a signer/nonce slot rather than dropping the +// update, so a carried-forward store reflects the latest tx for that slot. +func TestCosmosTxStoreAddOverwritesSlot(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + store.AddTx(newFeeKeyedMockTxWithPubKey(signer, 0, 1)) + store.AddTx(newFeeKeyedMockTxWithPubKey(signer, 0, 5)) // same slot, higher fee + + require.Equal(t, 1, store.Len()) + txs := store.Txs() + require.Len(t, txs, 1) + feeTx, ok := txs[0].(sdk.FeeTx) + require.True(t, ok) + require.Equal(t, sdk.NewInt64Coin(feeKeyedMockTxDenom, 5*100_000), feeTx.GetFee()[0]) +} + func TestCosmosTxStoreOrdersBucketByNonceSum(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) diff --git a/server/server_app_options.go b/server/server_app_options.go index 59157518a..eaec8c667 100644 --- a/server/server_app_options.go +++ b/server/server_app_options.go @@ -114,7 +114,9 @@ func GetBlockGasLimit(appOpts servertypes.AppOptions, logger log.Logger) uint64 maxGas := genDoc.ConsensusParams.Block.MaxGas if maxGas == -1 { - logger.Warn("genesis max_gas is unlimited (-1), using max int64 block gas limit") + logger.Warn("genesis max_gas is unlimited (-1), using max int64 block gas limit; " + + "with app-side mempool an unbounded block can reap a whole tx backlog " + + "and exceed timeout_propose — set a finite consensus block.max_gas") return math.MaxInt64 } if maxGas < -1 { From eafccdb82071cc999461c126f737cd9a331cc1ff Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 04:35:35 +0800 Subject: [PATCH 02/22] cleanup --- mempool/internal/heightsync/heightsync.go | 19 ++-- .../internal/heightsync/heightsync_test.go | 4 +- mempool/recheck_pool.go | 4 +- mempool/tx_store.go | 89 ++++++++++++------- 4 files changed, 69 insertions(+), 47 deletions(-) diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index 012cd51d4..925737300 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -116,11 +116,13 @@ type HeightSync[Store any] struct { // fields of the Store itself mu sync.RWMutex - // staleFallback makes GetStore return the most recent completed Store + // staleFallback makes GetStore return the current carried-forward Store // (instead of nil) when it times out while still behind the target height. - // Only enable it for stores that stay free of state a committed block - // invalidated (see the cosmos pool's committed-nonce watermark), otherwise - // a stale Store could serve already-committed txs. + // That Store is at a height <= target and, even mid-recheck, carry-forward + // keeps it a valid subset of validated txs. Only enable it for stores that + // stay free of state a committed block invalidated (see the cosmos pool's + // committed-nonce watermark), otherwise a stale Store could serve + // already-committed txs. staleFallback bool logger log.Logger @@ -285,10 +287,11 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto return nil } // Rather than starve the caller (e.g. an empty block proposal), - // fall back to the most recent completed Store. It is at a height - // <= target, so it is safe to serve as long as producers keep it - // free of state that a since-committed block invalidated (the - // cosmos pool does this via its committed-nonce watermark). + // fall back to the current carried-forward Store: it is at a + // height <= target and, even mid-recheck, holds a valid subset of + // validated txs. Safe to serve as long as producers keep it free + // of state a since-committed block invalidated (the cosmos pool + // does this via its committed-nonce watermark). hs.mu.RLock() value := hs.store hs.mu.RUnlock() diff --git a/mempool/internal/heightsync/heightsync_test.go b/mempool/internal/heightsync/heightsync_test.go index 84e851d5d..1fc3c65b3 100644 --- a/mempool/internal/heightsync/heightsync_test.go +++ b/mempool/internal/heightsync/heightsync_test.go @@ -212,8 +212,8 @@ func TestStartNewHeightFromCarriesStore(t *testing.T) { require.Equal(t, []string{"carried", "fresh"}, result.get()) } -// With the stale-fallback option, GetStore returns the most recent completed -// store instead of nil when it times out behind the target height. +// With the stale-fallback option, GetStore returns the current store (at a +// height <= target) instead of nil when it times out behind the target height. func TestStaleFallbackReturnsLastStore(t *testing.T) { hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()).WithStaleFallback() diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index a4f9704d9..196a1c5bf 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -433,9 +433,7 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header // a pass cancelled by next block does not discard all progress and starve proposals. // The pass prunes whatever became invalid, committed txs are kept out by // the store's watermark (see CosmosTxStore.PruneCommitted). - m.recheckedTxs.StartNewHeightFrom(newHead.Number, func(prev *CosmosTxStore) *CosmosTxStore { - return prev.Clone() - }) + m.recheckedTxs.StartNewHeightFrom(newHead.Number, (*CosmosTxStore).Clone) defer m.recheckedTxs.EndCurrentHeight() latestCtx, err := m.blockchain.GetLatestContext() diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 4d7168538..4e014a5c6 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -1,18 +1,38 @@ package mempool import ( + "context" "fmt" + "maps" "slices" "strconv" "strings" "sync" + "go.opentelemetry.io/otel/metric" + "cosmossdk.io/log/v2" sdk "github.com/cosmos/cosmos-sdk/types" sdkmempool "github.com/cosmos/cosmos-sdk/types/mempool" ) +// consumedWatermarkSize reports how many per-signer committed-nonce watermarks +// the carried-forward store retains. The consumed map is never evicted, so this +// gauge surfaces unexpected growth. +var consumedWatermarkSize metric.Int64Gauge + +func init() { + var err error + consumedWatermarkSize, err = meter.Int64Gauge( + "cosmos_tx_store.consumed_watermark_size", + metric.WithDescription("Number of per-signer committed-nonce watermarks retained by the carried-forward store"), + ) + if err != nil { + panic(err) + } +} + // CosmosTxStore is a set of cosmos transactions that can be added to or // removed from. type CosmosTxStore struct { @@ -24,7 +44,9 @@ type CosmosTxStore struct { // exists because the store is carried across heights (see Clone) — without // it, a recheck pass or an Insert racing FinalizeBlock could re-add a // just-committed tx and feed it back into a proposal. Holds one entry per - // signer that has ever committed (grows with active accounts, not traffic). + // signer that has ever committed (grows with active accounts, not traffic); + // never evicted, so PruneCommitted reports its size via the + // cosmos_tx_store.consumed_watermark_size gauge. consumed map[string]uint64 logger log.Logger @@ -64,22 +86,16 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { clone := &CosmosTxStore{ txs: make(map[string]cosmosTxBucket, len(s.txs)), - consumed: make(map[string]uint64, len(s.consumed)), + consumed: maps.Clone(s.consumed), nextUnkeyed: s.nextUnkeyed, logger: s.logger, signerExtractor: s.signerExtractor, } for signerKey, bucket := range s.txs { - txs := make([]cosmosTxWithMetadata, len(bucket.txs)) - copy(txs, bucket.txs) - signers := make(map[string]struct{}, len(bucket.signers)) - for signer := range bucket.signers { - signers[signer] = struct{}{} + clone.txs[signerKey] = cosmosTxBucket{ + txs: slices.Clone(bucket.txs), + signers: maps.Clone(bucket.signers), } - clone.txs[signerKey] = cosmosTxBucket{txs: txs, signers: signers} - } - for signer, nonce := range s.consumed { - clone.consumed[signer] = nonce } return clone } @@ -149,17 +165,9 @@ func (s *CosmosTxStore) InvalidateFrom(tx sdk.Tx) int { return 0 } - removed := 0 - for signerKey, existingBucket := range s.txs { - if !bucketContainsAnySigner(existingBucket, storedTx.nonceMap) { - continue - } - removed += s.filterBucketLocked(signerKey, existingBucket, func(t cosmosTxWithMetadata) bool { - return invalidatesCosmosTx(t, storedTx.nonceMap) - }) - } - - return removed + return s.filterSignerBucketsLocked(storedTx.nonceMap, func(t cosmosTxWithMetadata) bool { + return invalidatesCosmosTx(t, storedTx.nonceMap) + }) } // RemoveTx removes a single tx from the store if present. It is the counterpart @@ -176,11 +184,7 @@ func (s *CosmosTxStore) RemoveTx(tx sdk.Tx) bool { return false } - bucket, ok := s.txs[storedTx.signerKey] - if !ok { - return false - } - return s.filterBucketLocked(storedTx.signerKey, bucket, func(t cosmosTxWithMetadata) bool { + return s.filterBucketLocked(storedTx.signerKey, s.txs[storedTx.signerKey], func(t cosmosTxWithMetadata) bool { return t.txKey == storedTx.txKey }) > 0 } @@ -207,13 +211,15 @@ func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { } // drop any stored tx now under a watermark: a tx is invalid if ANY of its - // signers has a consumed nonce, since every signer must be executable - removed := 0 - for signerKey, bucket := range s.txs { - removed += s.filterBucketLocked(signerKey, bucket, func(t cosmosTxWithMetadata) bool { - return s.isConsumedLocked(t.nonceMap) - }) - } + // signers has a consumed nonce. Only buckets sharing a signer with the + // just-committed tx can hold a newly consumed tx (a prior commit already + // pruned the rest and AddTx rejects re-adds). + removed := s.filterSignerBucketsLocked(nonceMap, func(t cosmosTxWithMetadata) bool { + return s.isConsumedLocked(t.nonceMap) + }) + + // report the unbounded watermark map's size for monitoring + consumedWatermarkSize.Record(context.Background(), int64(len(s.consumed))) return removed } @@ -256,6 +262,21 @@ func (s *CosmosTxStore) filterBucketLocked(signerKey string, bucket cosmosTxBuck return removed } +// filterSignerBucketsLocked removes every tx matching match from the buckets +// sharing at least one signer with nonceMap, skipping unrelated buckets so the +// scan stays proportional to the supplied tx rather than the whole pool. +// Callers must hold s.mu. Returns the number of txs removed. +func (s *CosmosTxStore) filterSignerBucketsLocked(nonceMap map[string]uint64, match func(cosmosTxWithMetadata) bool) int { + removed := 0 + for signerKey, bucket := range s.txs { + if !bucketContainsAnySigner(bucket, nonceMap) { + continue + } + removed += s.filterBucketLocked(signerKey, bucket, match) + } + return removed +} + func (s *CosmosTxStore) newCosmosTxWithMetadata(tx sdk.Tx) cosmosTxWithMetadata { storedTx := cosmosTxWithMetadata{tx: tx} From 740c403268c7d42c5a22d45cc24deacf9dcb13cf Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 17:30:01 +0800 Subject: [PATCH 03/22] fix: age committed-nonce watermarks and cover unordered/EVM commits * retire watermarks once a completed recheck pass revalidates pool: bounds maps and heals marks from optimistically-executed blocks that never committed * drop committed unordered txs by identity instead of watermarking (ChooseNonce value is timestamp instead of sequence) * prune cosmos snapshot for committed EVM txs, which consume same account sequence --- mempool/mempool.go | 3 ++ mempool/recheck_pool.go | 3 ++ mempool/tx_store.go | 62 +++++++++++++++++++++++++----------- mempool/tx_store_test.go | 68 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 19 deletions(-) diff --git a/mempool/mempool.go b/mempool/mempool.go index 1843bc534..7dbfc7a05 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -481,6 +481,9 @@ func (m *Mempool) removeEVMTx(tx sdk.Tx, msgEthereumTx *evmtypes.MsgEthereumTx, if reason.Caller == sdkmempool.CallerRunTxFinalize { _ = m.txTracker.IncludedInBlock(hash) m.recordNonceAdvances(tx) + // an EVM tx consumes the same account sequence, so drop stale + // same-account cosmos txs from the snapshot too + m.recheckCosmosPool.PruneCommitted(tx) } if m.shouldRemoveFromEVMPool(hash, reason) { diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 196a1c5bf..18fe96f4c 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -541,6 +541,9 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } } txsRemoved = len(removeTxs) + + // a completed pass makes watermarks recorded before it redundant + m.recheckedTxs.Do(func(store *CosmosTxStore) { store.AgeWatermarks() }) } // markTxRechecked adds a tx into the height synced cosmos tx store. diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 4e014a5c6..767e71e3a 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -17,9 +17,8 @@ import ( sdkmempool "github.com/cosmos/cosmos-sdk/types/mempool" ) -// consumedWatermarkSize reports how many per-signer committed-nonce watermarks -// the carried-forward store retains. The consumed map is never evicted, so this -// gauge surfaces unexpected growth. +// consumedWatermarkSize reports how many per-signer committed-nonce +// watermarks survived the latest aging (see AgeWatermarks). var consumedWatermarkSize metric.Int64Gauge func init() { @@ -39,15 +38,12 @@ type CosmosTxStore struct { txs map[string]cosmosTxBucket nextUnkeyed uint64 - // consumed is a per-signer high-water mark of nonces consumed by committed - // blocks: AddTx rejects and PruneCommitted drops txs at or below it. It - // exists because the store is carried across heights (see Clone) — without - // it, a recheck pass or an Insert racing FinalizeBlock could re-add a - // just-committed tx and feed it back into a proposal. Holds one entry per - // signer that has ever committed (grows with active accounts, not traffic); - // never evicted, so PruneCommitted reports its size via the - // cosmos_tx_store.consumed_watermark_size gauge. - consumed map[string]uint64 + // consumed and prevConsumed hold two generations of per-signer high-water + // marks of committed nonces, so the carried-forward store cannot re-admit + // a just-committed tx: AddTx rejects and PruneCommitted drops txs at or + // below a mark; AgeWatermarks retires the older generation. + consumed map[string]uint64 + prevConsumed map[string]uint64 logger log.Logger signerExtractor sdkmempool.SignerExtractionAdapter @@ -72,6 +68,7 @@ func NewCosmosTxStore(l log.Logger) *CosmosTxStore { return &CosmosTxStore{ txs: make(map[string]cosmosTxBucket), consumed: make(map[string]uint64), + prevConsumed: make(map[string]uint64), logger: l, signerExtractor: sdkmempool.NewDefaultSignerExtractionAdapter(), } @@ -87,6 +84,7 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { clone := &CosmosTxStore{ txs: make(map[string]cosmosTxBucket, len(s.txs)), consumed: maps.Clone(s.consumed), + prevConsumed: maps.Clone(s.prevConsumed), nextUnkeyed: s.nextUnkeyed, logger: s.logger, signerExtractor: s.signerExtractor, @@ -184,9 +182,15 @@ func (s *CosmosTxStore) RemoveTx(tx sdk.Tx) bool { return false } - return s.filterBucketLocked(storedTx.signerKey, s.txs[storedTx.signerKey], func(t cosmosTxWithMetadata) bool { - return t.txKey == storedTx.txKey - }) > 0 + return s.removeTxKeyLocked(storedTx.signerKey, storedTx.txKey) > 0 +} + +// removeTxKeyLocked removes the tx with the exact txKey from its signer-set +// bucket. Callers must hold s.mu. Returns the number of txs removed (0 or 1). +func (s *CosmosTxStore) removeTxKeyLocked(signerKey, txKey string) int { + return s.filterBucketLocked(signerKey, s.txs[signerKey], func(t cosmosTxWithMetadata) bool { + return t.txKey == txKey + }) } // PruneCommitted records that a committed block consumed the given tx's @@ -203,6 +207,13 @@ func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { return 0 } + // An unordered tx consumes no sequence — its nonce is a timeout timestamp + // that would blacklist the signer if watermarked. Drop exactly this tx; + // on-chain unordered-nonce tracking prevents re-execution. + if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { + return s.removeTxKeyLocked(cosmosTxSignerSetKey(nonceMap), cosmosTxKey(nonceMap)) + } + // bump the per-signer high-water mark for signer, nonce := range nonceMap { if cur, exists := s.consumed[signer]; !exists || nonce > cur { @@ -214,14 +225,24 @@ func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { // signers has a consumed nonce. Only buckets sharing a signer with the // just-committed tx can hold a newly consumed tx (a prior commit already // pruned the rest and AddTx rejects re-adds). - removed := s.filterSignerBucketsLocked(nonceMap, func(t cosmosTxWithMetadata) bool { + return s.filterSignerBucketsLocked(nonceMap, func(t cosmosTxWithMetadata) bool { return s.isConsumedLocked(t.nonceMap) }) +} - // report the unbounded watermark map's size for monitoring - consumedWatermarkSize.Record(context.Background(), int64(len(s.consumed))) +// AgeWatermarks retires the older watermark generation. Call it only after +// an uncancelled recheck pass: the pool was revalidated against state at +// least as new as those marks' commits, so ante now rejects the re-adds they +// guarded against; marks written mid-pass survive one more generation. This +// also heals marks from optimistically-executed blocks that never committed. +func (s *CosmosTxStore) AgeWatermarks() { + s.mu.Lock() + defer s.mu.Unlock() - return removed + s.prevConsumed = s.consumed + s.consumed = make(map[string]uint64) + + consumedWatermarkSize.Record(context.Background(), int64(len(s.prevConsumed))) } // isConsumedLocked reports whether any signer of the given nonceMap sits at or @@ -231,6 +252,9 @@ func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { if mark, ok := s.consumed[signer]; ok && nonce <= mark { return true } + if mark, ok := s.prevConsumed[signer]; ok && nonce <= mark { + return true + } } return false } diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index 19a836ad0..7d07db35e 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -2,6 +2,7 @@ package mempool import ( "testing" + "time" "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" @@ -93,6 +94,25 @@ func (m *keyedMockTx) GetSignaturesV2() ([]signingtypes.SignatureV2, error) { }}, nil } +// unorderedMockTx is a keyedMockTx flagged unordered: its ChooseNonce value is +// the timeout timestamp, not the (zero) sequence. +type unorderedMockTx struct { + keyedMockTx + timeout time.Time +} + +var _ sdk.TxWithUnordered = (*unorderedMockTx)(nil) + +func newUnorderedMockTxWithPubKey(pubKeyBytes []byte, timeout time.Time) sdk.Tx { + return &unorderedMockTx{ + keyedMockTx: keyedMockTx{pubKey: ðsecp256k1.PubKey{Key: pubKeyBytes}}, + timeout: timeout, + } +} + +func (m *unorderedMockTx) GetUnordered() bool { return true } +func (m *unorderedMockTx) GetTimeoutTimeStamp() time.Time { return m.timeout } + func newMultiKeyedMockTx(pubKeyBytes [][]byte, sequences []uint64) sdk.Tx { pubKeys := make([]cryptotypes.PubKey, 0, len(pubKeyBytes)) for _, pubKey := range pubKeyBytes { @@ -311,6 +331,29 @@ func TestCosmosTxStoreCloneIsIndependent(t *testing.T) { require.Equal(t, 3, clone.Len()) } +// A watermark blocks re-adds for its own generation plus one aging, then is +// retired: two completed recheck passes have covered the commit by then. +func TestCosmosTxStoreAgeWatermarks(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0))) + + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 0, store.Len()) + + // first aging keeps the mark one more generation + store.AgeWatermarks() + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 0, store.Len()) + + // second aging retires it; a stale (e.g. never-committed) mark heals here + store.AgeWatermarks() + store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + require.Equal(t, 1, store.Len()) +} + func TestCosmosTxStorePruneCommitted(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) @@ -335,6 +378,31 @@ func TestCosmosTxStorePruneCommitted(t *testing.T) { require.Equal(t, 1, store.Len()) } +// Committing an unordered tx must not watermark the signer — that would +// blacklist their ordered txs. Only the exact tx is dropped. +func TestCosmosTxStorePruneCommittedUnordered(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signer := newPubKeyBytes(t) + timeout := time.Unix(1_700_000_000, 0) + unordered := newUnorderedMockTxWithPubKey(signer, timeout) + earlier := newUnorderedMockTxWithPubKey(signer, timeout.Add(-time.Second)) + ordered := newKeyedMockTxWithPubKey(signer, 5) + + store.AddTx(unordered) + store.AddTx(earlier) + store.AddTx(ordered) + require.Equal(t, 3, store.Len()) + + // only the committed unordered tx is dropped, not the signer's other txs + require.Equal(t, 1, store.PruneCommitted(unordered)) + require.Equal(t, 2, store.Len()) + + // no watermark was recorded: the signer's ordered txs stay addable + store.AddTx(newKeyedMockTxWithPubKey(signer, 6)) + require.Equal(t, 3, store.Len()) +} + // A committed single-signer tx must evict a pooled multi-signer tx that shares // that signer/nonce — the exact case the deferred-removal comment warns about. func TestCosmosTxStorePruneCommittedMultiSigner(t *testing.T) { From 510524c90b82b1a276e12d38562388919f151147 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 18:26:15 +0800 Subject: [PATCH 04/22] fix: purge stale txs from carried-forward snapshot * drop failed-recheck txs at detection time since end-of-pass removal loop is skipped on cancellation * invalidate a replaced tx whose signer set differs from its replacement via replacement hook, where InvalidateFrom cannot see it * stop carrying unkeyed txs across heights: they are unremovable and would duplicate every pass --- mempool/recheck_pool.go | 16 +++++++--- mempool/recheck_pool_test.go | 59 ++++++++++++++++++++++++++++++++++++ mempool/tx_store.go | 24 +++++++++++++++ mempool/tx_store_test.go | 41 +++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 5 deletions(-) diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 18fe96f4c..9ab8288c5 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -130,7 +130,7 @@ func NewRecheckMempool( blockchain, defaultCosmosPoolConfig, maxTxs, - onTransactionReplace(reapList, signerExtractor, reserver, logger), + onTransactionReplace(reapList, recheckedTxs, signerExtractor, reserver, logger), ) return &RecheckMempool{ @@ -502,6 +502,10 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } removeTxs = append(removeTxs, txn) + // Drop from the snapshot at detection: the removal loop below is + // skipped on cancellation. ExtMempool removal still waits for the + // loop (reservations, multi-signer identification). + m.markTxRemoved(txn) if keepFuturesOnError { iter = iter.Next() @@ -531,9 +535,6 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header continue } m.reapList.DropCosmosTx(txn) - // Drop from the carried-forward snapshot too; otherwise a tx that just - // failed recheck would linger in the store from the previous height. - m.markTxRemoved(txn) if err := m.unreserveTx(txn); err != nil { m.logger.Error("failed to release reservations", "err", err) @@ -645,16 +646,21 @@ func cosmosPoolConfig( func onTransactionReplace( reapList *reaplist.ReapList, + recheckedTxs *heightsync.HeightSync[CosmosTxStore], signerExtractor sdkmempool.SignerExtractionAdapter, reserver *reserver.ReservationHandle, logger log.Logger, ) func(oldTx, newTx sdk.Tx) { - return func(oldTx, _ sdk.Tx) { + return func(oldTx, newTx sdk.Tx) { // tx is being replaced, we need to drop the tx that is going to be removed // from the reap list. we assume that the tx doing the replacing has // already been inserted into the reaplist via the insert. reapList.DropCosmosTx(oldTx) + // drop the replaced tx from the snapshot when its signer set differs + // from the replacement's (see CosmosTxStore.InvalidateReplaced) + recheckedTxs.Do(func(store *CosmosTxStore) { store.InvalidateReplaced(oldTx, newTx) }) + addrs, err := extractEVMAddresses(signerExtractor, oldTx) if err != nil { return diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index 3cdd6da36..a60e26b76 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -726,6 +726,65 @@ func TestRecheckMempool_RecheckedTxs(t *testing.T) { } } +// A tx that fails recheck must leave the snapshot at detection time — the +// end-of-pass removal loop is skipped on cancellation. +func TestRecheckMempool_FailedTxDroppedBeforeRemovalLoop(t *testing.T) { + tracker := reserver.NewReservationTracker() + handle := tracker.NewHandle(1) + ctx := newRecheckTestContext() + bc := newTestBlockchain(t, ctx) + + const numTxs = 3 + + var failPass atomic.Bool + var calls atomic.Int32 + ready := make(chan struct{}) + gate := make(chan struct{}) + anteHandler := func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + if !failPass.Load() { + return ctx, nil + } + switch calls.Add(1) { + case 1: + // first rechecked tx fails + return ctx, errors.New("recheck failure") + case 2: + // second stalls the pass before its removal loop can run + ready <- struct{}{} + <-gate + } + return ctx, nil + } + + rc := newMockRechecker(ctx, anteHandler) + mp := mempool.NewRecheckMempool( + nil, 0, handle, rc, + newTestRecheckedTxs(), newTestReapList(), bc, log.NewNopLogger(), + ) + mp.Start(testHeader(0)) + defer mp.Close() + + for range numTxs { + key, _ := crypto.GenerateKey() + require.NoError(t, mp.Insert(ctx, newRecheckTestTx(t, key))) + } + mp.TriggerRecheckSync(testHeader(1)) + require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(1))), numTxs) + + failPass.Store(true) + mp.TriggerRecheck(testHeader(2)) + <-ready // one tx has failed and the pass is stalled mid-iteration + + getCtx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + require.Len(t, collectIteratorTxs(mp.RecheckedTxs(getCtx, big.NewInt(2))), numTxs-1, + "failed tx must leave the snapshot before the removal loop runs") + + // let the stalled pass finish; remaining txs pass without signalling again + failPass.Store(false) + close(gate) +} + // TestRecheckMempool_CarryForwardSurvivesCancellation verifies the fix for the // cosmos-pool proposal starvation: a recheck pass carries the previous height's // validated set forward, so a pass that is cancelled (or merely still running) diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 767e71e3a..dee9d7a39 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -90,6 +90,11 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { signerExtractor: s.signerExtractor, } for signerKey, bucket := range s.txs { + // Unkeyed txs are unremovable and get a fresh key on every AddTx, so a + // carried copy would duplicate once per pass; let each pass re-add them. + if signerKey == unkeyedSignerKey { + continue + } clone.txs[signerKey] = cosmosTxBucket{ txs: slices.Clone(bucket.txs), signers: maps.Clone(bucket.signers), @@ -168,6 +173,25 @@ func (s *CosmosTxStore) InvalidateFrom(tx sdk.Tx) int { }) } +// InvalidateReplaced removes a replaced tx (and txs validated on top of its +// nonces) when its signer set differs from the replacement's, which +// InvalidateFrom(newTx) cannot see in newTx's own bucket. Same-set +// replacements stay with InvalidateFrom. Returns the number of txs removed. +func (s *CosmosTxStore) InvalidateReplaced(oldTx, newTx sdk.Tx) int { + s.mu.Lock() + defer s.mu.Unlock() + + oldStored := s.newCosmosTxWithMetadata(oldTx) + newStored := s.newCosmosTxWithMetadata(newTx) + if oldStored.signerKey == "" || oldStored.txKey == "" || oldStored.signerKey == newStored.signerKey { + return 0 + } + + return s.filterSignerBucketsLocked(oldStored.nonceMap, func(t cosmosTxWithMetadata) bool { + return invalidatesCosmosTx(t, oldStored.nonceMap) + }) +} + // RemoveTx removes a single tx from the store if present. It is the counterpart // to AddTx used when a recheck pass drops a tx that became invalid: with a // carried-forward store the tx would otherwise linger from the previous height. diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index 7d07db35e..c2a998651 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -378,6 +378,47 @@ func TestCosmosTxStorePruneCommitted(t *testing.T) { require.Equal(t, 1, store.Len()) } +// Unkeyed txs must not be carried across heights: they get a fresh key on +// every AddTx and cannot be removed, so a carried copy duplicates every pass. +func TestCosmosTxStoreCloneDropsUnkeyed(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + store.AddTx(newMockTx(1)) // no signers: stored under the unkeyed bucket + store.AddTx(newKeyedMockTxWithPubKey(newPubKeyBytes(t), 0)) + require.Equal(t, 2, store.Len()) + + clone := store.Clone() + require.Equal(t, 1, clone.Len(), "clone must not carry the unkeyed bucket") + + // the re-add a recheck pass would perform yields exactly one copy again + clone.AddTx(newMockTx(1)) + require.Equal(t, 2, clone.Len()) +} + +// A replaced multi-signer tx lives in a bucket InvalidateFrom(newTx) cannot +// see; InvalidateReplaced drops it (and its dependents) by its own identity. +func TestCosmosTxStoreInvalidateReplaced(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signerA := newPubKeyBytes(t) + signerB := newPubKeyBytes(t) + oldTx := newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{5, 0}) + dependent := newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{6, 1}) + newTx := newKeyedMockTxWithPubKey(signerA, 5) + + store.AddTx(oldTx) + store.AddTx(dependent) + require.Equal(t, 2, store.Len()) + + // same signer set is a no-op: InvalidateFrom owns that case + require.Equal(t, 0, store.InvalidateReplaced(oldTx, oldTx)) + require.Equal(t, 2, store.Len()) + + // different signer set drops the old tx and anything atop its nonces + require.Equal(t, 2, store.InvalidateReplaced(oldTx, newTx)) + require.Equal(t, 0, store.Len()) +} + // Committing an unordered tx must not watermark the signer — that would // blacklist their ordered txs. Only the exact tx is dropped. func TestCosmosTxStorePruneCommittedUnordered(t *testing.T) { From bb64f9fa4d69e00157ac0dee5781e2a7be443734 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 18:28:53 +0800 Subject: [PATCH 05/22] avoid serve store beyond target height in stale fallback --- mempool/internal/heightsync/heightsync.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index 925737300..e36331b1f 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -294,6 +294,10 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto // does this via its committed-nonce watermark). hs.mu.RLock() value := hs.store + // heights can skip past target while we waited, never serve future state + if hs.currentHeight.Cmp(height) > 0 { + value = nil + } hs.mu.RUnlock() return value } From 55fccca9814e92f188e83f925d81f05655f549be Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 18:36:01 +0800 Subject: [PATCH 06/22] speed up tx store scans and inserts index buckets by signer so PruneCommitted and InvalidateFrom visit only matching buckets instead of probing whole store --- mempool/tx_store.go | 79 +++++++++++++++++++--------------------- mempool/tx_store_test.go | 13 +++++++ 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/mempool/tx_store.go b/mempool/tx_store.go index dee9d7a39..3d7305dd9 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -38,6 +38,11 @@ type CosmosTxStore struct { txs map[string]cosmosTxBucket nextUnkeyed uint64 + // signerBuckets indexes signer -> keys of the buckets containing it, so + // shared-signer scans (PruneCommitted, InvalidateFrom) touch only matching + // buckets. Membership changes only when a bucket is created or emptied. + signerBuckets map[string]map[string]struct{} + // consumed and prevConsumed hold two generations of per-signer high-water // marks of committed nonces, so the carried-forward store cannot re-admit // a just-committed tx: AddTx rejects and PruneCommitted drops txs at or @@ -67,6 +72,7 @@ type cosmosTxWithMetadata struct { func NewCosmosTxStore(l log.Logger) *CosmosTxStore { return &CosmosTxStore{ txs: make(map[string]cosmosTxBucket), + signerBuckets: make(map[string]map[string]struct{}), consumed: make(map[string]uint64), prevConsumed: make(map[string]uint64), logger: l, @@ -83,12 +89,16 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { clone := &CosmosTxStore{ txs: make(map[string]cosmosTxBucket, len(s.txs)), + signerBuckets: make(map[string]map[string]struct{}, len(s.signerBuckets)), consumed: maps.Clone(s.consumed), prevConsumed: maps.Clone(s.prevConsumed), nextUnkeyed: s.nextUnkeyed, logger: s.logger, signerExtractor: s.signerExtractor, } + for signer, bucketKeys := range s.signerBuckets { + clone.signerBuckets[signer] = maps.Clone(bucketKeys) + } for signerKey, bucket := range s.txs { // Unkeyed txs are unremovable and get a fresh key on every AddTx, so a // carried copy would duplicate once per pass; let each pass re-add them. @@ -124,23 +134,25 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { storedTx.txKey = s.newUnkeyedStoreKey() } + // bucket.txs is sorted by (nonceSum, txKey): overwrite an occupied slot — + // each recheck pass re-adds still-valid txs and the newest wins. bucket := s.txs[storedTx.signerKey] - for i, existing := range bucket.txs { - if existing.txKey == storedTx.txKey { - // The slot is already occupied — expected with a carried-forward - // store, where each recheck pass re-adds still-valid txs. Overwrite: - // the pool admits one tx per signer/nonce, so the newest wins. - bucket.txs[i] = storedTx - s.txs[storedTx.signerKey] = bucket - return - } + i, found := slices.BinarySearchFunc(bucket.txs, storedTx, compareCosmosTxWithMetadata) + if found { + bucket.txs[i] = storedTx + return } if bucket.signers == nil { bucket.signers = signerSetFromNonceMap(storedTx.nonceMap) + for signer := range bucket.signers { + if s.signerBuckets[signer] == nil { + s.signerBuckets[signer] = make(map[string]struct{}) + } + s.signerBuckets[signer][storedTx.signerKey] = struct{}{} + } } - bucket.txs = append(bucket.txs, storedTx) - slices.SortFunc(bucket.txs, compareCosmosTxWithMetadata) + bucket.txs = slices.Insert(bucket.txs, i, storedTx) s.txs[storedTx.signerKey] = bucket } @@ -240,9 +252,7 @@ func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { // bump the per-signer high-water mark for signer, nonce := range nonceMap { - if cur, exists := s.consumed[signer]; !exists || nonce > cur { - s.consumed[signer] = nonce - } + s.consumed[signer] = max(s.consumed[signer], nonce) } // drop any stored tx now under a watermark: a tx is invalid if ANY of its @@ -287,22 +297,20 @@ func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { // match returns true, deleting the bucket if it empties. Callers must hold // s.mu. Returns the number of txs removed. func (s *CosmosTxStore) filterBucketLocked(signerKey string, bucket cosmosTxBucket, match func(cosmosTxWithMetadata) bool) int { - next := bucket.txs[:0] - removed := 0 - for _, existing := range bucket.txs { - if match(existing) { - removed++ - continue - } - next = append(next, existing) - } + next := slices.DeleteFunc(bucket.txs, match) + removed := len(bucket.txs) - len(next) if removed == 0 { return 0 } - clear(bucket.txs[len(next):]) if len(next) == 0 { delete(s.txs, signerKey) + for signer := range bucket.signers { + delete(s.signerBuckets[signer], signerKey) + if len(s.signerBuckets[signer]) == 0 { + delete(s.signerBuckets, signer) + } + } return removed } bucket.txs = next @@ -310,17 +318,15 @@ func (s *CosmosTxStore) filterBucketLocked(signerKey string, bucket cosmosTxBuck return removed } -// filterSignerBucketsLocked removes every tx matching match from the buckets -// sharing at least one signer with nonceMap, skipping unrelated buckets so the -// scan stays proportional to the supplied tx rather than the whole pool. -// Callers must hold s.mu. Returns the number of txs removed. +// filterSignerBucketsLocked removes every tx matching match from the buckets sharing a signer +// with nonceMap, via signer index. Callers must hold s.mu. Returns the number of txs removed. func (s *CosmosTxStore) filterSignerBucketsLocked(nonceMap map[string]uint64, match func(cosmosTxWithMetadata) bool) int { removed := 0 - for signerKey, bucket := range s.txs { - if !bucketContainsAnySigner(bucket, nonceMap) { - continue + for signer := range nonceMap { + // repeat visits of a multi-signer bucket match nothing + for signerKey := range s.signerBuckets[signer] { + removed += s.filterBucketLocked(signerKey, s.txs[signerKey], match) } - removed += s.filterBucketLocked(signerKey, bucket, match) } return removed } @@ -425,15 +431,6 @@ func signerSetFromNonceMap(nonceMap map[string]uint64) map[string]struct{} { return signers } -func bucketContainsAnySigner(bucket cosmosTxBucket, thresholds map[string]uint64) bool { - for signer := range thresholds { - if _, ok := bucket.signers[signer]; ok { - return true - } - } - return false -} - func compareCosmosTxWithMetadata(a, b cosmosTxWithMetadata) int { if a.nonceSum < b.nonceSum { return -1 diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index c2a998651..0ae9bfe75 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -378,6 +378,19 @@ func TestCosmosTxStorePruneCommitted(t *testing.T) { require.Equal(t, 1, store.Len()) } +func TestCosmosTxStorePruneCommittedMultiSignerOnClone(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + signerA := newPubKeyBytes(t) + signerB := newPubKeyBytes(t) + store.AddTx(newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{0, 0})) + + clone := store.Clone() + require.Equal(t, 1, clone.PruneCommitted(newKeyedMockTxWithPubKey(signerA, 0))) + require.Equal(t, 0, clone.Len()) + require.Equal(t, 1, store.Len(), "pruning the clone must not touch the source") +} + // Unkeyed txs must not be carried across heights: they get a fresh key on // every AddTx and cannot be removed, so a carried copy duplicates every pass. func TestCosmosTxStoreCloneDropsUnkeyed(t *testing.T) { From 93bc0ad3d3a0ad8cfd66154927c9d2d99b3f01d5 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 19:56:55 +0800 Subject: [PATCH 07/22] fix signerExtractor --- mempool/tx_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 3d7305dd9..3579f2e84 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -76,7 +76,7 @@ func NewCosmosTxStore(l log.Logger) *CosmosTxStore { consumed: make(map[string]uint64), prevConsumed: make(map[string]uint64), logger: l, - signerExtractor: sdkmempool.NewDefaultSignerExtractionAdapter(), + signerExtractor: NewEthSignerExtractionAdapter(sdkmempool.NewDefaultSignerExtractionAdapter()), } } From 20ac6505a92e17141916e9e476063a6dbf9241dd Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 16 Jul 2026 20:21:52 +0800 Subject: [PATCH 08/22] more tests --- .../internal/heightsync/heightsync_test.go | 27 ++ mempool/recheck_pool_test.go | 263 +++++++++++++++--- mempool/tx_store_test.go | 119 ++++++++ 3 files changed, 378 insertions(+), 31 deletions(-) diff --git a/mempool/internal/heightsync/heightsync_test.go b/mempool/internal/heightsync/heightsync_test.go index 1fc3c65b3..0cd7794cb 100644 --- a/mempool/internal/heightsync/heightsync_test.go +++ b/mempool/internal/heightsync/heightsync_test.go @@ -244,6 +244,33 @@ func TestNoStaleFallbackReturnsNil(t *testing.T) { require.Nil(t, hv.GetStore(ctx, big.NewInt(2))) } +// The stale fallback must never serve a store from a height past the target: +// that is future state the target's proposal must not see, so GetStore +// returns nil instead. +func TestStaleFallbackDoesNotServePastTarget(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()).WithStaleFallback() + + // park a getter for height 2 on the height-behind wait + getCtx, cancelGet := context.WithCancel(context.Background()) + defer cancelGet() + valueChan := make(chan *testStore) + go func() { + valueChan <- hv.GetStore(getCtx, big.NewInt(2)) + }() + time.Sleep(1 * time.Second) + + // Skip to height 3, cancelling the getter while carry holds the write + // lock: it wakes on the fallback path with the height past its target. + hv.StartNewHeightFrom(big.NewInt(3), func(prev *testStore) *testStore { + cancelGet() + return prev + }) + + require.Nil(t, <-valueChan, "fallback served a store from a height past the target") + }) +} + func TestConcurrentDo(t *testing.T) { hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()) diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index a60e26b76..b32390e7e 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -19,6 +19,7 @@ import ( "google.golang.org/protobuf/proto" "github.com/cosmos/evm/crypto/ethsecp256k1" + "github.com/cosmos/evm/encoding" "github.com/cosmos/evm/mempool" "github.com/cosmos/evm/mempool/internal/heightsync" "github.com/cosmos/evm/mempool/internal/reaplist" @@ -30,6 +31,7 @@ import ( "cosmossdk.io/log/v2" sdkmath "cosmossdk.io/math" + "github.com/cosmos/cosmos-sdk/client" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" storetypes "github.com/cosmos/cosmos-sdk/store/v2/types" "github.com/cosmos/cosmos-sdk/testutil" @@ -846,6 +848,156 @@ func TestRecheckMempool_CarryForwardSurvivesCancellation(t *testing.T) { close(gate) } +func newStartedRecheckMempool( + t *testing.T, + ctx sdk.Context, + cfg *sdkmempool.PriorityNonceMempoolConfig[sdkmath.Int], + ante sdk.AnteHandler, +) (*mempool.RecheckMempool, *heightsync.HeightSync[mempool.CosmosTxStore]) { + t.Helper() + + recheckedTxs := newTestRecheckedTxs() + mp := mempool.NewRecheckMempool( + cfg, 0, reserver.NewReservationTracker().NewHandle(1), newMockRechecker(ctx, ante), + recheckedTxs, newTestReapList(), newTestBlockchain(t, ctx), log.NewNopLogger(), + ) + mp.Start(testHeader(0)) + t.Cleanup(func() { + require.NoError(t, mp.Close()) + }) + return mp, recheckedTxs +} + +func setupEVMChainConfig(t *testing.T) client.TxConfig { + t.Helper() + + vmtypes.NewEVMConfigurator().ResetTestConfig() + require.NoError(t, vmtypes.SetChainConfig(vmtypes.DefaultChainConfig(constants.EighteenDecimalsChainID))) + require.NoError(t, vmtypes.NewEVMConfigurator(). + WithEVMCoinInfo(constants.ChainsCoinInfo[constants.EighteenDecimalsChainID]). + Configure()) + + encodingConfig := encoding.MakeConfig(constants.EighteenDecimalsChainID) + vmtypes.RegisterInterfaces(encodingConfig.InterfaceRegistry) + return encodingConfig.TxConfig +} + +// snapshotAccepts reports whether the snapshot admits tx (AddTx refuses txs +// under a committed-nonce watermark), leaving the store as it found it. +func snapshotAccepts(hs *heightsync.HeightSync[mempool.CosmosTxStore], tx sdk.Tx) bool { + accepted := false + hs.Do(func(store *mempool.CosmosTxStore) { + before := store.Len() + store.AddTx(tx) + accepted = store.Len() > before + store.RemoveTx(tx) + }) + return accepted +} + +// An EVM tx consumes same sequence as its account's cosmos txs, so a committed +// one must prune them — its sender and nonce exist only in the eth payload. +func TestCosmosTxStorePruneCommittedEVMTxDropsCosmosTxs(t *testing.T) { + txConfig := setupEVMChainConfig(t) + + key, err := crypto.GenerateKey() + require.NoError(t, err) + other, err := crypto.GenerateKey() + require.NoError(t, err) + + store := mempool.NewCosmosTxStore(log.NewNopLogger()) + store.AddTx(newRecheckTestTxWithNonce(t, key, 0)) + store.AddTx(newRecheckTestTxWithNonce(t, key, 1)) + store.AddTx(newRecheckTestTxWithNonce(t, other, 0)) + require.Equal(t, 3, store.Len()) + + // committing the account's EVM tx at nonce 0 consumes that sequence + evmTx := createMsgEthereumTx(t, txConfig, key, 0, big.NewInt(1e8)) + require.Equal(t, 1, store.PruneCommitted(evmTx), + "an EVM commit must prune the same account's cosmos tx at the consumed sequence") + + // the account's later nonce and the unrelated account are both untouched + require.Equal(t, 2, store.Len()) + + // the watermark also keeps the consumed sequence from being re-added + store.AddTx(newRecheckTestTxWithNonce(t, key, 0)) + require.Equal(t, 2, store.Len()) +} + +// A watermark must age out after two completed passes: those passes revalidated +// the pool past the commit, and a mark kept longer — e.g. one left by an optimistically +// executed block that never committed — would blacklist signer's nonce forever. +func TestRecheckMempool_CompletedRecheckAgesWatermarks(t *testing.T) { + ctx := newRecheckTestContext() + mp, recheckedTxs := newStartedRecheckMempool(t, ctx, nil, noopAnteHandler) + + key, err := crypto.GenerateKey() + require.NoError(t, err) + committed := newRecheckTestTxWithNonce(t, key, 4) + + // committing nonce 4 watermarks the signer, so the snapshot refuses it + mp.PruneCommitted(committed) + require.False(t, snapshotAccepts(recheckedTxs, committed)) + + // one pass retires the mark to the older generation, where it still blocks + mp.TriggerRecheckSync(testHeader(1)) + require.False(t, snapshotAccepts(recheckedTxs, committed)) + + // the second pass retires it for good + mp.TriggerRecheckSync(testHeader(2)) + require.True(t, snapshotAccepts(recheckedTxs, committed), + "a watermark must not outlive two completed recheck passes") +} + +// A cancelled pass revalidated nothing, so it must not age watermarks: +// retiring a mark a generation early lets the snapshot re-admit a committed tx. +func TestRecheckMempool_CancelledRecheckKeepsWatermarks(t *testing.T) { + ctx := newRecheckTestContext() + + var blockPass atomic.Bool + ready := make(chan struct{}) + gate := make(chan struct{}) + anteHandler := func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + if blockPass.Load() { + ready <- struct{}{} + <-gate + } + return ctx, nil + } + + mp, recheckedTxs := newStartedRecheckMempool(t, ctx, nil, anteHandler) + + // a pooled tx gives the stalling pass something to stall on + poolKey, err := crypto.GenerateKey() + require.NoError(t, err) + require.NoError(t, mp.Insert(ctx, newRecheckTestTx(t, poolKey))) + + // an unrelated signer commits nonce 4, watermarking it + committedKey, err := crypto.GenerateKey() + require.NoError(t, err) + committed := newRecheckTestTxWithNonce(t, committedKey, 4) + mp.PruneCommitted(committed) + require.False(t, snapshotAccepts(recheckedTxs, committed)) + + // stall a height-1 pass, then let height 2 cancel it + blockPass.Store(true) + mp.TriggerRecheck(testHeader(1)) + <-ready + done := mp.TriggerRecheck(testHeader(2)) + blockPass.Store(false) + close(gate) + <-done + + // Only the height-2 pass completed, so the mark aged once and still covers + // the commit. Aging on the cancelled pass too would have retired it. + require.False(t, snapshotAccepts(recheckedTxs, committed), + "a cancelled pass must not age watermarks") + + // the next completed pass retires it + mp.TriggerRecheckSync(testHeader(3)) + require.True(t, snapshotAccepts(recheckedTxs, committed)) +} + func TestRecheckMempool_RecheckedTxsBlocksUntilComplete(t *testing.T) { acc := newRecheckTestAccount(t) tracker := reserver.NewReservationTracker() @@ -908,12 +1060,7 @@ func TestRecheckMempool_RecheckedTxsBlocksUntilComplete(t *testing.T) { } func TestRecheckMempool_RecheckerNoContextOnInsert(t *testing.T) { - // setup mocks for blockchain fetching latest block - vmtypes.NewEVMConfigurator().ResetTestConfig() - require.NoError(t, vmtypes.SetChainConfig(vmtypes.DefaultChainConfig(constants.EighteenDecimalsChainID))) - require.NoError(t, vmtypes.NewEVMConfigurator(). - WithEVMCoinInfo(constants.ChainsCoinInfo[constants.EighteenDecimalsChainID]). - Configure()) + setupEVMChainConfig(t) acc := newRecheckTestAccount(t) tracker := reserver.NewReservationTracker() @@ -946,7 +1093,7 @@ func newRecheckTestTxWithNonce(t *testing.T, key *ecdsa.PrivateKey, nonce uint64 return &recheckTestTx{key: key, sequence: nonce} } -func newRecheckTestTxWithGasPrice(t *testing.T, key *ecdsa.PrivateKey, nonce uint64, gasPrice int64) sdk.Tx { +func newRecheckTestTxWithGasPrice(t *testing.T, key *ecdsa.PrivateKey, nonce uint64, gasPrice int64) *recheckTestTx { t.Helper() return &recheckTestTx{ key: key, @@ -1143,6 +1290,36 @@ func customReplacementConfig() *sdkmempool.PriorityNonceMempoolConfig[sdkmath.In } } +// A replaced tx with a different signer set than its replacement sits in a bucket +// InvalidateFrom(newTx) never visits, only replacement hook can drop it and txs stacked on its nonces. +func TestRecheckMempool_ReplacementWithDifferentSignerSetInvalidatesRechecked(t *testing.T) { + ctx := newRecheckTestContext() + mp, _ := newStartedRecheckMempool(t, ctx, customReplacementConfig(), noopAnteHandler) + + sender, err := crypto.GenerateKey() + require.NoError(t, err) + cosigner, err := crypto.GenerateKey() + require.NoError(t, err) + + // sender's nonce 5 co-signed by a second account, plus one stacked on it + coSigned := newCoSignedRecheckTestTx(t, sender, cosigner, 5, 0, 1) + dependent := newCoSignedRecheckTestTx(t, sender, cosigner, 6, 1, 1) + require.NoError(t, mp.Insert(ctx, coSigned)) + require.NoError(t, mp.Insert(ctx, dependent)) + require.Equal(t, []sdk.Tx{coSigned, dependent}, + collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(0)))) + + // replace at the same sender and nonce with a tx sender signs alone + replacement := newRecheckTestTxWithGasPrice(t, sender, 5, 2) + require.NoError(t, mp.Insert(ctx, replacement)) + + // Only the replacement may survive: the co-signed tx left the pool, and the + // dependent was validated on a nonce the replacement now owns. + rechecked := collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(0))) + require.Equal(t, []sdk.Tx{replacement}, rechecked, + "the replaced tx and its dependent must not linger in the snapshot") +} + func TestRecheckMempool_RecheckRebuildsSnapshotAfterReplacement(t *testing.T) { ctx := newRecheckTestContext() tracker := reserver.NewReservationTracker() @@ -1188,9 +1365,8 @@ func TestRecheckMempool_RecheckRebuildsSnapshotAfterReplacement(t *testing.T) { require.Equal(t, []sdk.Tx{tx3, replacement, tx5, tx6}, rechecked) } -// TestRecheckMempool_RecheckDropsFromReapList verifies that when a tx fails -// recheck and gets removed from the pool, it is also dropped from the reap -// list. Txs that pass recheck must remain reapable. +// To verify that when a tx fails recheck and gets removed from the pool, it is also dropped +// from reap list. Txs that pass recheck must remain reapable. func TestRecheckMempool_RecheckDropsFromReapList(t *testing.T) { ctx := newRecheckTestContext() tracker := reserver.NewReservationTracker() @@ -1278,18 +1454,30 @@ func TestRecheckMempool_ReplacementDropsFromReapList(t *testing.T) { require.Equal(t, expected, reaped[0]) } -// newRecheckTestTx creates a minimal sdk.Tx for unit testing RecheckMempool. func newRecheckTestTx(t *testing.T, key *ecdsa.PrivateKey) sdk.Tx { t.Helper() return &recheckTestTx{key: key} } -// recheckTestTx is a minimal sdk.Tx implementation for unit testing. type recheckTestTx struct { + key *ecdsa.PrivateKey + sequence uint64 + gas uint64 + fee sdk.Coins + cosigners []recheckTestSigner +} + +type recheckTestSigner struct { key *ecdsa.PrivateKey sequence uint64 - gas uint64 - fee sdk.Coins +} + +func (m *recheckTestTx) signers() []recheckTestSigner { + return append([]recheckTestSigner{{key: m.key, sequence: m.sequence}}, m.cosigners...) +} + +func recheckTestPubKey(key *ecdsa.PrivateKey) cryptotypes.PubKey { + return ðsecp256k1.PubKey{Key: crypto.CompressPubkey(&key.PublicKey)} } const recheckTestFeeDenom = "atest" @@ -1327,29 +1515,44 @@ func (m *recheckTestTx) FeeGranter() []byte { } func (m *recheckTestTx) GetSigners() ([][]byte, error) { - pubKeyBytes := crypto.CompressPubkey(&m.key.PublicKey) - pubKey := ðsecp256k1.PubKey{Key: pubKeyBytes} - return [][]byte{pubKey.Address().Bytes()}, nil + signers := make([][]byte, 0, len(m.cosigners)+1) + for _, s := range m.signers() { + signers = append(signers, recheckTestPubKey(s.key).Address().Bytes()) + } + return signers, nil } func (m *recheckTestTx) GetPubKeys() ([]cryptotypes.PubKey, error) { - pubKeyBytes := crypto.CompressPubkey(&m.key.PublicKey) - pubKey := ðsecp256k1.PubKey{Key: pubKeyBytes} - return []cryptotypes.PubKey{pubKey}, nil + pubKeys := make([]cryptotypes.PubKey, 0, len(m.cosigners)+1) + for _, s := range m.signers() { + pubKeys = append(pubKeys, recheckTestPubKey(s.key)) + } + return pubKeys, nil } func (m *recheckTestTx) GetSignaturesV2() ([]signingtypes.SignatureV2, error) { - pubKeyBytes := crypto.CompressPubkey(&m.key.PublicKey) - pubKey := ðsecp256k1.PubKey{Key: pubKeyBytes} - return []signingtypes.SignatureV2{ - { - PubKey: pubKey, - Sequence: m.sequence, - }, - }, nil + sigs := make([]signingtypes.SignatureV2, 0, len(m.cosigners)+1) + for _, s := range m.signers() { + sigs = append(sigs, signingtypes.SignatureV2{ + PubKey: recheckTestPubKey(s.key), + Sequence: s.sequence, + }) + } + return sigs, nil +} + +func newCoSignedRecheckTestTx( + t *testing.T, + key, cosigner *ecdsa.PrivateKey, + nonce, cosignerNonce uint64, + gasPrice int64, +) sdk.Tx { + t.Helper() + tx := newRecheckTestTxWithGasPrice(t, key, nonce, gasPrice) + tx.cosigners = []recheckTestSigner{{key: cosigner, sequence: cosignerNonce}} + return tx } -// recheckTestAccount holds test account data. type recheckTestAccount struct { key *ecdsa.PrivateKey address common.Address @@ -1373,12 +1576,10 @@ func noopAnteHandler(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil } -// newTestRecheckedTxs creates a HeightSync[CosmosTxStore] for testing, starting at height 0. func newTestRecheckedTxs() *heightsync.HeightSync[mempool.CosmosTxStore] { return heightsync.New(big.NewInt(0), mempool.NewCosmosTxStore, log.NewNopLogger()) } -// collectIteratorTxs drains an sdkmempool.Iterator into a slice. func collectIteratorTxs(iter sdkmempool.Iterator) []sdk.Tx { var txs []sdk.Tx for iter != nil { diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index 0ae9bfe75..742dfff10 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -1,6 +1,7 @@ package mempool import ( + "slices" "testing" "time" @@ -279,6 +280,30 @@ func newPubKeyBytes(t *testing.T) []byte { return crypto.CompressPubkey(&key.PublicKey) } +// signerKeyOf returns the key the store indexes a signer under: its account +// address, not the pubkey bytes the mocks are constructed from. +func signerKeyOf(pubKeyBytes []byte) string { + return string((ðsecp256k1.PubKey{Key: pubKeyBytes}).Address().Bytes()) +} + +// requireSignerIndexConsistent rebuilds the signer index from the buckets and +// compares: a missing entry hides a bucket from shared-signer scans, a stale +// one leaks. +func requireSignerIndexConsistent(t *testing.T, store *CosmosTxStore) { + t.Helper() + + want := make(map[string]map[string]struct{}) + for signerKey, bucket := range store.txs { + for signer := range bucket.signers { + if want[signer] == nil { + want[signer] = make(map[string]struct{}) + } + want[signer][signerKey] = struct{}{} + } + } + require.Equal(t, want, store.signerBuckets) +} + func TestCosmosTxStoreRemoveTx(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) @@ -408,6 +433,100 @@ func TestCosmosTxStoreCloneDropsUnkeyed(t *testing.T) { require.Equal(t, 2, clone.Len()) } +// The signer index must name every bucket a signer sits in and nothing more: +// it decides which buckets a shared-signer scan visits. +func TestCosmosTxStoreSignerIndexTracksBuckets(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + pubA, pubB := newPubKeyBytes(t), newPubKeyBytes(t) + keyA, keyB := signerKeyOf(pubA), signerKeyOf(pubB) + + store.AddTx(newKeyedMockTxWithPubKey(pubA, 0)) + store.AddTx(newKeyedMockTxWithPubKey(pubA, 1)) + store.AddTx(newKeyedMockTxWithPubKey(pubB, 0)) + store.AddTx(newMultiKeyedMockTx([][]byte{pubA, pubB}, []uint64{9, 9})) + store.AddTx(newMockTx(1)) // unkeyed: no signers, so it earns no index entry + require.Equal(t, 5, store.Len()) + requireSignerIndexConsistent(t, store) + + // each signer sits in its own bucket and in the one they share + require.Len(t, store.signerBuckets[keyA], 2) + require.Len(t, store.signerBuckets[keyB], 2) + + // Committing A's nonce 9 empties both of A's buckets: its own (nonces 0 and + // 1 fall under the watermark) and the shared one. B's own bucket is a + // different signer set and survives untouched. + require.Equal(t, 3, store.PruneCommitted(newKeyedMockTxWithPubKey(pubA, 9))) + requireSignerIndexConsistent(t, store) + require.NotContains(t, store.signerBuckets, keyA, "A has no bucket left to scan") + require.Len(t, store.signerBuckets[keyB], 1) + + // the index must not outlive the last keyed tx + require.True(t, store.RemoveTx(newKeyedMockTxWithPubKey(pubB, 0))) + requireSignerIndexConsistent(t, store) + require.Empty(t, store.signerBuckets) + require.Equal(t, 1, store.Len(), "the unkeyed tx is untouched by signer scans") +} + +// A clone's per-signer bucket sets must be copies: sharing them would let a +// prune on one height's store reach back into the previous height's. +func TestCosmosTxStoreCloneSignerIndexIsIndependent(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + pubA, pubB := newPubKeyBytes(t), newPubKeyBytes(t) + store.AddTx(newMultiKeyedMockTx([][]byte{pubA, pubB}, []uint64{0, 0})) + store.AddTx(newKeyedMockTxWithPubKey(pubA, 3)) + store.AddTx(newMockTx(1)) // unkeyed: dropped by Clone, and indexed by neither + + clone := store.Clone() + requireSignerIndexConsistent(t, clone) + + // emptying every bucket the clone has must leave the source's index whole + require.Equal(t, 2, clone.PruneCommitted(newKeyedMockTxWithPubKey(pubA, 3))) + require.Equal(t, 0, clone.Len()) + require.Empty(t, clone.signerBuckets) + requireSignerIndexConsistent(t, store) + require.Len(t, store.signerBuckets[signerKeyOf(pubA)], 2) + require.Len(t, store.signerBuckets[signerKeyOf(pubB)], 1) +} + +// Two txs of one signer set can share a nonce sum; only AddTx's txKey +// tie-break keeps the second from overwriting the first. +func TestCosmosTxStoreAddTxDistinguishesEqualNonceSums(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + pubA, pubB := newPubKeyBytes(t), newPubKeyBytes(t) + first := newMultiKeyedMockTx([][]byte{pubA, pubB}, []uint64{5, 0}) + second := newMultiKeyedMockTx([][]byte{pubA, pubB}, []uint64{0, 5}) + + store.AddTx(first) + store.AddTx(second) + require.Equal(t, 2, store.Len(), "equal nonce sums must not collapse into one slot") + require.ElementsMatch(t, []sdk.Tx{first, second}, store.Txs()) + requireSignerIndexConsistent(t, store) + + // a recheck pass re-adds both: each overwrites its own slot, no duplicates + store.AddTx(first) + store.AddTx(second) + require.Equal(t, 2, store.Len()) + require.ElementsMatch(t, []sdk.Tx{first, second}, store.Txs()) +} + +// Unkeyed store keys compare as strings ("unkeyed/10" < "unkeyed/9"), and +// AddTx's binary search relies on inserts landing in that same order. +func TestCosmosTxStoreUnkeyedInsertsStaySortedPastTen(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + const count = 12 + for i := range count { + store.AddTx(newMockTx(i)) + } + + require.Equal(t, count, store.Len(), "every unkeyed tx gets its own slot") + require.True(t, slices.IsSortedFunc(store.txs[unkeyedSignerKey].txs, compareCosmosTxWithMetadata)) + requireSignerIndexConsistent(t, store) +} + // A replaced multi-signer tx lives in a bucket InvalidateFrom(newTx) cannot // see; InvalidateReplaced drops it (and its dependents) by its own identity. func TestCosmosTxStoreInvalidateReplaced(t *testing.T) { From 81b89167431df15883c5ac121af2ef346a1ca94a Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 3 Aug 2026 10:32:56 +0800 Subject: [PATCH 09/22] return nil instead of panick when height sync skips past target --- mempool/internal/heightsync/heightsync.go | 16 ++++++++++++---- mempool/internal/heightsync/heightsync_test.go | 6 ++---- mempool/recheck_pool.go | 2 +- mempool/tx_store.go | 4 ++++ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index e36331b1f..f82dc24e2 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -222,6 +222,7 @@ func (hs *HeightSync[Store]) isHeightEnded() bool { // reached the target height, GetStore blocks until the height is reached or the // context expires. If the height is reached, GetStore waits for EndCurrentHeight // to be called (or for the context to expire) before returning. +// If HeightSync has already moved past the target height, GetStore returns nil. func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Store { genesis := big.NewInt(0) @@ -235,11 +236,18 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto cmp := hs.currentHeight.Cmp(height) - // should never see a situation where the HeightSync is ahead of - // the caller + // Heights can skip past the target when producer triggers coalesce + // under backlog. Target's Store is gone and future state must not be + // served, so return nil. if cmp > 0 { - defer hs.mu.RUnlock() // defer unlock since the panic will read - panic(fmt.Errorf("HeightSync.Get called for height %d, but current height is %d; cannot serve requests in the past", height, hs.currentHeight)) + current := hs.currentHeight.String() + hs.mu.RUnlock() + hs.logger.Warn( + "height sync moved past requested height, no store to serve", + "requested_height", height.String(), + "current_height", current, + ) + return nil } // if we're at the target height, wait for completion or timeout diff --git a/mempool/internal/heightsync/heightsync_test.go b/mempool/internal/heightsync/heightsync_test.go index 0cd7794cb..dc28b824f 100644 --- a/mempool/internal/heightsync/heightsync_test.go +++ b/mempool/internal/heightsync/heightsync_test.go @@ -156,7 +156,7 @@ func TestGetBehindByTwoHeights(t *testing.T) { }) } -func TestPanicOnOldHeight(t *testing.T) { +func TestNilOnOldHeight(t *testing.T) { hv := heightsync.New(big.NewInt(1), newTestValue, log.NewNopLogger()) hv.StartNewHeight(big.NewInt(1)) @@ -166,9 +166,7 @@ func TestPanicOnOldHeight(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - require.Panics(t, func() { - hv.GetStore(ctx, big.NewInt(1)) - }) + require.Nil(t, hv.GetStore(ctx, big.NewInt(1))) } func TestStartNewHeightResetsValue(t *testing.T) { diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 9ab8288c5..d9139a244 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -329,7 +329,7 @@ func (m *RecheckMempool) TriggerRecheckSync(newHead *ethtypes.Header) { // RecheckedTxs returns the txs that have been rechecked for a height. The // RecheckMempool must be currently operating on this height (i.e. recheck has // been triggered on this height via TriggerRecheck). If height is in the past -// (TriggerRecheck has been called on height + 1), this will panic. If height +// (TriggerRecheck has been called on a later height), this returns nil. If height // is in the future, this will block until TriggerReset is called for height, // or the context times out. func (m *RecheckMempool) RecheckedTxs(ctx context.Context, height *big.Int) sdkmempool.Iterator { diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 3579f2e84..e9563fbea 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -234,6 +234,10 @@ func (s *CosmosTxStore) removeTxKeyLocked(signerKey, txKey string) int { // called synchronously as a block is finalized so the carried-forward store can // never feed an already-committed tx into a later proposal, even before the // next recheck pass runs. Returns the number of stored txs pruned. +// +// baseapp removes a block tx with CallerRunTxFinalize only after ante +// succeeds, which consumes the signers' sequences even if message execution +// later fails — so every tx reaching here is safe to watermark. func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { s.mu.Lock() defer s.mu.Unlock() From 0db110f4f8332b0e5f69b110e74990467144905e Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 09:49:25 +0800 Subject: [PATCH 10/22] verify proposal txs against latest state * a carried-forward snapshot is validated at a height <= the one being proposed, so evmd can no longer assume every tx the mempool selects is still valid. * drop NoCheckProposalTxVerifier and let BaseApp's default verifier run ante over each selected tx, cost is bounded by proposal's gas budget, not pool size. --- CHANGELOG.md | 2 +- evmd/mempool.go | 7 ++++-- evmd/tx_verifier.go | 27 ----------------------- mempool/internal/heightsync/heightsync.go | 17 ++++++-------- 4 files changed, 13 insertions(+), 40 deletions(-) delete mode 100644 evmd/tx_verifier.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 47c4a7bac..346c89e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,7 +71,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. -- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve last completed snapshot when recheck loop falls behind a proposal. +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now verifies proposal txs with BaseApp's default verifier, since a carried-forward snapshot no longer guarantees every selected tx is valid at the proposed height. ## v0.6.0 diff --git a/evmd/mempool.go b/evmd/mempool.go index 55986ce2a..10f6408ac 100644 --- a/evmd/mempool.go +++ b/evmd/mempool.go @@ -53,9 +53,12 @@ func (app *EVMD) configureEVMMempool(appOpts servertypes.AppOptions, logger log. app.EVMMempool = mempool - // create ABCI handlers + // Verify every selected tx with BaseApp's default verifier rather than + // trusting the mempool: under backlog the cosmos pool serves a snapshot + // validated at an earlier height, so a tx in it may since have become + // invalid. Cost is bounded by the proposal's gas budget, not the pool size. prepareProposalHandler := baseapp. - NewDefaultProposalHandler(mempool, NewNoCheckProposalTxVerifier(app.BaseApp)). + NewDefaultProposalHandler(mempool, app.BaseApp). PrepareProposalHandler() insertTxHandler := mempool.NewInsertTxHandler(app.TxDecode) diff --git a/evmd/tx_verifier.go b/evmd/tx_verifier.go deleted file mode 100644 index 224d3abe6..000000000 --- a/evmd/tx_verifier.go +++ /dev/null @@ -1,27 +0,0 @@ -package evmd - -import ( - "github.com/cosmos/cosmos-sdk/baseapp" - sdk "github.com/cosmos/cosmos-sdk/types" -) - -var _ baseapp.ProposalTxVerifier = &NoCheckProposalTxVerifier{} - -type NoCheckProposalTxVerifier struct { - *baseapp.BaseApp -} - -func NewNoCheckProposalTxVerifier(b *baseapp.BaseApp) *NoCheckProposalTxVerifier { - return &NoCheckProposalTxVerifier{BaseApp: b} -} - -// PrepareProposalVerifyTx overrides the typical tx verification done in -// BaseApp's PrepareProposalHandler. The default PrepareProposalVerifyTx -// implementation encodes the tx to bytes, then calls runTx in 'checktx' mode, -// executing all antehandlers. -// -// We now override the implementation to only verify that the tx can be encoded -// to bytes, since we will guarantee that all txs selected are valid elsewhere. -func (txv *NoCheckProposalTxVerifier) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { - return txv.TxEncode(tx) -} diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index f82dc24e2..0ce14f5c4 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -118,11 +118,11 @@ type HeightSync[Store any] struct { // staleFallback makes GetStore return the current carried-forward Store // (instead of nil) when it times out while still behind the target height. - // That Store is at a height <= target and, even mid-recheck, carry-forward - // keeps it a valid subset of validated txs. Only enable it for stores that - // stay free of state a committed block invalidated (see the cosmos pool's - // committed-nonce watermark), otherwise a stale Store could serve - // already-committed txs. + // Entries in it were validated at a height <= target, so enabling it needs + // both: producers keep already-committed entries out (the cosmos pool's + // committed-nonce watermark), and consumers re-verify anything that must + // hold against latest state, since a later block can have invalidated an + // entry for some other reason (evmd's PrepareProposal runs ante). staleFallback bool logger log.Logger @@ -295,11 +295,8 @@ func (hs *HeightSync[Store]) GetStore(ctx context.Context, height *big.Int) *Sto return nil } // Rather than starve the caller (e.g. an empty block proposal), - // fall back to the current carried-forward Store: it is at a - // height <= target and, even mid-recheck, holds a valid subset of - // validated txs. Safe to serve as long as producers keep it free - // of state a since-committed block invalidated (the cosmos pool - // does this via its committed-nonce watermark). + // fall back to the carried-forward Store, under the staleFallback + // contract above. hs.mu.RLock() value := hs.store // heights can skip past target while we waited, never serve future state From 53ef4db328bd7af5bfa860d959b966043054e57c Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 09:52:37 +0800 Subject: [PATCH 11/22] reproduce proposal starvation via real ABCI path --- .../mempool/test_mempool_integration_abci.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/integration/mempool/test_mempool_integration_abci.go b/tests/integration/mempool/test_mempool_integration_abci.go index ef5354587..6555773bd 100644 --- a/tests/integration/mempool/test_mempool_integration_abci.go +++ b/tests/integration/mempool/test_mempool_integration_abci.go @@ -1096,3 +1096,52 @@ func (s *IntegrationTestSuite) TestMultiPoolInteractions() { }) } } + +// TestProposalStarvationWhenRecheckLagsHeight checks that a proposal one height +// ahead of last recheck still draws from the pool, the situation a backlog +// creates by cancelling every recheck pass. +func (s *IntegrationTestSuite) TestProposalStarvationWhenRecheckLagsHeight() { + const numTxs = 8 + + s.TearDownTest() + s.SetupTest() + + mpool := s.network.App.GetMempool() + kMp, ok := mpool.(*evmmempool.Mempool) + if !ok { + s.T().Skip("EVM mempool not configured") + } + + // A backlog of cosmos txs, one per signer so none depend on another. + txs := make([]sdk.Tx, 0, numTxs) + for i := range numTxs { + txs = append(txs, s.createCosmosSendTx(s.keyring.GetKey(i), big.NewInt(1000000000))) + } + s.Require().NoError(s.insertTxs(txs)) + + // One full recheck pass at the current head, validating against real state. + head := kMp.GetBlockchain().CurrentBlock() + kMp.RecheckCosmosTxs(head) + + _, err := s.network.FinalizeBlock() + s.Require().NoError(err) + + proposedTxCount := func(height int64) int { + res, err := s.network.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 1_000_000, + Height: height, + }) + s.Require().NoError(err) + return len(res.Txs) + } + + // Control: the height the snapshot was built for proposes everything. + current := s.network.GetContext().BlockHeight() + s.Require().Equal(numTxs, proposedTxCount(current+1), + "in-sync proposal should carry the whole validated backlog") + + // One height further on, which no recheck pass has reached. Pre-fix this + // served nothing and block came out empty. + s.Require().Equal(numTxs, proposedTxCount(current+2), + "proposal starved: the pool holds a validated backlog but served nothing") +} From a635e215f59b161bfde3dd2807ef8c02a2ea7037 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 10:02:35 +0800 Subject: [PATCH 12/22] skip proposal re-verification for txs validated at head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * stamp snapshot entries with the height their ante validation ran against: a pass stamps what it re-adds, carried entries keep the stamp of the pass that validated them. * byTx indexes stored pointer-typed txs straight to their stamp, so ValidatedAt is one map hit and a pass re-adding a known tx only refreshes its stamp — no signer extraction (cgo pubkey decompression) or fmt-built keys per candidate. Removals and slot overwrites unindex, so a replaced or pruned tx never keeps vouching. * evmd's SnapshotVerifiedTxVerifier encodes entries stamped at head and runs full ante only for carried ones — exactly the txs that may have become invalid. Head comes from an atomic mirror of the pinned context's height. * 200-tx proposal: 1.0ms encode-only (main), 16ms unconditional ante, 0.93–1.10ms with this verifier in the steady state. --- CHANGELOG.md | 2 +- evmd/mempool.go | 10 ++-- evmd/tx_verifier.go | 32 ++++++++++++ mempool/blockchain.go | 15 ++++++ mempool/mempool.go | 22 ++++++++ mempool/recheck_pool.go | 11 ++++ mempool/tx_store.go | 101 +++++++++++++++++++++++++++++++++++-- mempool/tx_store_test.go | 106 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 evmd/tx_verifier.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 346c89e0c..5da30a105 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,7 +71,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. -- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now verifies proposal txs with BaseApp's default verifier, since a carried-forward snapshot no longer guarantees every selected tx is valid at the proposed height. +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now re-verifies proposal txs whose snapshot entry was validated at an earlier height (a carried-forward snapshot no longer guarantees validity at the proposed height); entries validated at the head skip re-verification, keeping steady-state proposal cost unchanged. ## v0.6.0 diff --git a/evmd/mempool.go b/evmd/mempool.go index 10f6408ac..27ee252ec 100644 --- a/evmd/mempool.go +++ b/evmd/mempool.go @@ -53,12 +53,12 @@ func (app *EVMD) configureEVMMempool(appOpts servertypes.AppOptions, logger log. app.EVMMempool = mempool - // Verify every selected tx with BaseApp's default verifier rather than - // trusting the mempool: under backlog the cosmos pool serves a snapshot - // validated at an earlier height, so a tx in it may since have become - // invalid. Cost is bounded by the proposal's gas budget, not the pool size. + // Under backlog the cosmos pool serves a snapshot validated at an earlier + // height, so a selected tx may since have become invalid; the verifier + // re-runs ante for exactly those txs and skips it for entries the mempool + // proves were validated at the head (see SnapshotVerifiedTxVerifier). prepareProposalHandler := baseapp. - NewDefaultProposalHandler(mempool, app.BaseApp). + NewDefaultProposalHandler(mempool, NewSnapshotVerifiedTxVerifier(app.BaseApp, mempool)). PrepareProposalHandler() insertTxHandler := mempool.NewInsertTxHandler(app.TxDecode) diff --git a/evmd/tx_verifier.go b/evmd/tx_verifier.go new file mode 100644 index 000000000..ee68d6afa --- /dev/null +++ b/evmd/tx_verifier.go @@ -0,0 +1,32 @@ +package evmd + +import ( + evmmempool "github.com/cosmos/evm/mempool" + + "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +var _ baseapp.ProposalTxVerifier = &SnapshotVerifiedTxVerifier{} + +// SnapshotVerifiedTxVerifier re-runs ante over a proposal candidate only when +// the mempool cannot show it was already validated at the current head — +// under backlog the cosmos pool serves a snapshot carried from an earlier +// height, and only those carried entries may since have become invalid. +type SnapshotVerifiedTxVerifier struct { + *baseapp.BaseApp + mempool *evmmempool.Mempool +} + +func NewSnapshotVerifiedTxVerifier(b *baseapp.BaseApp, mempool *evmmempool.Mempool) *SnapshotVerifiedTxVerifier { + return &SnapshotVerifiedTxVerifier{BaseApp: b, mempool: mempool} +} + +// PrepareProposalVerifyTx encodes txs validated at the head height and defers +// to BaseApp's full ante verification for stale or unknown ones. +func (txv *SnapshotVerifiedTxVerifier) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { + if txv.mempool.ProposalTxValidatedAtHead(tx) { + return txv.TxEncode(tx) + } + return txv.BaseApp.PrepareProposalVerifyTx(tx) +} diff --git a/mempool/blockchain.go b/mempool/blockchain.go index 2c49042d5..bef25e3b6 100644 --- a/mempool/blockchain.go +++ b/mempool/blockchain.go @@ -47,6 +47,10 @@ type Blockchain struct { mu sync.RWMutex coinInfo atomic.Pointer[evmtypes.EvmCoinInfo] + // latestHeight mirrors latestCtx's block height (0 when unset), so hot + // paths can read the pinned height without copying a full sdk.Context. + latestHeight atomic.Int64 + testingCommitMu sync.RWMutex } @@ -259,6 +263,17 @@ func (b *Blockchain) setLatestContext(ctx sdk.Context) { b.mu.Lock() defer b.mu.Unlock() b.latestCtx = ctx + if ctx.Context() == nil { + b.latestHeight.Store(0) + } else { + b.latestHeight.Store(ctx.BlockHeight()) + } +} + +// LatestHeight returns the pinned context's block height, or 0 when no +// context is pinned yet. +func (b *Blockchain) LatestHeight() int64 { + return b.latestHeight.Load() } // GetLatestContext returns the latest context as updated by the block, diff --git a/mempool/mempool.go b/mempool/mempool.go index 7dbfc7a05..1fdc6bdbf 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -663,6 +663,28 @@ func (m *Mempool) cosmosIterator( return m.recheckCosmosPool.OrderedRecheckedTxs(ctx, height, bondDenom, baseFee) } +// ProposalTxValidatedAtHead reports whether the mempool's snapshot copy of tx +// was ante-validated at the head height, i.e. re-running ante for a proposal +// could not learn anything new. EVM txs always qualify (their snapshot has no +// stale fallback); a cosmos tx qualifies only when its stamp matches the head, +// so carried or unknown txs report false and must be re-verified. +// +// Head mirrors the pinned context's height. If the notify path dies the pin +// and the stamps freeze together; committed txs are still excluded by the +// watermark, which is fed by FinalizeBlock removals. +func (m *Mempool) ProposalTxValidatedAtHead(tx sdk.Tx) bool { + if _, err := evmTxFromCosmosTx(tx); err == nil { + return true + } + head := m.blockchain.LatestHeight() + if head <= 0 { + // no pinned context yet, nothing can prove validation at head + return false + } + height, ok := m.recheckCosmosPool.SnapshotValidatedAt(tx) + return ok && height == uint64(head) +} + // TrackTx submits a tx to be tracked for its tx inclusion metrics. func (m *Mempool) TrackTx(hash common.Hash) error { return m.txTracker.Track(hash) diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index d9139a244..197e8c2e5 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -443,6 +443,10 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } m.rechecker.Update(latestCtx, newHead) + // stamp the snapshot only once validation actually runs against this + // height's state (see CosmosTxStore.SetHeight) + m.recheckedTxs.Do(func(store *CosmosTxStore) { store.SetHeight(newHead.Number.Uint64()) }) + failedAtSequence := make(map[string]uint64) removeTxs := make([]sdk.Tx, 0) @@ -547,6 +551,13 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header m.recheckedTxs.Do(func(store *CosmosTxStore) { store.AgeWatermarks() }) } +// SnapshotValidatedAt reports the height the current snapshot's copy of txn +// was validated at, and false if the snapshot does not hold that exact tx. +func (m *RecheckMempool) SnapshotValidatedAt(txn sdk.Tx) (height uint64, ok bool) { + m.recheckedTxs.Do(func(store *CosmosTxStore) { height, ok = store.ValidatedAt(txn) }) + return height, ok +} + // markTxRechecked adds a tx into the height synced cosmos tx store. func (m *RecheckMempool) markTxRechecked(txn sdk.Tx) { m.recheckedTxs.Do(func(store *CosmosTxStore) { store.AddTx(txn) }) diff --git a/mempool/tx_store.go b/mempool/tx_store.go index e9563fbea..c3f754b30 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -2,8 +2,8 @@ package mempool import ( "context" - "fmt" "maps" + "reflect" "slices" "strconv" "strings" @@ -50,6 +50,18 @@ type CosmosTxStore struct { consumed map[string]uint64 prevConsumed map[string]uint64 + // height is the chain height the snapshot is being (re)built for (see + // SetHeight). AddTx stamps entries with it; carried entries keep the stamp + // of the pass that validated them (see ValidatedAt). + height uint64 + + // byTx indexes stored pointer-typed txs to their validatedAt stamp, so + // ValidatedAt and AddTx re-adds skip signer extraction and key building. + // A same-nonce replacement is a different object, so it never vouches for + // the tx it replaced. Non-pointer txs are not indexed (interface map keys + // must be comparable) and take the slow paths. + byTx map[sdk.Tx]uint64 + logger log.Logger signerExtractor sdkmempool.SignerExtractionAdapter mu sync.RWMutex @@ -75,6 +87,7 @@ func NewCosmosTxStore(l log.Logger) *CosmosTxStore { signerBuckets: make(map[string]map[string]struct{}), consumed: make(map[string]uint64), prevConsumed: make(map[string]uint64), + byTx: make(map[sdk.Tx]uint64), logger: l, signerExtractor: NewEthSignerExtractionAdapter(sdkmempool.NewDefaultSignerExtractionAdapter()), } @@ -92,7 +105,9 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { signerBuckets: make(map[string]map[string]struct{}, len(s.signerBuckets)), consumed: maps.Clone(s.consumed), prevConsumed: maps.Clone(s.prevConsumed), + byTx: maps.Clone(s.byTx), nextUnkeyed: s.nextUnkeyed, + height: s.height, logger: s.logger, signerExtractor: s.signerExtractor, } @@ -113,11 +128,52 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { return clone } +// SetHeight records the chain height the snapshot is being (re)built for. +// Each pass calls it once, after the rechecker context moves to that height's +// state, so AddTx stamps entries with the height their validation ran against. +func (s *CosmosTxStore) SetHeight(height uint64) { + s.mu.Lock() + defer s.mu.Unlock() + s.height = height +} + +// ValidatedAt returns the height the stored copy of tx was last validated at. +// The entry must be the same tx object — a same-nonce replacement does not +// vouch for the tx it replaced — so replaced or absent txs report false. +func (s *CosmosTxStore) ValidatedAt(tx sdk.Tx) (uint64, bool) { + if !isPointerTx(tx) { + return 0, false + } + s.mu.RLock() + defer s.mu.RUnlock() + height, ok := s.byTx[tx] + return height, ok +} + +// isPointerTx reports whether tx's dynamic type is a pointer. Only pointer +// txs enter byTx (the mempool pipeline shares one decoded object per tx, and +// interface map keys must be comparable); anything else takes the slow paths. +func isPointerTx(tx sdk.Tx) bool { + return reflect.ValueOf(tx).Kind() == reflect.Pointer +} + // AddTx adds a single tx to the store while constructing a validated snapshot. func (s *CosmosTxStore) AddTx(tx sdk.Tx) { s.mu.Lock() defer s.mu.Unlock() + indexable := isPointerTx(tx) + + // Fast path: re-adding a known tx only refreshes its stamp — its bucket + // entry is already correct, and its presence proves it survived every + // watermark prune. + if indexable { + if _, ok := s.byTx[tx]; ok { + s.byTx[tx] = s.height + return + } + } + storedTx := s.newCosmosTxWithMetadata(tx) // Reject txs whose nonce a committed block already consumed. This guards the @@ -134,12 +190,25 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { storedTx.txKey = s.newUnkeyedStoreKey() } + // unkeyed txs are unremovable, so they are never indexed + if storedTx.signerKey == unkeyedSignerKey { + indexable = false + } + // bucket.txs is sorted by (nonceSum, txKey): overwrite an occupied slot — // each recheck pass re-adds still-valid txs and the newest wins. bucket := s.txs[storedTx.signerKey] i, found := slices.BinarySearchFunc(bucket.txs, storedTx, compareCosmosTxWithMetadata) if found { + // a replacement occupies the replaced tx's slot; its stamp must not + // vouch for the tx it replaced + if old := bucket.txs[i].tx; isPointerTx(old) { + delete(s.byTx, old) + } bucket.txs[i] = storedTx + if indexable { + s.byTx[tx] = s.height + } return } @@ -154,6 +223,9 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { } bucket.txs = slices.Insert(bucket.txs, i, storedTx) s.txs[storedTx.signerKey] = bucket + if indexable { + s.byTx[tx] = s.height + } } // InvalidateFrom removes any stored tx that depends on the supplied tx's signer/nonces. @@ -298,10 +370,19 @@ func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { } // filterBucketLocked removes every tx in the bucket at signerKey for which -// match returns true, deleting the bucket if it empties. Callers must hold -// s.mu. Returns the number of txs removed. +// match returns true (dropping it from the byTx index too), deleting the +// bucket if it empties. Callers must hold s.mu. Returns the number of txs +// removed. func (s *CosmosTxStore) filterBucketLocked(signerKey string, bucket cosmosTxBucket, match func(cosmosTxWithMetadata) bool) int { - next := slices.DeleteFunc(bucket.txs, match) + next := slices.DeleteFunc(bucket.txs, func(t cosmosTxWithMetadata) bool { + if !match(t) { + return false + } + if isPointerTx(t.tx) { + delete(s.byTx, t.tx) + } + return true + }) removed := len(bucket.txs) - len(next) if removed == 0 { return 0 @@ -364,13 +445,23 @@ func cosmosTxSignerSetKey(nonceMap map[string]uint64) string { return b.String() } +// txKeyZeroPad left-pads nonces to the width of MaxUint64 so the string +// ordering of keys matches numeric nonce ordering. +const txKeyZeroPad = "00000000000000000000" + func cosmosTxKey(nonceMap map[string]uint64) string { var b strings.Builder for i, k := range sortedSignerKeys(nonceMap) { if i > 0 { b.WriteByte('|') } - fmt.Fprintf(&b, "%s/%020d", k, nonceMap[k]) + // equivalent to fmt.Fprintf(&b, "%s/%020d", ...) without fmt's + // reflection; this runs for every tx added on every recheck pass + nonce := strconv.FormatUint(nonceMap[k], 10) + b.WriteString(k) + b.WriteByte('/') + b.WriteString(txKeyZeroPad[:len(txKeyZeroPad)-len(nonce)]) + b.WriteString(nonce) } return b.String() diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index 742dfff10..12f94667f 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -763,3 +763,109 @@ func TestCosmosTxStoreInvalidateFromMultiSignerEvictsSingleSigner(t *testing.T) require.ElementsMatch(t, []sdk.Tx{bobTx3, eveTx9}, store.Txs()) } + +func TestCosmosTxStoreValidatedAtTracksHeights(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + tx := newKeyedMockTx(t, 5) + store.SetHeight(7) + store.AddTx(tx) + + // stamped with the height the pass validated it at + height, ok := store.ValidatedAt(tx) + require.True(t, ok) + require.Equal(t, uint64(7), height) + + // a carried clone keeps the stamp of the pass that validated the entry... + clone := store.Clone() + clone.SetHeight(8) + height, ok = clone.ValidatedAt(tx) + require.True(t, ok) + require.Equal(t, uint64(7), height) + + // ...until its pass re-adds the tx, which re-stamps it + clone.AddTx(tx) + height, ok = clone.ValidatedAt(tx) + require.True(t, ok) + require.Equal(t, uint64(8), height) + + // the source store is unaffected by the clone's re-stamp + height, ok = store.ValidatedAt(tx) + require.True(t, ok) + require.Equal(t, uint64(7), height) + + // an absent tx does not resolve + _, ok = clone.ValidatedAt(newKeyedMockTx(t, 6)) + require.False(t, ok) +} + +// A same-signer same-nonce replacement occupies the replaced tx's slot; its +// stamp must not vouch for the tx it replaced. +func TestCosmosTxStoreValidatedAtRejectsReplacement(t *testing.T) { + store := NewCosmosTxStore(log.NewNopLogger()) + + key, err := crypto.GenerateKey() + require.NoError(t, err) + signer := crypto.CompressPubkey(&key.PublicKey) + + replaced := newKeyedMockTxWithPubKey(signer, 3) + replacement := newKeyedMockTxWithPubKey(signer, 3) + + store.SetHeight(7) + store.AddTx(replaced) + store.SetHeight(8) + store.AddTx(replacement) // same (signer, nonce): overwrites the slot + + height, ok := store.ValidatedAt(replacement) + require.True(t, ok) + require.Equal(t, uint64(8), height) + + _, ok = store.ValidatedAt(replaced) + require.False(t, ok, "a replacement's stamp must not vouch for the replaced tx") +} + +// Every removal path must drop a tx from the identity index, or a dead entry +// would keep vouching for it. +func TestCosmosTxStoreValidatedAtDroppedOnRemoval(t *testing.T) { + key, err := crypto.GenerateKey() + require.NoError(t, err) + signer := crypto.CompressPubkey(&key.PublicKey) + + newStore := func(txs ...sdk.Tx) *CosmosTxStore { + store := NewCosmosTxStore(log.NewNopLogger()) + store.SetHeight(7) + for _, tx := range txs { + store.AddTx(tx) + } + return store + } + assertDropped := func(store *CosmosTxStore, txs ...sdk.Tx) { + t.Helper() + for _, tx := range txs { + _, ok := store.ValidatedAt(tx) + require.False(t, ok) + } + } + + tx3 := newKeyedMockTxWithPubKey(signer, 3) + tx4 := newKeyedMockTxWithPubKey(signer, 4) + + // RemoveTx + store := newStore(tx3) + require.True(t, store.RemoveTx(tx3)) + assertDropped(store, tx3) + + // InvalidateFrom removes the tx and its dependents + store = newStore(tx3, tx4) + require.Equal(t, 2, store.InvalidateFrom(tx3)) + assertDropped(store, tx3, tx4) + + // PruneCommitted watermarks and drops at-or-below entries + store = newStore(tx3, tx4) + require.Equal(t, 1, store.PruneCommitted(tx3)) + assertDropped(store, tx3) + + // a consumed tx must not re-enter via the fast path either + store.AddTx(tx3) + assertDropped(store, tx3) +} From fb189ea3b88bc0c59551067491e6d68a33d9fbce Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 10:02:50 +0800 Subject: [PATCH 13/22] skip redundant signature crypto during recheck passes * recheck passes now run with IsReCheckTx, the contract CometBFT's own recheck uses. * audited every IsReCheckTx site in the assembled chain; IBC's RedundantRelayDecorator now runs its recheck-mode eviction as upstream intended. * 200-tx recheck pass: 80us/tx -> 54us/tx. --- CHANGELOG.md | 2 +- mempool/recheck_pool.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5da30a105..1f330a2a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,7 +71,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. -- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now re-verifies proposal txs whose snapshot entry was validated at an earlier height (a carried-forward snapshot no longer guarantees validity at the proposed height); entries validated at the head skip re-verification, keeping steady-state proposal cost unchanged. +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now re-verifies proposal txs whose snapshot entry was validated at an earlier height (a carried-forward snapshot no longer guarantees validity at the proposed height); entries validated at the head skip re-verification, keeping steady-state proposal cost unchanged. Recheck passes now run in recheck mode, so sigverify skips the crypto already done at insert (~1.5x faster passes). ## v0.6.0 diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 197e8c2e5..728c211a0 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -483,7 +483,10 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header keepFuturesOnError := false if !invalidTx { ctx, write := m.rechecker.GetContext() - _, err := m.rechecker.RecheckCosmos(ctx, txn) + // Signatures were verified on insert and the bytes have not changed, + // so recheck mode lets sigverify skip the crypto, state-dependent + // checks (sequence, fees, balances) still run. + _, err := m.rechecker.RecheckCosmos(ctx.WithIsReCheckTx(true), txn) if err == nil { write() m.markTxRechecked(txn) From 1a79898629b5c63508dd827f91e055552f95584b Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 10:52:16 +0800 Subject: [PATCH 14/22] pin SDK config-registry scope in evmd * bech32 parsing reaches it several times per tx * pin scope to app name to reduce lookup to a map hit * win applies to every GetConfig caller in the node, not just mempool * 200-tx recheck pass: 54us/tx -> 44.5us/tx --- evmd/app.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/evmd/app.go b/evmd/app.go index 204345c2e..50ebc644f 100644 --- a/evmd/app.go +++ b/evmd/app.go @@ -132,6 +132,13 @@ func init() { // manually update the power reduction by replacing micro (u) -> atto (a) evmos sdk.DefaultPowerReduction = utils.AttoPowerReduction + // Pin config-registry scope to skip per-call syscalls, see cosmos-sdk#26729 + if os.Getenv(sdk.EnvConfigScope) == "" { + if err := os.Setenv(sdk.EnvConfigScope, appName); err != nil { + panic(err) + } + } + defaultNodeHome = evmconfig.MustGetDefaultNodeHome() } From 9efa5766963b05abd0e835ed5a92b6ba0d8b0503 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 10:53:22 +0800 Subject: [PATCH 15/22] cache pinned-generation block header for hot paths CurrentBlock rebuilds header from a fresh query context plus keeper reads on every call, paid by proposal path via the iterator's base-fee lookup and by every EVM insert's fee-cap check. 200-tx recheck pass: ~42us/tx (main: ~79us/tx), proposal steady state 0.93-1.10ms, parity with main. --- mempool/blockchain.go | 19 +++++++++++++++++++ mempool/iterator.go | 2 +- mempool/mempool.go | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/mempool/blockchain.go b/mempool/blockchain.go index bef25e3b6..6c9a64c93 100644 --- a/mempool/blockchain.go +++ b/mempool/blockchain.go @@ -51,6 +51,9 @@ type Blockchain struct { // paths can read the pinned height without copying a full sdk.Context. latestHeight atomic.Int64 + // pinnedHeader caches the header for the current pin generation, setLatestContext invalidates it + pinnedHeader atomic.Pointer[types.Header] + testingCommitMu sync.RWMutex } @@ -268,6 +271,22 @@ func (b *Blockchain) setLatestContext(ctx sdk.Context) { } else { b.latestHeight.Store(ctx.BlockHeight()) } + b.pinnedHeader.Store(nil) +} + +// PinnedHeader returns the current block header, cached per pin generation +// (headers only change at commit, and the pin refreshes right after). Use it +// on hot paths that tolerate pin-refresh granularity, CurrentBlock always +// rebuilds fresh. +func (b *Blockchain) PinnedHeader() *types.Header { + if h := b.pinnedHeader.Load(); h != nil { + return h + } + h := b.CurrentBlock() + if h != b.zeroHeader { + b.pinnedHeader.Store(h) + } + return h } // LatestHeight returns the pinned context's block height, or 0 when no diff --git a/mempool/iterator.go b/mempool/iterator.go index 392544878..cdd0f9c9c 100644 --- a/mempool/iterator.go +++ b/mempool/iterator.go @@ -294,7 +294,7 @@ func currentBaseFee(blockchain *Blockchain) *uint256.Int { return nil } - header := blockchain.CurrentBlock() + header := blockchain.PinnedHeader() if header == nil || header.BaseFee == nil { return nil } diff --git a/mempool/mempool.go b/mempool/mempool.go index 1fdc6bdbf..869965fef 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -339,7 +339,7 @@ func (m *Mempool) insert(tx sdk.Tx) (<-chan error, error) { ethTx := ethMsg.AsTransaction() // Reject txs below base fee up-front, which can never be included. - if baseFee := m.blockchain.CurrentBlock().BaseFee; baseFee != nil && ethTx.GasFeeCapIntCmp(baseFee) < 0 { + if baseFee := m.blockchain.PinnedHeader().BaseFee; baseFee != nil && ethTx.GasFeeCapIntCmp(baseFee) < 0 { return nil, sdkerrors.ErrInsufficientFee.Wrapf( "max fee per gas (%s) is lower than the base fee (%s)", ethTx.GasFeeCap(), baseFee, From 0e8fd5d2feda2c9a215105a3c56cf450888a86fe Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 22:54:46 +0800 Subject: [PATCH 16/22] skip stale-watermarked signers in a pass instead of silently rejecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * a stale mark (optimistically executed block that never committed): ante succeeds, write() bumps the cached sequence, AddTx silently rejects — later same-signer txs enter the snapshot as a nonce gap * ante disambiguates: real marks still evict via ErrWrongSequence; under a stale mark, skip the tx and its dependents without evicting until the mark ages out --- mempool/recheck_pool.go | 40 +++++++++++++++++++++++++++++ mempool/recheck_pool_test.go | 49 ++++++++++++++++++++++++++++++++++++ mempool/tx_store.go | 18 +++++++++++++ 3 files changed, 107 insertions(+) diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 728c211a0..4f542794e 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -448,6 +448,7 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header m.recheckedTxs.Do(func(store *CosmosTxStore) { store.SetHeight(newHead.Number.Uint64()) }) failedAtSequence := make(map[string]uint64) + consumedAtSequence := make(map[string]uint64) removeTxs := make([]sdk.Tx, 0) // context.Background() safe to use here since ExtMempool is a @@ -480,6 +481,22 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } } + // skip (without evicting) dependents of a stale watermark detected + // below — without the ancestor's write() their ante would wrongly + // evict still-valid txs + staleConsumed := false + for _, s := range signers { + if seq, ok := consumedAtSequence[string(s.Signer)]; ok && seq <= s.Sequence { + staleConsumed = true + break + } + } + if staleConsumed && !invalidTx { + m.markTxRemoved(txn) // keep it out of the snapshot for this pass + iter = iter.Next() + continue + } + keepFuturesOnError := false if !invalidTx { ctx, write := m.rechecker.GetContext() @@ -488,6 +505,23 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header // checks (sequence, fees, balances) still run. _, err := m.rechecker.RecheckCosmos(ctx.WithIsReCheckTx(true), txn) if err == nil { + // Ante succeeding under a watermark means the mark is stale (an + // optimistically executed block that never committed) — a real + // mark fails ErrWrongSequence and evicts below. Writing the + // cache and letting AddTx silently reject would gap the + // snapshot, so skip the tx and its dependents this pass without + // evicting; they return once the mark ages out. + if m.snapshotConsumedBy(txn, signers) { + m.markTxRemoved(txn) + for _, s := range signers { + key := string(s.Signer) + if existing, ok := consumedAtSequence[key]; !ok || existing > s.Sequence { + consumedAtSequence[key] = s.Sequence + } + } + iter = iter.Next() + continue + } write() m.markTxRechecked(txn) iter = iter.Next() @@ -571,6 +605,12 @@ func (m *RecheckMempool) markTxRemoved(txn sdk.Tx) { m.recheckedTxs.Do(func(store *CosmosTxStore) { store.RemoveTx(txn) }) } +// snapshotConsumedBy reports whether a committed-nonce watermark covers txn. +func (m *RecheckMempool) snapshotConsumedBy(txn sdk.Tx, signers []sdkmempool.SignerData) (consumed bool) { + m.recheckedTxs.Do(func(store *CosmosTxStore) { consumed = store.IsConsumedBy(txn, signers) }) + return consumed +} + // PruneCommitted records that a block being finalized consumed tx's // signer/nonces and drops tx (and any lower-nonced sibling) from current snapshot. // It runs synchronously during FinalizeBlock so carried-forward store diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index b32390e7e..a62276d43 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -1592,3 +1592,52 @@ func collectIteratorTxs(iter sdkmempool.Iterator) []sdk.Tx { } return txs } + +// A stale watermark must not gap the snapshot: the pass skips consumed tx and +// its dependents without evicting them, and both return once the mark ages out. +func TestRecheckMempool_StaleWatermarkSkipsDependentsWithoutGap(t *testing.T) { + tracker := reserver.NewReservationTracker() + handle := tracker.NewHandle(1) + ctx := newRecheckTestContext() + bc := newTestBlockchain(t, ctx) + + // ante always succeeds: the chain never committed the watermarked nonce + rc := newMockRechecker(ctx, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + }) + mp := mempool.NewRecheckMempool( + nil, 0, handle, rc, + newTestRecheckedTxs(), newTestReapList(), bc, log.NewNopLogger(), + ) + mp.Start(testHeader(0)) + defer mp.Close() + + key, err := crypto.GenerateKey() + require.NoError(t, err) + tx0 := newRecheckTestTxWithNonce(t, key, 0) + tx1 := newRecheckTestTxWithNonce(t, key, 1) + require.NoError(t, mp.Insert(context.Background(), tx0)) + require.NoError(t, mp.Insert(context.Background(), tx1)) + + mp.TriggerRecheckSync(testHeader(1)) + require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(1))), 2) + + // watermark nonce 0 as if a block consumed it, without advancing state + mp.PruneCommitted(tx0) + + // the pass must not produce a gapped snapshot (tx1 without tx0)... + mp.TriggerRecheckSync(testHeader(2)) + require.Empty(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(2))), + "neither the consumed tx nor its dependent may be in the snapshot") + // ...and must not evict either tx from the pool + require.Equal(t, 2, mp.CountTx()) + + // mark moves to the older generation: still enforced + mp.TriggerRecheckSync(testHeader(3)) + require.Empty(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(3)))) + + // mark aged out: both txs return, no gap at any point + mp.TriggerRecheckSync(testHeader(4)) + require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(4))), 2) + require.Equal(t, 2, mp.CountTx()) +} diff --git a/mempool/tx_store.go b/mempool/tx_store.go index c3f754b30..4a50365e5 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -355,6 +355,24 @@ func (s *CosmosTxStore) AgeWatermarks() { consumedWatermarkSize.Record(context.Background(), int64(len(s.prevConsumed))) } +// IsConsumedBy reports whether any of tx's (signer, nonce) pairs sits at or +// below a committed-nonce watermark (see PruneCommitted). The caller supplies +// already-extracted signers so the check does not re-pay signer extraction. +func (s *CosmosTxStore) IsConsumedBy(tx sdk.Tx, signers []sdkmempool.SignerData) bool { + nonceMap := make(map[string]uint64, len(signers)) + for _, sig := range signers { + nonce, err := sdkmempool.ChooseNonce(sig.Sequence, tx) + if err != nil { + return false + } + nonceMap[string(sig.Signer)] = nonce + } + + s.mu.RLock() + defer s.mu.RUnlock() + return s.isConsumedLocked(nonceMap) +} + // isConsumedLocked reports whether any signer of the given nonceMap sits at or // below the committed high-water mark. Callers must hold s.mu. func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { From a81cb0d8b8fb42e6914c9c20f5c7120493ba2a0e Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 22:54:47 +0800 Subject: [PATCH 17/22] key proposal re-verification to proposal's base height * the skip predicate trusted the notify-driven pin, which can lag the last commit (or die with the event bus), letting stale entries skip ante * record req.Height-1 on the verifier per proposal; stamps behind base fail closed into re-verification --- CHANGELOG.md | 2 +- evmd/mempool.go | 19 +++++++++++++------ evmd/tx_verifier.go | 24 ++++++++++++++++++------ mempool/blockchain.go | 15 --------------- mempool/mempool.go | 23 +++++++---------------- 5 files changed, 39 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f330a2a4..88bc1cc32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,7 +71,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. -- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under large backlogs: carry rechecked snapshot across heights, prune committed txs via a per-signer nonce watermark, and serve the carried-forward snapshot when the recheck loop falls behind a proposal. `evmd` now re-verifies proposal txs whose snapshot entry was validated at an earlier height (a carried-forward snapshot no longer guarantees validity at the proposed height); entries validated at the head skip re-verification, keeping steady-state proposal cost unchanged. Recheck passes now run in recheck mode, so sigverify skips the crypto already done at insert (~1.5x faster passes). +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under backlogs: carry the rechecked snapshot across heights, watermark committed nonces, serve the carried snapshot when recheck lags, re-verify proposal txs not validated at the proposal base, and skip redundant sigverify on recheck. ## v0.6.0 diff --git a/evmd/mempool.go b/evmd/mempool.go index 27ee252ec..70b800c00 100644 --- a/evmd/mempool.go +++ b/evmd/mempool.go @@ -1,6 +1,8 @@ package evmd import ( + abci "github.com/cometbft/cometbft/abci/types" + evmmempool "github.com/cosmos/evm/mempool" "github.com/cosmos/evm/server" evmtypes "github.com/cosmos/evm/x/vm/types" @@ -53,13 +55,18 @@ func (app *EVMD) configureEVMMempool(appOpts servertypes.AppOptions, logger log. app.EVMMempool = mempool - // Under backlog the cosmos pool serves a snapshot validated at an earlier - // height, so a selected tx may since have become invalid; the verifier - // re-runs ante for exactly those txs and skips it for entries the mempool - // proves were validated at the head (see SnapshotVerifiedTxVerifier). - prepareProposalHandler := baseapp. - NewDefaultProposalHandler(mempool, NewSnapshotVerifiedTxVerifier(app.BaseApp, mempool)). + // Re-run ante for any selected tx the mempool cannot prove was validated + // at the height this proposal builds on (see SnapshotVerifiedTxVerifier). + // The base comes from the ABCI request, not the notify-driven pin, which + // can lag a beat behind the last commit. + verifier := NewSnapshotVerifiedTxVerifier(app.BaseApp, mempool) + defaultProposalHandler := baseapp. + NewDefaultProposalHandler(mempool, verifier). PrepareProposalHandler() + prepareProposalHandler := func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { + verifier.SetProposalBase(req.Height - 1) + return defaultProposalHandler(ctx, req) + } insertTxHandler := mempool.NewInsertTxHandler(app.TxDecode) reapTxsHandler := mempool.NewReapTxsHandler() diff --git a/evmd/tx_verifier.go b/evmd/tx_verifier.go index ee68d6afa..f221d2ebb 100644 --- a/evmd/tx_verifier.go +++ b/evmd/tx_verifier.go @@ -1,6 +1,8 @@ package evmd import ( + "sync/atomic" + evmmempool "github.com/cosmos/evm/mempool" "github.com/cosmos/cosmos-sdk/baseapp" @@ -10,22 +12,32 @@ import ( var _ baseapp.ProposalTxVerifier = &SnapshotVerifiedTxVerifier{} // SnapshotVerifiedTxVerifier re-runs ante over a proposal candidate only when -// the mempool cannot show it was already validated at the current head — -// under backlog the cosmos pool serves a snapshot carried from an earlier -// height, and only those carried entries may since have become invalid. +// the mempool cannot show it was validated at the height the proposal builds +// on. The base is set per proposal from the ABCI request, so a lagging +// recheck pin fails closed into re-verification. type SnapshotVerifiedTxVerifier struct { *baseapp.BaseApp mempool *evmmempool.Mempool + + // proposalBase is the last committed height the in-flight proposal builds + // on (req.Height - 1), set by the prepare-proposal handler before txs are + // verified. Zero means unknown and re-verifies everything. + proposalBase atomic.Int64 } func NewSnapshotVerifiedTxVerifier(b *baseapp.BaseApp, mempool *evmmempool.Mempool) *SnapshotVerifiedTxVerifier { return &SnapshotVerifiedTxVerifier{BaseApp: b, mempool: mempool} } -// PrepareProposalVerifyTx encodes txs validated at the head height and defers -// to BaseApp's full ante verification for stale or unknown ones. +// SetProposalBase records the height the next proposal builds on. +func (txv *SnapshotVerifiedTxVerifier) SetProposalBase(height int64) { + txv.proposalBase.Store(height) +} + +// PrepareProposalVerifyTx encodes txs validated at the proposal's base height +// and defers to BaseApp's full ante verification for stale or unknown ones. func (txv *SnapshotVerifiedTxVerifier) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { - if txv.mempool.ProposalTxValidatedAtHead(tx) { + if base := txv.proposalBase.Load(); base > 0 && txv.mempool.ProposalTxValidatedAt(tx, uint64(base)) { return txv.TxEncode(tx) } return txv.BaseApp.PrepareProposalVerifyTx(tx) diff --git a/mempool/blockchain.go b/mempool/blockchain.go index 6c9a64c93..98384ff78 100644 --- a/mempool/blockchain.go +++ b/mempool/blockchain.go @@ -47,10 +47,6 @@ type Blockchain struct { mu sync.RWMutex coinInfo atomic.Pointer[evmtypes.EvmCoinInfo] - // latestHeight mirrors latestCtx's block height (0 when unset), so hot - // paths can read the pinned height without copying a full sdk.Context. - latestHeight atomic.Int64 - // pinnedHeader caches the header for the current pin generation, setLatestContext invalidates it pinnedHeader atomic.Pointer[types.Header] @@ -266,11 +262,6 @@ func (b *Blockchain) setLatestContext(ctx sdk.Context) { b.mu.Lock() defer b.mu.Unlock() b.latestCtx = ctx - if ctx.Context() == nil { - b.latestHeight.Store(0) - } else { - b.latestHeight.Store(ctx.BlockHeight()) - } b.pinnedHeader.Store(nil) } @@ -289,12 +280,6 @@ func (b *Blockchain) PinnedHeader() *types.Header { return h } -// LatestHeight returns the pinned context's block height, or 0 when no -// context is pinned yet. -func (b *Blockchain) LatestHeight() int64 { - return b.latestHeight.Load() -} - // GetLatestContext returns the latest context as updated by the block, // or attempts to retrieve it again if unavailable. func (b *Blockchain) GetLatestContext() (sdk.Context, error) { diff --git a/mempool/mempool.go b/mempool/mempool.go index 869965fef..c32347548 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -663,26 +663,17 @@ func (m *Mempool) cosmosIterator( return m.recheckCosmosPool.OrderedRecheckedTxs(ctx, height, bondDenom, baseFee) } -// ProposalTxValidatedAtHead reports whether the mempool's snapshot copy of tx -// was ante-validated at the head height, i.e. re-running ante for a proposal -// could not learn anything new. EVM txs always qualify (their snapshot has no -// stale fallback); a cosmos tx qualifies only when its stamp matches the head, -// so carried or unknown txs report false and must be re-verified. -// -// Head mirrors the pinned context's height. If the notify path dies the pin -// and the stamps freeze together; committed txs are still excluded by the -// watermark, which is fed by FinalizeBlock removals. -func (m *Mempool) ProposalTxValidatedAtHead(tx sdk.Tx) bool { +// ProposalTxValidatedAt reports whether the mempool's snapshot copy of tx was +// ante-validated at exactly base, the last committed height the proposal +// builds on. EVM txs always qualify (their snapshot has no stale fallback); +// carried or unknown cosmos txs report false and must be re-verified. Base is +// caller-supplied, so a lagging or dead notify path fails closed. +func (m *Mempool) ProposalTxValidatedAt(tx sdk.Tx, base uint64) bool { if _, err := evmTxFromCosmosTx(tx); err == nil { return true } - head := m.blockchain.LatestHeight() - if head <= 0 { - // no pinned context yet, nothing can prove validation at head - return false - } height, ok := m.recheckCosmosPool.SnapshotValidatedAt(tx) - return ok && height == uint64(head) + return ok && height == base } // TrackTx submits a tx to be tracked for its tx inclusion metrics. From abd7beecc9b952517608e6e90fb269a3e00e3cf3 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 23:03:40 +0800 Subject: [PATCH 18/22] rm committed-nonce watermark subsystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * base-height re-verification already rejects committed txs (wrong sequence), and entries stamped at base cannot be committed ones — the watermark only pre-filtered what the verifier rejects anyway, at the cost of the branch's trickiest invariants: two-generation aging, optimistic-execution false marks, and silent AddTx rejection with its nonce-gap cascade * stale-fallback contract becomes single-sided: consumers must re-verify entries not validated at the proposal base, which evmd's proposal handler does by default * store's signer extractor reverts to the SDK default — the Eth wrapper existed only so PruneCommitted could read EVM tx signers * 200-tx bench unchanged: recheck ~42us/tx, proposal 0.82-0.95ms steady state --- CHANGELOG.md | 2 +- mempool/internal/heightsync/heightsync.go | 9 +- mempool/mempool.go | 13 +- mempool/recheck_pool.go | 58 +------- mempool/recheck_pool_test.go | 165 ---------------------- mempool/tx_store.go | 146 ++----------------- mempool/tx_store_test.go | 160 ++------------------- 7 files changed, 28 insertions(+), 525 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88bc1cc32..0d84ebd47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,7 +71,7 @@ - [\#1050](https://github.com/cosmos/evm/pull/1050) Align precompile gas calculation with expected EVM gas semantics. - [\#1107](https://github.com/cosmos/evm/pull/1107) Skip StateDB commit error transactions during receipt conversion to prevent `invalid message index` errors in block RPCs. - [\#1216](https://github.com/cosmos/evm/pull/1216) Fix blocking on mempool event bus unsubscribe. -- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under backlogs: carry the rechecked snapshot across heights, watermark committed nonces, serve the carried snapshot when recheck lags, re-verify proposal txs not validated at the proposal base, and skip redundant sigverify on recheck. +- [\#1227](https://github.com/cosmos/evm/issues/1227) Fix cosmos-pool proposal starvation under backlogs: carry the rechecked snapshot across heights, serve the carried snapshot when recheck lags, re-verify proposal txs not validated at the proposal base, and skip redundant sigverify on recheck. ## v0.6.0 diff --git a/mempool/internal/heightsync/heightsync.go b/mempool/internal/heightsync/heightsync.go index 0ce14f5c4..5737aded1 100644 --- a/mempool/internal/heightsync/heightsync.go +++ b/mempool/internal/heightsync/heightsync.go @@ -118,11 +118,10 @@ type HeightSync[Store any] struct { // staleFallback makes GetStore return the current carried-forward Store // (instead of nil) when it times out while still behind the target height. - // Entries in it were validated at a height <= target, so enabling it needs - // both: producers keep already-committed entries out (the cosmos pool's - // committed-nonce watermark), and consumers re-verify anything that must - // hold against latest state, since a later block can have invalidated an - // entry for some other reason (evmd's PrepareProposal runs ante). + // Entries in it were validated at a height <= target — possibly including + // txs a later block committed or invalidated — so consumers MUST re-verify + // anything that has to hold against latest state (evmd's PrepareProposal + // re-runs ante for entries not validated at the proposal base). staleFallback bool logger log.Logger diff --git a/mempool/mempool.go b/mempool/mempool.go index c32347548..6941dcbce 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -200,9 +200,9 @@ func NewMempool( panic("tx pool should contain only legacypool") } - // Stale fallback: when recheck loop falls behind consensus, serve last completed - // snapshot rather than an empty proposal — since store's committed-nonce - // watermark keeps already-committed txs out. + // Stale fallback: when the recheck loop falls behind consensus, serve the + // carried snapshot rather than an empty proposal; the proposal verifier + // re-runs ante for entries not validated at the proposal base. heightSync := heightsync.New( blockchain.CurrentBlock().Number, NewCosmosTxStore, @@ -458,10 +458,6 @@ func (m *Mempool) removeCosmosTx(tx sdk.Tx, reason sdkmempool.RemoveReason) erro if reason.Caller == sdkmempool.CallerRunTxFinalize { m.recordNonceAdvances(tx) - // Prune committed tx from recheck snapshot synchronously. Snapshot is carried - // across heights, so without this a just-committed tx could be served - // into next proposal before async recheck pass drops it. - m.recheckCosmosPool.PruneCommitted(tx) } if err := m.recheckCosmosPool.Remove(tx); err != nil { @@ -481,9 +477,6 @@ func (m *Mempool) removeEVMTx(tx sdk.Tx, msgEthereumTx *evmtypes.MsgEthereumTx, if reason.Caller == sdkmempool.CallerRunTxFinalize { _ = m.txTracker.IncludedInBlock(hash) m.recordNonceAdvances(tx) - // an EVM tx consumes the same account sequence, so drop stale - // same-account cosmos txs from the snapshot too - m.recheckCosmosPool.PruneCommitted(tx) } if m.shouldRemoveFromEVMPool(hash, reason) { diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index 4f542794e..fd3ec540b 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -430,9 +430,9 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header defer m.mu.Unlock() // Carry the validated set forward instead of resetting to an empty store, - // a pass cancelled by next block does not discard all progress and starve proposals. - // The pass prunes whatever became invalid, committed txs are kept out by - // the store's watermark (see CosmosTxStore.PruneCommitted). + // so a pass cancelled by the next block does not discard all progress and + // starve proposals. The pass prunes whatever became invalid; anything it + // has not re-validated keeps an old stamp and is re-verified at proposal. m.recheckedTxs.StartNewHeightFrom(newHead.Number, (*CosmosTxStore).Clone) defer m.recheckedTxs.EndCurrentHeight() @@ -448,7 +448,6 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header m.recheckedTxs.Do(func(store *CosmosTxStore) { store.SetHeight(newHead.Number.Uint64()) }) failedAtSequence := make(map[string]uint64) - consumedAtSequence := make(map[string]uint64) removeTxs := make([]sdk.Tx, 0) // context.Background() safe to use here since ExtMempool is a @@ -481,22 +480,6 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } } - // skip (without evicting) dependents of a stale watermark detected - // below — without the ancestor's write() their ante would wrongly - // evict still-valid txs - staleConsumed := false - for _, s := range signers { - if seq, ok := consumedAtSequence[string(s.Signer)]; ok && seq <= s.Sequence { - staleConsumed = true - break - } - } - if staleConsumed && !invalidTx { - m.markTxRemoved(txn) // keep it out of the snapshot for this pass - iter = iter.Next() - continue - } - keepFuturesOnError := false if !invalidTx { ctx, write := m.rechecker.GetContext() @@ -505,23 +488,6 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header // checks (sequence, fees, balances) still run. _, err := m.rechecker.RecheckCosmos(ctx.WithIsReCheckTx(true), txn) if err == nil { - // Ante succeeding under a watermark means the mark is stale (an - // optimistically executed block that never committed) — a real - // mark fails ErrWrongSequence and evicts below. Writing the - // cache and letting AddTx silently reject would gap the - // snapshot, so skip the tx and its dependents this pass without - // evicting; they return once the mark ages out. - if m.snapshotConsumedBy(txn, signers) { - m.markTxRemoved(txn) - for _, s := range signers { - key := string(s.Signer) - if existing, ok := consumedAtSequence[key]; !ok || existing > s.Sequence { - consumedAtSequence[key] = s.Sequence - } - } - iter = iter.Next() - continue - } write() m.markTxRechecked(txn) iter = iter.Next() @@ -583,9 +549,6 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } } txsRemoved = len(removeTxs) - - // a completed pass makes watermarks recorded before it redundant - m.recheckedTxs.Do(func(store *CosmosTxStore) { store.AgeWatermarks() }) } // SnapshotValidatedAt reports the height the current snapshot's copy of txn @@ -605,21 +568,6 @@ func (m *RecheckMempool) markTxRemoved(txn sdk.Tx) { m.recheckedTxs.Do(func(store *CosmosTxStore) { store.RemoveTx(txn) }) } -// snapshotConsumedBy reports whether a committed-nonce watermark covers txn. -func (m *RecheckMempool) snapshotConsumedBy(txn sdk.Tx, signers []sdkmempool.SignerData) (consumed bool) { - m.recheckedTxs.Do(func(store *CosmosTxStore) { consumed = store.IsConsumedBy(txn, signers) }) - return consumed -} - -// PruneCommitted records that a block being finalized consumed tx's -// signer/nonces and drops tx (and any lower-nonced sibling) from current snapshot. -// It runs synchronously during FinalizeBlock so carried-forward store -// can never feed an already-committed tx into a later proposal, -// even before next recheck pass runs. -func (m *RecheckMempool) PruneCommitted(txn sdk.Tx) { - m.recheckedTxs.Do(func(store *CosmosTxStore) { store.PruneCommitted(txn) }) -} - // markTxInserted conservatively updates the current height snapshot for live inserts. // If the inserted tx replaces an existing tx, any other txs from the same sender with // a higher nonce is dropped and rebuilt by the next recheck. diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index a62276d43..12f0a4d75 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -882,122 +882,6 @@ func setupEVMChainConfig(t *testing.T) client.TxConfig { return encodingConfig.TxConfig } -// snapshotAccepts reports whether the snapshot admits tx (AddTx refuses txs -// under a committed-nonce watermark), leaving the store as it found it. -func snapshotAccepts(hs *heightsync.HeightSync[mempool.CosmosTxStore], tx sdk.Tx) bool { - accepted := false - hs.Do(func(store *mempool.CosmosTxStore) { - before := store.Len() - store.AddTx(tx) - accepted = store.Len() > before - store.RemoveTx(tx) - }) - return accepted -} - -// An EVM tx consumes same sequence as its account's cosmos txs, so a committed -// one must prune them — its sender and nonce exist only in the eth payload. -func TestCosmosTxStorePruneCommittedEVMTxDropsCosmosTxs(t *testing.T) { - txConfig := setupEVMChainConfig(t) - - key, err := crypto.GenerateKey() - require.NoError(t, err) - other, err := crypto.GenerateKey() - require.NoError(t, err) - - store := mempool.NewCosmosTxStore(log.NewNopLogger()) - store.AddTx(newRecheckTestTxWithNonce(t, key, 0)) - store.AddTx(newRecheckTestTxWithNonce(t, key, 1)) - store.AddTx(newRecheckTestTxWithNonce(t, other, 0)) - require.Equal(t, 3, store.Len()) - - // committing the account's EVM tx at nonce 0 consumes that sequence - evmTx := createMsgEthereumTx(t, txConfig, key, 0, big.NewInt(1e8)) - require.Equal(t, 1, store.PruneCommitted(evmTx), - "an EVM commit must prune the same account's cosmos tx at the consumed sequence") - - // the account's later nonce and the unrelated account are both untouched - require.Equal(t, 2, store.Len()) - - // the watermark also keeps the consumed sequence from being re-added - store.AddTx(newRecheckTestTxWithNonce(t, key, 0)) - require.Equal(t, 2, store.Len()) -} - -// A watermark must age out after two completed passes: those passes revalidated -// the pool past the commit, and a mark kept longer — e.g. one left by an optimistically -// executed block that never committed — would blacklist signer's nonce forever. -func TestRecheckMempool_CompletedRecheckAgesWatermarks(t *testing.T) { - ctx := newRecheckTestContext() - mp, recheckedTxs := newStartedRecheckMempool(t, ctx, nil, noopAnteHandler) - - key, err := crypto.GenerateKey() - require.NoError(t, err) - committed := newRecheckTestTxWithNonce(t, key, 4) - - // committing nonce 4 watermarks the signer, so the snapshot refuses it - mp.PruneCommitted(committed) - require.False(t, snapshotAccepts(recheckedTxs, committed)) - - // one pass retires the mark to the older generation, where it still blocks - mp.TriggerRecheckSync(testHeader(1)) - require.False(t, snapshotAccepts(recheckedTxs, committed)) - - // the second pass retires it for good - mp.TriggerRecheckSync(testHeader(2)) - require.True(t, snapshotAccepts(recheckedTxs, committed), - "a watermark must not outlive two completed recheck passes") -} - -// A cancelled pass revalidated nothing, so it must not age watermarks: -// retiring a mark a generation early lets the snapshot re-admit a committed tx. -func TestRecheckMempool_CancelledRecheckKeepsWatermarks(t *testing.T) { - ctx := newRecheckTestContext() - - var blockPass atomic.Bool - ready := make(chan struct{}) - gate := make(chan struct{}) - anteHandler := func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { - if blockPass.Load() { - ready <- struct{}{} - <-gate - } - return ctx, nil - } - - mp, recheckedTxs := newStartedRecheckMempool(t, ctx, nil, anteHandler) - - // a pooled tx gives the stalling pass something to stall on - poolKey, err := crypto.GenerateKey() - require.NoError(t, err) - require.NoError(t, mp.Insert(ctx, newRecheckTestTx(t, poolKey))) - - // an unrelated signer commits nonce 4, watermarking it - committedKey, err := crypto.GenerateKey() - require.NoError(t, err) - committed := newRecheckTestTxWithNonce(t, committedKey, 4) - mp.PruneCommitted(committed) - require.False(t, snapshotAccepts(recheckedTxs, committed)) - - // stall a height-1 pass, then let height 2 cancel it - blockPass.Store(true) - mp.TriggerRecheck(testHeader(1)) - <-ready - done := mp.TriggerRecheck(testHeader(2)) - blockPass.Store(false) - close(gate) - <-done - - // Only the height-2 pass completed, so the mark aged once and still covers - // the commit. Aging on the cancelled pass too would have retired it. - require.False(t, snapshotAccepts(recheckedTxs, committed), - "a cancelled pass must not age watermarks") - - // the next completed pass retires it - mp.TriggerRecheckSync(testHeader(3)) - require.True(t, snapshotAccepts(recheckedTxs, committed)) -} - func TestRecheckMempool_RecheckedTxsBlocksUntilComplete(t *testing.T) { acc := newRecheckTestAccount(t) tracker := reserver.NewReservationTracker() @@ -1592,52 +1476,3 @@ func collectIteratorTxs(iter sdkmempool.Iterator) []sdk.Tx { } return txs } - -// A stale watermark must not gap the snapshot: the pass skips consumed tx and -// its dependents without evicting them, and both return once the mark ages out. -func TestRecheckMempool_StaleWatermarkSkipsDependentsWithoutGap(t *testing.T) { - tracker := reserver.NewReservationTracker() - handle := tracker.NewHandle(1) - ctx := newRecheckTestContext() - bc := newTestBlockchain(t, ctx) - - // ante always succeeds: the chain never committed the watermarked nonce - rc := newMockRechecker(ctx, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { - return ctx, nil - }) - mp := mempool.NewRecheckMempool( - nil, 0, handle, rc, - newTestRecheckedTxs(), newTestReapList(), bc, log.NewNopLogger(), - ) - mp.Start(testHeader(0)) - defer mp.Close() - - key, err := crypto.GenerateKey() - require.NoError(t, err) - tx0 := newRecheckTestTxWithNonce(t, key, 0) - tx1 := newRecheckTestTxWithNonce(t, key, 1) - require.NoError(t, mp.Insert(context.Background(), tx0)) - require.NoError(t, mp.Insert(context.Background(), tx1)) - - mp.TriggerRecheckSync(testHeader(1)) - require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(1))), 2) - - // watermark nonce 0 as if a block consumed it, without advancing state - mp.PruneCommitted(tx0) - - // the pass must not produce a gapped snapshot (tx1 without tx0)... - mp.TriggerRecheckSync(testHeader(2)) - require.Empty(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(2))), - "neither the consumed tx nor its dependent may be in the snapshot") - // ...and must not evict either tx from the pool - require.Equal(t, 2, mp.CountTx()) - - // mark moves to the older generation: still enforced - mp.TriggerRecheckSync(testHeader(3)) - require.Empty(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(3)))) - - // mark aged out: both txs return, no gap at any point - mp.TriggerRecheckSync(testHeader(4)) - require.Len(t, collectIteratorTxs(mp.RecheckedTxs(context.Background(), big.NewInt(4))), 2) - require.Equal(t, 2, mp.CountTx()) -} diff --git a/mempool/tx_store.go b/mempool/tx_store.go index 4a50365e5..3ebbd8be9 100644 --- a/mempool/tx_store.go +++ b/mempool/tx_store.go @@ -1,7 +1,6 @@ package mempool import ( - "context" "maps" "reflect" "slices" @@ -9,29 +8,12 @@ import ( "strings" "sync" - "go.opentelemetry.io/otel/metric" - "cosmossdk.io/log/v2" sdk "github.com/cosmos/cosmos-sdk/types" sdkmempool "github.com/cosmos/cosmos-sdk/types/mempool" ) -// consumedWatermarkSize reports how many per-signer committed-nonce -// watermarks survived the latest aging (see AgeWatermarks). -var consumedWatermarkSize metric.Int64Gauge - -func init() { - var err error - consumedWatermarkSize, err = meter.Int64Gauge( - "cosmos_tx_store.consumed_watermark_size", - metric.WithDescription("Number of per-signer committed-nonce watermarks retained by the carried-forward store"), - ) - if err != nil { - panic(err) - } -} - // CosmosTxStore is a set of cosmos transactions that can be added to or // removed from. type CosmosTxStore struct { @@ -39,17 +21,10 @@ type CosmosTxStore struct { nextUnkeyed uint64 // signerBuckets indexes signer -> keys of the buckets containing it, so - // shared-signer scans (PruneCommitted, InvalidateFrom) touch only matching - // buckets. Membership changes only when a bucket is created or emptied. + // shared-signer scans (InvalidateFrom, InvalidateReplaced) touch only + // matching buckets. Membership changes when a bucket is created or emptied. signerBuckets map[string]map[string]struct{} - // consumed and prevConsumed hold two generations of per-signer high-water - // marks of committed nonces, so the carried-forward store cannot re-admit - // a just-committed tx: AddTx rejects and PruneCommitted drops txs at or - // below a mark; AgeWatermarks retires the older generation. - consumed map[string]uint64 - prevConsumed map[string]uint64 - // height is the chain height the snapshot is being (re)built for (see // SetHeight). AddTx stamps entries with it; carried entries keep the stamp // of the pass that validated them (see ValidatedAt). @@ -85,17 +60,15 @@ func NewCosmosTxStore(l log.Logger) *CosmosTxStore { return &CosmosTxStore{ txs: make(map[string]cosmosTxBucket), signerBuckets: make(map[string]map[string]struct{}), - consumed: make(map[string]uint64), - prevConsumed: make(map[string]uint64), byTx: make(map[sdk.Tx]uint64), logger: l, - signerExtractor: NewEthSignerExtractionAdapter(sdkmempool.NewDefaultSignerExtractionAdapter()), + signerExtractor: sdkmempool.NewDefaultSignerExtractionAdapter(), } } // Clone returns a deep-enough copy of store for carrying the validated set forward // into next height. The tx values are shared (immutable), but the -// bucket/index/consumed maps are copied so mutations on clone do not affect source. +// bucket/index maps are copied so mutations on clone do not affect source. func (s *CosmosTxStore) Clone() *CosmosTxStore { s.mu.RLock() defer s.mu.RUnlock() @@ -103,8 +76,6 @@ func (s *CosmosTxStore) Clone() *CosmosTxStore { clone := &CosmosTxStore{ txs: make(map[string]cosmosTxBucket, len(s.txs)), signerBuckets: make(map[string]map[string]struct{}, len(s.signerBuckets)), - consumed: maps.Clone(s.consumed), - prevConsumed: maps.Clone(s.prevConsumed), byTx: maps.Clone(s.byTx), nextUnkeyed: s.nextUnkeyed, height: s.height, @@ -164,9 +135,8 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { indexable := isPointerTx(tx) - // Fast path: re-adding a known tx only refreshes its stamp — its bucket - // entry is already correct, and its presence proves it survived every - // watermark prune. + // fast path: re-adding a known tx only refreshes its stamp — its bucket + // entry is already correct if indexable { if _, ok := s.byTx[tx]; ok { s.byTx[tx] = s.height @@ -176,13 +146,6 @@ func (s *CosmosTxStore) AddTx(tx sdk.Tx) { storedTx := s.newCosmosTxWithMetadata(tx) - // Reject txs whose nonce a committed block already consumed. This guards the - // carried-forward store from re-admitting an already-committed tx via a - // recheck pass or an Insert that races FinalizeBlock. - if s.isConsumedLocked(storedTx.nonceMap) { - return - } - if storedTx.signerKey == "" { storedTx.signerKey = unkeyedSignerKey } @@ -290,101 +253,10 @@ func (s *CosmosTxStore) RemoveTx(tx sdk.Tx) bool { return false } - return s.removeTxKeyLocked(storedTx.signerKey, storedTx.txKey) > 0 -} - -// removeTxKeyLocked removes the tx with the exact txKey from its signer-set -// bucket. Callers must hold s.mu. Returns the number of txs removed (0 or 1). -func (s *CosmosTxStore) removeTxKeyLocked(signerKey, txKey string) int { - return s.filterBucketLocked(signerKey, s.txs[signerKey], func(t cosmosTxWithMetadata) bool { - return t.txKey == txKey + removed := s.filterBucketLocked(storedTx.signerKey, s.txs[storedTx.signerKey], func(t cosmosTxWithMetadata) bool { + return t.txKey == storedTx.txKey }) -} - -// PruneCommitted records that a committed block consumed the given tx's -// signer/nonces and drops any stored tx at or below a consumed nonce. It is -// called synchronously as a block is finalized so the carried-forward store can -// never feed an already-committed tx into a later proposal, even before the -// next recheck pass runs. Returns the number of stored txs pruned. -// -// baseapp removes a block tx with CallerRunTxFinalize only after ante -// succeeds, which consumes the signers' sequences even if message execution -// later fails — so every tx reaching here is safe to watermark. -func (s *CosmosTxStore) PruneCommitted(tx sdk.Tx) int { - s.mu.Lock() - defer s.mu.Unlock() - - nonceMap, ok := s.cosmosTxNonceMap(tx) - if !ok { - return 0 - } - - // An unordered tx consumes no sequence — its nonce is a timeout timestamp - // that would blacklist the signer if watermarked. Drop exactly this tx; - // on-chain unordered-nonce tracking prevents re-execution. - if unordered, ok := tx.(sdk.TxWithUnordered); ok && unordered.GetUnordered() { - return s.removeTxKeyLocked(cosmosTxSignerSetKey(nonceMap), cosmosTxKey(nonceMap)) - } - - // bump the per-signer high-water mark - for signer, nonce := range nonceMap { - s.consumed[signer] = max(s.consumed[signer], nonce) - } - - // drop any stored tx now under a watermark: a tx is invalid if ANY of its - // signers has a consumed nonce. Only buckets sharing a signer with the - // just-committed tx can hold a newly consumed tx (a prior commit already - // pruned the rest and AddTx rejects re-adds). - return s.filterSignerBucketsLocked(nonceMap, func(t cosmosTxWithMetadata) bool { - return s.isConsumedLocked(t.nonceMap) - }) -} - -// AgeWatermarks retires the older watermark generation. Call it only after -// an uncancelled recheck pass: the pool was revalidated against state at -// least as new as those marks' commits, so ante now rejects the re-adds they -// guarded against; marks written mid-pass survive one more generation. This -// also heals marks from optimistically-executed blocks that never committed. -func (s *CosmosTxStore) AgeWatermarks() { - s.mu.Lock() - defer s.mu.Unlock() - - s.prevConsumed = s.consumed - s.consumed = make(map[string]uint64) - - consumedWatermarkSize.Record(context.Background(), int64(len(s.prevConsumed))) -} - -// IsConsumedBy reports whether any of tx's (signer, nonce) pairs sits at or -// below a committed-nonce watermark (see PruneCommitted). The caller supplies -// already-extracted signers so the check does not re-pay signer extraction. -func (s *CosmosTxStore) IsConsumedBy(tx sdk.Tx, signers []sdkmempool.SignerData) bool { - nonceMap := make(map[string]uint64, len(signers)) - for _, sig := range signers { - nonce, err := sdkmempool.ChooseNonce(sig.Sequence, tx) - if err != nil { - return false - } - nonceMap[string(sig.Signer)] = nonce - } - - s.mu.RLock() - defer s.mu.RUnlock() - return s.isConsumedLocked(nonceMap) -} - -// isConsumedLocked reports whether any signer of the given nonceMap sits at or -// below the committed high-water mark. Callers must hold s.mu. -func (s *CosmosTxStore) isConsumedLocked(nonceMap map[string]uint64) bool { - for signer, nonce := range nonceMap { - if mark, ok := s.consumed[signer]; ok && nonce <= mark { - return true - } - if mark, ok := s.prevConsumed[signer]; ok && nonce <= mark { - return true - } - } - return false + return removed > 0 } // filterBucketLocked removes every tx in the bucket at signerKey for which diff --git a/mempool/tx_store_test.go b/mempool/tx_store_test.go index 12f94667f..48aeeff40 100644 --- a/mempool/tx_store_test.go +++ b/mempool/tx_store_test.go @@ -3,7 +3,6 @@ package mempool import ( "slices" "testing" - "time" "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" @@ -95,25 +94,6 @@ func (m *keyedMockTx) GetSignaturesV2() ([]signingtypes.SignatureV2, error) { }}, nil } -// unorderedMockTx is a keyedMockTx flagged unordered: its ChooseNonce value is -// the timeout timestamp, not the (zero) sequence. -type unorderedMockTx struct { - keyedMockTx - timeout time.Time -} - -var _ sdk.TxWithUnordered = (*unorderedMockTx)(nil) - -func newUnorderedMockTxWithPubKey(pubKeyBytes []byte, timeout time.Time) sdk.Tx { - return &unorderedMockTx{ - keyedMockTx: keyedMockTx{pubKey: ðsecp256k1.PubKey{Key: pubKeyBytes}}, - timeout: timeout, - } -} - -func (m *unorderedMockTx) GetUnordered() bool { return true } -func (m *unorderedMockTx) GetTimeoutTimeStamp() time.Time { return m.timeout } - func newMultiKeyedMockTx(pubKeyBytes [][]byte, sequences []uint64) sdk.Tx { pubKeys := make([]cryptotypes.PubKey, 0, len(pubKeyBytes)) for _, pubKey := range pubKeyBytes { @@ -331,11 +311,12 @@ func TestCosmosTxStoreCloneIsIndependent(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) signer := newPubKeyBytes(t) - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) + tx0 := newKeyedMockTxWithPubKey(signer, 0) + store.AddTx(tx0) store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) store.AddTx(newKeyedMockTxWithPubKey(signer, 2)) - // carry a committed watermark forward too: drops nonce 0, leaving 1 and 2 - store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0)) + // remove one entry so the clone starts from a mutated source + require.True(t, store.RemoveTx(tx0)) require.Equal(t, 2, store.Len()) clone := store.Clone() @@ -350,74 +331,8 @@ func TestCosmosTxStoreCloneIsIndependent(t *testing.T) { require.True(t, store.RemoveTx(newKeyedMockTxWithPubKey(signer, 1))) require.Equal(t, 1, store.Len()) require.Equal(t, 3, clone.Len()) - - // the committed watermark is carried: the clone still rejects the consumed nonce - clone.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 3, clone.Len()) -} - -// A watermark blocks re-adds for its own generation plus one aging, then is -// retired: two completed recheck passes have covered the commit by then. -func TestCosmosTxStoreAgeWatermarks(t *testing.T) { - store := NewCosmosTxStore(log.NewNopLogger()) - - signer := newPubKeyBytes(t) - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0))) - - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 0, store.Len()) - - // first aging keeps the mark one more generation - store.AgeWatermarks() - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 0, store.Len()) - - // second aging retires it; a stale (e.g. never-committed) mark heals here - store.AgeWatermarks() - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 1, store.Len()) -} - -func TestCosmosTxStorePruneCommitted(t *testing.T) { - store := NewCosmosTxStore(log.NewNopLogger()) - - signer := newPubKeyBytes(t) - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) - store.AddTx(newKeyedMockTxWithPubKey(signer, 2)) - require.Equal(t, 3, store.Len()) - - // committing nonce 0 drops nonce 0, keeps 1 and 2 - require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 0))) - require.Equal(t, 2, store.Len()) - - // a re-add of the committed nonce is rejected by the watermark - store.AddTx(newKeyedMockTxWithPubKey(signer, 0)) - require.Equal(t, 2, store.Len()) - - // committing nonce 1 drops nonce 1, keeps 2 - require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signer, 1))) - require.Equal(t, 1, store.Len()) - store.AddTx(newKeyedMockTxWithPubKey(signer, 1)) - require.Equal(t, 1, store.Len()) -} - -func TestCosmosTxStorePruneCommittedMultiSignerOnClone(t *testing.T) { - store := NewCosmosTxStore(log.NewNopLogger()) - - signerA := newPubKeyBytes(t) - signerB := newPubKeyBytes(t) - store.AddTx(newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{0, 0})) - - clone := store.Clone() - require.Equal(t, 1, clone.PruneCommitted(newKeyedMockTxWithPubKey(signerA, 0))) - require.Equal(t, 0, clone.Len()) - require.Equal(t, 1, store.Len(), "pruning the clone must not touch the source") } -// Unkeyed txs must not be carried across heights: they get a fresh key on -// every AddTx and cannot be removed, so a carried copy duplicates every pass. func TestCosmosTxStoreCloneDropsUnkeyed(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) @@ -453,10 +368,9 @@ func TestCosmosTxStoreSignerIndexTracksBuckets(t *testing.T) { require.Len(t, store.signerBuckets[keyA], 2) require.Len(t, store.signerBuckets[keyB], 2) - // Committing A's nonce 9 empties both of A's buckets: its own (nonces 0 and - // 1 fall under the watermark) and the shared one. B's own bucket is a - // different signer set and survives untouched. - require.Equal(t, 3, store.PruneCommitted(newKeyedMockTxWithPubKey(pubA, 9))) + // Invalidating from A's nonce 0 empties both of A's buckets: its own and + // the shared one. B's own bucket is a different signer set and survives. + require.Equal(t, 3, store.InvalidateFrom(newKeyedMockTxWithPubKey(pubA, 0))) requireSignerIndexConsistent(t, store) require.NotContains(t, store.signerBuckets, keyA, "A has no bucket left to scan") require.Len(t, store.signerBuckets[keyB], 1) @@ -482,7 +396,7 @@ func TestCosmosTxStoreCloneSignerIndexIsIndependent(t *testing.T) { requireSignerIndexConsistent(t, clone) // emptying every bucket the clone has must leave the source's index whole - require.Equal(t, 2, clone.PruneCommitted(newKeyedMockTxWithPubKey(pubA, 3))) + require.Equal(t, 2, clone.InvalidateFrom(newMultiKeyedMockTx([][]byte{pubA, pubB}, []uint64{0, 0}))) require.Equal(t, 0, clone.Len()) require.Empty(t, clone.signerBuckets) requireSignerIndexConsistent(t, store) @@ -551,55 +465,6 @@ func TestCosmosTxStoreInvalidateReplaced(t *testing.T) { require.Equal(t, 0, store.Len()) } -// Committing an unordered tx must not watermark the signer — that would -// blacklist their ordered txs. Only the exact tx is dropped. -func TestCosmosTxStorePruneCommittedUnordered(t *testing.T) { - store := NewCosmosTxStore(log.NewNopLogger()) - - signer := newPubKeyBytes(t) - timeout := time.Unix(1_700_000_000, 0) - unordered := newUnorderedMockTxWithPubKey(signer, timeout) - earlier := newUnorderedMockTxWithPubKey(signer, timeout.Add(-time.Second)) - ordered := newKeyedMockTxWithPubKey(signer, 5) - - store.AddTx(unordered) - store.AddTx(earlier) - store.AddTx(ordered) - require.Equal(t, 3, store.Len()) - - // only the committed unordered tx is dropped, not the signer's other txs - require.Equal(t, 1, store.PruneCommitted(unordered)) - require.Equal(t, 2, store.Len()) - - // no watermark was recorded: the signer's ordered txs stay addable - store.AddTx(newKeyedMockTxWithPubKey(signer, 6)) - require.Equal(t, 3, store.Len()) -} - -// A committed single-signer tx must evict a pooled multi-signer tx that shares -// that signer/nonce — the exact case the deferred-removal comment warns about. -func TestCosmosTxStorePruneCommittedMultiSigner(t *testing.T) { - store := NewCosmosTxStore(log.NewNopLogger()) - - signerA := newPubKeyBytes(t) - signerB := newPubKeyBytes(t) - - // a tx signed by both A@0 and B@0 - multi := newMultiKeyedMockTx([][]byte{signerA, signerB}, []uint64{0, 0}) - store.AddTx(multi) - require.Equal(t, 1, store.Len()) - - // committing A@0 (single signer) must drop the multi-signer tx - require.Equal(t, 1, store.PruneCommitted(newKeyedMockTxWithPubKey(signerA, 0))) - require.Equal(t, 0, store.Len()) - - // and it stays out even if a recheck tries to re-add it - store.AddTx(multi) - require.Equal(t, 0, store.Len()) -} - -// AddTx overwrites the tx occupying a signer/nonce slot rather than dropping the -// update, so a carried-forward store reflects the latest tx for that slot. func TestCosmosTxStoreAddOverwritesSlot(t *testing.T) { store := NewCosmosTxStore(log.NewNopLogger()) @@ -859,13 +724,4 @@ func TestCosmosTxStoreValidatedAtDroppedOnRemoval(t *testing.T) { store = newStore(tx3, tx4) require.Equal(t, 2, store.InvalidateFrom(tx3)) assertDropped(store, tx3, tx4) - - // PruneCommitted watermarks and drops at-or-below entries - store = newStore(tx3, tx4) - require.Equal(t, 1, store.PruneCommitted(tx3)) - assertDropped(store, tx3) - - // a consumed tx must not re-enter via the fast path either - store.AddTx(tx3) - assertDropped(store, tx3) } From f2f879607af153693e140c0a06fe5ab420dec385 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Aug 2026 23:03:48 +0800 Subject: [PATCH 19/22] add 200-tx recheck and proposal perf harness logs per-pass and per-proposal timings --- tests/integration/mempool/test_perf.go | 67 ++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 tests/integration/mempool/test_perf.go diff --git a/tests/integration/mempool/test_perf.go b/tests/integration/mempool/test_perf.go new file mode 100644 index 000000000..5bbe4a24c --- /dev/null +++ b/tests/integration/mempool/test_perf.go @@ -0,0 +1,67 @@ +package mempool + +import ( + "math/big" + "time" + + abci "github.com/cometbft/cometbft/abci/types" + + evmmempool "github.com/cosmos/evm/mempool" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// TestPerfRecheckAndProposal logs recheck-pass and steady-state +// PrepareProposal timings over 200 cosmos txs. +func (s *IntegrationTestSuite) TestPerfRecheckAndProposal() { + const ( + signers = 20 + noncesPerSigner = 10 + iters = 10 + gasLimit = 200000 + ) + + kMp, ok := s.network.App.GetMempool().(*evmmempool.Mempool) + if !ok { + s.T().Skip("EVM mempool not configured") + } + + txs := make([]sdk.Tx, 0, signers*noncesPerSigner) + for nonce := range noncesPerSigner { + for i := range signers { + txs = append(txs, s.createCosmosSendTxWithNonceAndGas( + s.keyring.GetKey(i), uint64(nonce), big.NewInt(1000), gasLimit, big.NewInt(1000000000), + )) + } + } + s.Require().NoError(s.insertTxs(txs)) + + bench := func(fn func()) time.Duration { + fn() // warm + start := time.Now() + for range iters { + fn() + } + return time.Since(start) / iters + } + + head := kMp.GetBlockchain().CurrentBlock() + perPass := bench(func() { kMp.RecheckCosmosTxs(head) }) + + _, err := s.network.FinalizeBlock() + s.Require().NoError(err) + + height := s.network.GetContext().BlockHeight() + 1 + perProposal := bench(func() { + res, err := s.network.App.PrepareProposal(&abci.RequestPrepareProposal{ + MaxTxBytes: 10_000_000, + Height: height, + }) + s.Require().NoError(err) + s.Require().Len(res.Txs, len(txs)) + }) + + perTx := time.Duration(len(txs)) + s.T().Logf("PERF txs=%d recheck_pass=%v (%v/tx) proposal=%v (%v/tx)", + len(txs), perPass, perPass/perTx, perProposal, perProposal/perTx) +} From 37a684224190121e6e504487600f55ecc8b63ae6 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 4 Sep 2026 03:40:10 +0800 Subject: [PATCH 20/22] fix: stamp recheck snapshot with the validated state's height The pass validates against the pinned context but stamped entries with the trigger header's number, which can run one block ahead. A proposal at that base would then skip re-verifying txs the block already committed. doc deprecated EIP-712 decorator, unlike x/auth's, skips its sequence check in recheck mode doc what the recheck flag changes for ibc-go's relay decorator --- mempool/recheck_pool.go | 23 +++++++++++++++++------ mempool/recheck_pool_test.go | 28 +++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/mempool/recheck_pool.go b/mempool/recheck_pool.go index fd3ec540b..6c1e46cfe 100644 --- a/mempool/recheck_pool.go +++ b/mempool/recheck_pool.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/evm/mempool/internal/heightsync" "github.com/cosmos/evm/mempool/internal/reaplist" "github.com/cosmos/evm/mempool/reserver" + "github.com/cosmos/evm/utils" "cosmossdk.io/log/v2" "cosmossdk.io/math" @@ -443,9 +444,15 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header } m.rechecker.Update(latestCtx, newHead) - // stamp the snapshot only once validation actually runs against this - // height's state (see CosmosTxStore.SetHeight) - m.recheckedTxs.Do(func(store *CosmosTxStore) { store.SetHeight(newHead.Number.Uint64()) }) + // Stamp with the height of the state we validate against, not newHead's: + // a commit landing between the two reads leaves newHead one ahead, and a + // stamp from it would let a proposal skip re-verifying committed txs. + validatedHeight, err := utils.SafeUint64(latestCtx.BlockHeight()) + if err != nil { + m.logger.Error("invalid block height on recheck context", "err", err) + return + } + m.recheckedTxs.Do(func(store *CosmosTxStore) { store.SetHeight(validatedHeight) }) failedAtSequence := make(map[string]uint64) removeTxs := make([]sdk.Tx, 0) @@ -483,9 +490,13 @@ func (m *RecheckMempool) runRecheck(done chan struct{}, newHead *ethtypes.Header keepFuturesOnError := false if !invalidTx { ctx, write := m.rechecker.GetContext() - // Signatures were verified on insert and the bytes have not changed, - // so recheck mode lets sigverify skip the crypto, state-dependent - // checks (sequence, fees, balances) still run. + // Bytes are unchanged since insert verified them, so recheck mode + // skips the crypto while sequence, fee and balance checks still + // run. That holds for x/auth's decorator; the deprecated EIP-712 + // one skips its sequence check too, stranding committed txs here. + // ibc-go's relay decorator likewise skips packet proof and client + // message verification in this mode; its redundancy eviction was + // already active, since query contexts are CheckTx contexts. _, err := m.rechecker.RecheckCosmos(ctx.WithIsReCheckTx(true), txn) if err == nil { write() diff --git a/mempool/recheck_pool_test.go b/mempool/recheck_pool_test.go index 12f0a4d75..68d9a5dc8 100644 --- a/mempool/recheck_pool_test.go +++ b/mempool/recheck_pool_test.go @@ -848,24 +848,42 @@ func TestRecheckMempool_CarryForwardSurvivesCancellation(t *testing.T) { close(gate) } +// The stamp must be the height of the state a pass validates against, not the +// trigger header's, which can run one ahead. +func TestRecheckMempool_SnapshotStampFollowsValidatedState(t *testing.T) { + ctx := newRecheckTestContext().WithBlockHeight(5) + mp := newStartedRecheckMempool(t, ctx, nil, noopAnteHandler) + + key, err := crypto.GenerateKey() + require.NoError(t, err) + tx := newRecheckTestTx(t, key) + require.NoError(t, mp.Insert(ctx, tx)) + + // trigger with a header one ahead of the state the pass validates against + mp.TriggerRecheckSync(testHeader(6)) + + height, ok := mp.SnapshotValidatedAt(tx) + require.True(t, ok) + require.Equal(t, uint64(5), height, "stamp must follow the validated state, not the trigger header") +} + func newStartedRecheckMempool( t *testing.T, ctx sdk.Context, cfg *sdkmempool.PriorityNonceMempoolConfig[sdkmath.Int], ante sdk.AnteHandler, -) (*mempool.RecheckMempool, *heightsync.HeightSync[mempool.CosmosTxStore]) { +) *mempool.RecheckMempool { t.Helper() - recheckedTxs := newTestRecheckedTxs() mp := mempool.NewRecheckMempool( cfg, 0, reserver.NewReservationTracker().NewHandle(1), newMockRechecker(ctx, ante), - recheckedTxs, newTestReapList(), newTestBlockchain(t, ctx), log.NewNopLogger(), + newTestRecheckedTxs(), newTestReapList(), newTestBlockchain(t, ctx), log.NewNopLogger(), ) mp.Start(testHeader(0)) t.Cleanup(func() { require.NoError(t, mp.Close()) }) - return mp, recheckedTxs + return mp } func setupEVMChainConfig(t *testing.T) client.TxConfig { @@ -1178,7 +1196,7 @@ func customReplacementConfig() *sdkmempool.PriorityNonceMempoolConfig[sdkmath.In // InvalidateFrom(newTx) never visits, only replacement hook can drop it and txs stacked on its nonces. func TestRecheckMempool_ReplacementWithDifferentSignerSetInvalidatesRechecked(t *testing.T) { ctx := newRecheckTestContext() - mp, _ := newStartedRecheckMempool(t, ctx, customReplacementConfig(), noopAnteHandler) + mp := newStartedRecheckMempool(t, ctx, customReplacementConfig(), noopAnteHandler) sender, err := crypto.GenerateKey() require.NoError(t, err) From db0b087a983b71db74581cf0d82f9489677ab53f Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 4 Sep 2026 03:40:11 +0800 Subject: [PATCH 21/22] fix: tie the pinned header cache to the pin it was built from Nothing ordered the cache store against its invalidation, so a rebuild racing a pin refresh could land after it and serve a header a block behind. Build from the pinned context and tag the cache with a pin generation that readers check. --- mempool/blockchain.go | 58 +++++++++++++++++++++++++++----------- mempool/blockchain_test.go | 35 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/mempool/blockchain.go b/mempool/blockchain.go index 98384ff78..350dd4eed 100644 --- a/mempool/blockchain.go +++ b/mempool/blockchain.go @@ -44,15 +44,24 @@ type Blockchain struct { blockGasLimit uint64 previousHeaderHash common.Hash latestCtx sdk.Context - mu sync.RWMutex - coinInfo atomic.Pointer[evmtypes.EvmCoinInfo] + // pinGen counts pin updates, tying a cached header to the pin it came from + pinGen uint64 + mu sync.RWMutex + coinInfo atomic.Pointer[evmtypes.EvmCoinInfo] - // pinnedHeader caches the header for the current pin generation, setLatestContext invalidates it - pinnedHeader atomic.Pointer[types.Header] + // pinnedHeader caches the header built from the pinned context; readers + // ignore it once its generation tag is stale + pinnedHeader atomic.Pointer[pinnedHeader] testingCommitMu sync.RWMutex } +// pinnedHeader is a header tagged with the pin generation it was built from. +type pinnedHeader struct { + gen uint64 + header *types.Header +} + // NewBlockchain creates a new Blockchain instance that bridges Cosmos SDK state with Ethereum mempools. // The getCtxCallback function provides access to Cosmos SDK contexts at different heights, vmKeeper manages EVM state, // and feeMarketKeeper handles fee market operations like base fee calculations. @@ -92,10 +101,14 @@ func (b *Blockchain) CurrentBlock() *types.Header { if err != nil { return b.zeroHeader } + return b.headerFromContext(ctx, b.getPreviousHeaderHash()) +} +// headerFromContext constructs the header for the state in ctx, with +// previousHeaderHash as its parent. +func (b *Blockchain) headerFromContext(ctx sdk.Context, previousHeaderHash common.Hash) *types.Header { blockHeight := ctx.BlockHeight() // prevent the reorg from triggering after a restart since previousHeaderHash is stored as an in-memory variable - previousHeaderHash := b.getPreviousHeaderHash() if blockHeight > 1 && previousHeaderHash == (common.Hash{}) { return b.zeroHeader } @@ -256,28 +269,41 @@ func (b *Blockchain) setPreviousHeaderHash(h common.Hash) { b.mu.Lock() defer b.mu.Unlock() b.previousHeaderHash = h + b.pinGen++ } func (b *Blockchain) setLatestContext(ctx sdk.Context) { b.mu.Lock() defer b.mu.Unlock() b.latestCtx = ctx - b.pinnedHeader.Store(nil) + b.pinGen++ } -// PinnedHeader returns the current block header, cached per pin generation -// (headers only change at commit, and the pin refreshes right after). Use it -// on hot paths that tolerate pin-refresh granularity, CurrentBlock always -// rebuilds fresh. +// PinnedHeader returns the header for the pinned context, cached until the pin +// refreshes, so a rebuild that raced with a refresh is never served. Use it on +// hot paths that tolerate that granularity; CurrentBlock always rebuilds from +// the latest committed state. Before the first pin it falls back to +// CurrentBlock and caches nothing. func (b *Blockchain) PinnedHeader() *types.Header { - if h := b.pinnedHeader.Load(); h != nil { - return h + b.mu.RLock() + gen, ctx, previousHeaderHash := b.pinGen, b.latestCtx, b.previousHeaderHash + b.mu.RUnlock() + + if ctx.Context() == nil { + return b.CurrentBlock() + } + if cached := b.pinnedHeader.Load(); cached != nil && cached.gen == gen { + return cached.header } - h := b.CurrentBlock() - if h != b.zeroHeader { - b.pinnedHeader.Store(h) + + // keeper reads charge the shared pinned context's gas meter, so branch it + ctx, _ = ctx.CacheContext() + ctx = ctx.WithGasMeter(sdktypes.NewInfiniteGasMeter()) + header := b.headerFromContext(ctx, previousHeaderHash) + if header != b.zeroHeader { + b.pinnedHeader.Store(&pinnedHeader{gen: gen, header: header}) } - return h + return header } // GetLatestContext returns the latest context as updated by the block, diff --git a/mempool/blockchain_test.go b/mempool/blockchain_test.go index ffc7f25d7..26861a551 100644 --- a/mempool/blockchain_test.go +++ b/mempool/blockchain_test.go @@ -3,6 +3,7 @@ package mempool_test import ( "math/big" "sync" + "sync/atomic" "testing" "time" @@ -130,3 +131,37 @@ func TestBlockchainRaceCondition(t *testing.T) { require.NoError(t, err) require.NotNil(t, stateDB) } + +// PinnedHeader must follow the pin: the live state until the first pin, then +// the pinned header until the pin refreshes. +func TestBlockchainPinnedHeaderFollowsPin(t *testing.T) { + setupEVMChainConfig(t) + + mockVMKeeper := mocks.NewVMKeeperI(t) + mockFeeMarketKeeper := mocks.NewFeeMarketKeeper(t) + mockVMKeeper.On("GetBaseFee", mock.Anything).Return(big.NewInt(1000000000)).Maybe() + mockFeeMarketKeeper.On("GetBlockGasWanted", mock.Anything).Return(uint64(0)).Maybe() + + // the live state, advanced by the test as if blocks were committed + var height atomic.Int64 + getCtxCallback := func(int64, bool) (sdk.Context, error) { + return createMockContext().WithBlockHeight(height.Load()), nil + } + blockchain := mempool.NewBlockchain(getCtxCallback, log.NewNopLogger(), mockVMKeeper, mockFeeMarketKeeper, 21000000) + + // before the first pin the live state is served and nothing is cached + require.Equal(t, int64(0), blockchain.PinnedHeader().Number.Int64()) + height.Store(1) + require.Equal(t, int64(1), blockchain.PinnedHeader().Number.Int64()) + + // the pin at height 1 is served even after the live state moves on + blockchain.NotifyNewBlock() + require.Equal(t, int64(1), blockchain.PinnedHeader().Number.Int64()) + height.Store(2) + require.Equal(t, int64(2), blockchain.CurrentBlock().Number.Int64()) + require.Equal(t, int64(1), blockchain.PinnedHeader().Number.Int64(), "cached header must follow the pin, not the live state") + + // a refreshed pin replaces the cached header + blockchain.NotifyNewBlock() + require.Equal(t, int64(2), blockchain.PinnedHeader().Number.Int64()) +} From 2cead9d170f71bcda8b74ced82e6e6f6d6247763 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 4 Sep 2026 03:40:12 +0800 Subject: [PATCH 22/22] fix: replay encoded proposal txs ahead of a stale same-signer tx Encoding a tx validated at base leaves its ante effects out of the prepare-proposal state, so a stale tx of the same signer selected later failed its sequence check and was dropped, capping backlogged proposals. Queue encoded cosmos txs and verify them ahead of the next stale one. The verifier takes its base verifier and snapshot as interfaces so this is unit tested without a BaseApp. --- evmd/mempool.go | 2 +- evmd/tx_verifier.go | 97 ++++++++++++++++++++----- evmd/tx_verifier_test.go | 148 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 evmd/tx_verifier_test.go diff --git a/evmd/mempool.go b/evmd/mempool.go index e793f4cc6..1a8ac33a0 100644 --- a/evmd/mempool.go +++ b/evmd/mempool.go @@ -59,7 +59,7 @@ func (app *EVMD) configureEVMMempool(appOpts servertypes.AppOptions, logger log. // at the height this proposal builds on (see SnapshotVerifiedTxVerifier). // The base comes from the ABCI request, not the notify-driven pin, which // can lag a beat behind the last commit. - verifier := NewSnapshotVerifiedTxVerifier(app.BaseApp, mempool) + verifier := NewSnapshotVerifiedTxVerifier(app.BaseApp, mempool, logger) proposalHandler := baseapp.NewDefaultProposalHandler(mempool, verifier) defaultPrepareProposal := proposalHandler.PrepareProposalHandler() prepareProposalHandler := func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) { diff --git a/evmd/tx_verifier.go b/evmd/tx_verifier.go index f221d2ebb..2d5437d0c 100644 --- a/evmd/tx_verifier.go +++ b/evmd/tx_verifier.go @@ -1,9 +1,12 @@ package evmd import ( + "sync" "sync/atomic" - evmmempool "github.com/cosmos/evm/mempool" + evmtypes "github.com/cosmos/evm/x/vm/types" + + "cosmossdk.io/log/v2" "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" @@ -11,34 +14,94 @@ import ( var _ baseapp.ProposalTxVerifier = &SnapshotVerifiedTxVerifier{} +// ProposalSnapshot reports whether the mempool validated a proposal candidate +// at the height the proposal builds on (see Mempool.ProposalTxValidatedAt). +type ProposalSnapshot interface { + ProposalTxValidatedAt(tx sdk.Tx, base uint64) bool +} + // SnapshotVerifiedTxVerifier re-runs ante over a proposal candidate only when -// the mempool cannot show it was validated at the height the proposal builds -// on. The base is set per proposal from the ABCI request, so a lagging -// recheck pin fails closed into re-verification. +// the mempool cannot show it was validated at the proposal's base height, set +// per proposal from the ABCI request so a lagging pin fails closed. +// +// Encoding skips the ante effects (sequence bump, fee deduction) that a later +// stale tx of the same signer needs, so encoded cosmos txs are queued and +// verified ahead of the next stale one. Proposals without stale txs pay +// nothing; a mixed one pays once per encoded tx, bounded by block capacity. type SnapshotVerifiedTxVerifier struct { - *baseapp.BaseApp - mempool *evmmempool.Mempool + // ProposalTxVerifier runs full verification and the tx codec; evmd wires the BaseApp. + baseapp.ProposalTxVerifier + snapshot ProposalSnapshot + logger log.Logger - // proposalBase is the last committed height the in-flight proposal builds - // on (req.Height - 1), set by the prepare-proposal handler before txs are - // verified. Zero means unknown and re-verifies everything. + // proposalBase is the height the in-flight proposal builds on + // (req.Height - 1). Zero means unknown and re-verifies everything. proposalBase atomic.Int64 + + // encoded holds cosmos txs encoded without verification, in selection + // order, until their ante effects reach the prepare-proposal state. + mu sync.Mutex + encoded []sdk.Tx } -func NewSnapshotVerifiedTxVerifier(b *baseapp.BaseApp, mempool *evmmempool.Mempool) *SnapshotVerifiedTxVerifier { - return &SnapshotVerifiedTxVerifier{BaseApp: b, mempool: mempool} +func NewSnapshotVerifiedTxVerifier(base baseapp.ProposalTxVerifier, snapshot ProposalSnapshot, logger log.Logger) *SnapshotVerifiedTxVerifier { + return &SnapshotVerifiedTxVerifier{ + ProposalTxVerifier: base, + snapshot: snapshot, + logger: logger.With(log.ModuleKey, "SnapshotVerifiedTxVerifier"), + } } -// SetProposalBase records the height the next proposal builds on. +// SetProposalBase records the height the next proposal builds on. Its queue +// starts empty: the prepare-proposal state is rebuilt per proposal. func (txv *SnapshotVerifiedTxVerifier) SetProposalBase(height int64) { txv.proposalBase.Store(height) + txv.mu.Lock() + txv.encoded = nil + txv.mu.Unlock() } -// PrepareProposalVerifyTx encodes txs validated at the proposal's base height -// and defers to BaseApp's full ante verification for stale or unknown ones. +// PrepareProposalVerifyTx encodes txs validated at the proposal's base height, +// and fully verifies stale or unknown ones after landing the ante effects of +// the txs encoded before them. func (txv *SnapshotVerifiedTxVerifier) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { - if base := txv.proposalBase.Load(); base > 0 && txv.mempool.ProposalTxValidatedAt(tx, uint64(base)) { - return txv.TxEncode(tx) + if base := txv.proposalBase.Load(); base > 0 && txv.snapshot.ProposalTxValidatedAt(tx, uint64(base)) { + bz, err := txv.TxEncode(tx) + if err == nil && !isEVMTx(tx) { + txv.mu.Lock() + txv.encoded = append(txv.encoded, tx) + txv.mu.Unlock() + } + return bz, err + } + txv.verifyEncoded() + return txv.ProposalTxVerifier.PrepareProposalVerifyTx(tx) +} + +// verifyEncoded replays the queued txs through full verification so their ante +// effects reach the prepare-proposal state. They passed at base, so a failure +// is only logged: the stale tx behind it then fails on its own and is skipped. +func (txv *SnapshotVerifiedTxVerifier) verifyEncoded() { + txv.mu.Lock() + queued := txv.encoded + txv.encoded = nil + txv.mu.Unlock() + + for _, tx := range queued { + if _, err := txv.ProposalTxVerifier.PrepareProposalVerifyTx(tx); err != nil { + txv.logger.Warn("encoded proposal tx failed verification at the proposal base", "err", err) + } + } +} + +// isEVMTx reports whether tx carries a single MsgEthereumTx. Such txs are +// never queued: the reserver keeps an address in one pool at a time, so they +// never precede a stale cosmos tx of the same signer. +func isEVMTx(tx sdk.Tx) bool { + msgs := tx.GetMsgs() + if len(msgs) != 1 { + return false } - return txv.BaseApp.PrepareProposalVerifyTx(tx) + _, ok := msgs[0].(*evmtypes.MsgEthereumTx) + return ok } diff --git a/evmd/tx_verifier_test.go b/evmd/tx_verifier_test.go new file mode 100644 index 000000000..5e6ac4f24 --- /dev/null +++ b/evmd/tx_verifier_test.go @@ -0,0 +1,148 @@ +package evmd + +import ( + "testing" + + "github.com/stretchr/testify/require" + protov2 "google.golang.org/protobuf/proto" + + evmtypes "github.com/cosmos/evm/x/vm/types" + + "cosmossdk.io/log/v2" + + "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// stubTx is a minimal sdk.Tx whose identity is its pointer. +type stubTx struct { + msgs []sdk.Msg +} + +func (s *stubTx) GetMsgs() []sdk.Msg { return s.msgs } +func (s *stubTx) GetMsgsV2() ([]protov2.Message, error) { return nil, nil } + +// recordingVerifier records which txs reached full verification and which +// were only encoded. +type recordingVerifier struct { + baseapp.ProposalTxVerifier // unused methods + verified []sdk.Tx + encoded []sdk.Tx +} + +func (v *recordingVerifier) PrepareProposalVerifyTx(tx sdk.Tx) ([]byte, error) { + v.verified = append(v.verified, tx) + return []byte("verified"), nil +} + +func (v *recordingVerifier) TxEncode(tx sdk.Tx) ([]byte, error) { + v.encoded = append(v.encoded, tx) + return []byte("encoded"), nil +} + +// stampedSnapshot maps txs to the height they were validated at. +type stampedSnapshot map[sdk.Tx]uint64 + +func (s stampedSnapshot) ProposalTxValidatedAt(tx sdk.Tx, base uint64) bool { + height, ok := s[tx] + return ok && height == base +} + +func TestSnapshotVerifiedTxVerifier(t *testing.T) { + const base = int64(10) + + var ( + fresh1 = &stubTx{} + fresh2 = &stubTx{} + stale1 = &stubTx{} + stale2 = &stubTx{} + evm = &stubTx{msgs: []sdk.Msg{&evmtypes.MsgEthereumTx{}}} + ) + snapshot := stampedSnapshot{ + fresh1: uint64(base), + fresh2: uint64(base), + stale1: uint64(base) - 1, + stale2: uint64(base) - 1, + evm: uint64(base), + } + + testCases := []struct { + name string + base int64 + txs []sdk.Tx + wantVerified []sdk.Tx + wantEncoded []sdk.Tx + }{ + { + name: "txs validated at base are encoded, not verified", + base: base, + txs: []sdk.Tx{fresh1, fresh2}, + wantEncoded: []sdk.Tx{fresh1, fresh2}, + }, + { + name: "encoded txs are verified ahead of the stale tx that follows", + base: base, + txs: []sdk.Tx{fresh1, fresh2, stale1}, + wantVerified: []sdk.Tx{fresh1, fresh2, stale1}, + wantEncoded: []sdk.Tx{fresh1, fresh2}, + }, + { + name: "each encoded tx is replayed once", + base: base, + txs: []sdk.Tx{fresh1, stale1, fresh2, stale2}, + wantVerified: []sdk.Tx{fresh1, stale1, fresh2, stale2}, + wantEncoded: []sdk.Tx{fresh1, fresh2}, + }, + { + name: "an unknown base verifies everything", + base: 0, + txs: []sdk.Tx{fresh1, stale1}, + wantVerified: []sdk.Tx{fresh1, stale1}, + }, + { + name: "evm txs are encoded but never replayed", + base: base, + txs: []sdk.Tx{evm, stale1}, + wantVerified: []sdk.Tx{stale1}, + wantEncoded: []sdk.Tx{evm}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + recorder := &recordingVerifier{} + verifier := NewSnapshotVerifiedTxVerifier(recorder, snapshot, log.NewNopLogger()) + verifier.SetProposalBase(tc.base) + + for _, tx := range tc.txs { + _, err := verifier.PrepareProposalVerifyTx(tx) + require.NoError(t, err) + } + + require.Equal(t, tc.wantVerified, recorder.verified) + require.Equal(t, tc.wantEncoded, recorder.encoded) + }) + } +} + +// A new proposal rebuilds the prepare-proposal state, so txs encoded for the +// previous proposal must not be replayed into it. +func TestSnapshotVerifiedTxVerifier_NewProposalDropsQueue(t *testing.T) { + const base = int64(10) + fresh := &stubTx{} + stale := &stubTx{} + snapshot := stampedSnapshot{fresh: uint64(base), stale: uint64(base) - 1} + + recorder := &recordingVerifier{} + verifier := NewSnapshotVerifiedTxVerifier(recorder, snapshot, log.NewNopLogger()) + + verifier.SetProposalBase(base) + _, err := verifier.PrepareProposalVerifyTx(fresh) + require.NoError(t, err) + require.Empty(t, recorder.verified) + + verifier.SetProposalBase(base) + _, err = verifier.PrepareProposalVerifyTx(stale) + require.NoError(t, err) + require.Equal(t, []sdk.Tx{stale}, recorder.verified, "previous proposal's queue must not be replayed") +}