mempool: make Update/Reap index maintenance incremental

[masih]

Every block, txStore.Update called compact, which recomputes inInclusionOrder (two O(m log m) sorts over the whole mempool) and then throws away and rebuilds byHash/byEvmHash/byNonce by re-inserting every tx. Reap(remove=true) was worse: it computed the inclusion order to pick txs, then called compact, which computed it a second time. At the ~10k tx mempools seen in production this is tens of milliseconds of serialized work on the consensus path per block.

Update now removes executed/invalid/expired txs in place through a new remove helper and, when the store still fits in softLimit, calls refresh instead of compact: it clears and re-fetches account state, drops txs whose nonce fell below the account nonce, re-caches their metadata and re-derives readiness per account with advanceReady (the loop extracted from insert), all in O(m) with no sort and no index rebuild. nextNonce is still reset to firstNonce and rebuilt by walking byNonce, so readiness semantics are unchanged. Reap(remove=true) removes the reaped txs incrementally and re-derives readiness only for the affected accounts (refreshReady); it only falls back to compact when the store is still above softLimit, where eviction genuinely needs the full order. I did not reuse the pre-removal order for that fallback because reaping the head of an account demotes its successors from ready to pending, so the post-removal order is not a filter of the pre-removal one. Since the RPC snapshot is no longer a byproduct of compact, RecentSnapshot recomputes it lazily when snapshotStale is set.

compactTotal/compactDurationSeconds now only fire when compact actually runs (insert overflow, or when a store above softLimit needs eviction), so those series will drop in normal operation. Reviewers should look most closely at refresh and refreshReady against the previous compact re-insertion path. Existing mempool tests pass unchanged under -race; the new tx_bench_test.go shows Update going from ~20ms to ~9ms at 10k txs (32→9ms at 20k txs over 5k accounts, 121→49ms at 50k) and Reap(remove=true) from ~21ms to ~7ms (50→15ms at 20k), the remainder being the unavoidable per-account state re-fetch and the single inclusion-order sort.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Sep 17, 2026, 1:34 PM

[codecov]

Codecov Report

❌ Patch coverage is 92.30769% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.78%. Comparing base (0b253c2) to head (00db340).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sei-tendermint/internal/mempool/tx.go	92.30%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4240      +/-   ##
==========================================
- Coverage   66.91%   65.78%   -1.14%     
==========================================
  Files        2176     2055     -121     
  Lines      167167   155277   -11890     
==========================================
- Hits       111858   102142    -9716     
+ Misses      55168    52994    -2174     
  Partials      141      141              

Flag	Coverage Δ
sei-chain-pr	91.16% <92.30%> (?)
sei-db	74.50% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
sei-tendermint/internal/mempool/tx.go	93.76% <92.30%> (-1.27%)	⬇️

... and 121 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cursor]

PR Summary

Medium Risk
Touches consensus-path mempool readiness, eviction, and snapshot semantics; behavior is intended to match prior compact paths but reviewers should validate refresh/refreshReady against the old re-insert flow.

Overview
Replaces per-block full compact with incremental mempool index maintenance on the normal path when the store stays within softLimit, cutting repeated O(m log m) inclusion sorts and full index rebuilds on Update and Reap(remove=true).

Update now removes executed/invalid/expired txs via a shared remove helper, then calls refresh (O(m): re-fetch account nonces/balances, drop txs below account nonce, re-derive readiness with advanceReady) instead of always compacting. compact runs only when the store is still above softLimit after removals (eviction still needs full inclusion order).

Reap(remove=true) removes reaped txs incrementally and runs refreshReady only for affected EVM accounts (successor txs become pending until the next block Update); it compacts only if still over softLimit. RPC snapshots are no longer a side effect of compact: snapshotStale + lazy recompute in RecentSnapshot.

Readiness/nonce logic is refactored into account, cacheMetadata, advanceReady, and refreshReady (shared with insert). tx_bench_test.go benchmarks Update, inInclusionOrder, and Reap removal at large account counts.

^{Reviewed by Cursor Bugbot for commit 00db340. Bugbot is set up for automated code reviews on this repo. Configure here.}

[seidroid]

Replacing the per-block compact with incremental remove/refresh/refreshReady looks correct: readiness is still derived by resetting nextNonce to firstNonce and walking byNonce, counter bookkeeping and the RemovedTxs/EvictedTxs/recordPendingNonce* metrics match the old re-insertion path, and the reap-demotion semantics (successors of a reaped head become pending until the next Update) are identical to what compact(inner, false) produced. No blockers; the main gap is test coverage for the new partial-reap path.

Findings: 0 blocking | 3 non-blocking | 1 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• [suggestion] The new refreshReady demotion path has no direct test. The only existing Reap(remove=true) coverage is TestTxMempool_DescendingNonceDrain, which submits nonces in descending order so exactly one tx is ready per block — refreshReady never actually demotes a successor there. Add a txStore-level test that inserts a contiguous ready chain for one account, reaps only its head (MaxTxs < chain length), and asserts the successors moved to pending with correct ready/total count and byte totals, that NextNonce falls back to firstNonce, and that the following Update promotes them again. Same for refresh: a case where the app nonce advances past some mempool txs and another where the balance drops below a mid-chain tx's requiredBalance, asserting the drop/demotion and that byEvmHash/byNonce no longer hold the dropped entries. These are the two functions the PR description itself flags as the ones to scrutinise, and they are the only paths Update/Reap now take in normal operation.
• [suggestion] benchTxStore raises Size, PendingSize, MaxTxsBytes and MaxPendingTxsBytes but leaves TestConfig's CacheSize at 1000 while inserting 10k–50k txs. Since refresh pushes metadata for every surviving tx, each iteration evicts and reallocates ~m LRU nodes that a right-sized cache would only relink, so the reported Update timings include allocation churn production (CacheSize default 10000) would not see at the 10k point. Setting cfg.CacheSize = numAccounts * txsPerAccount (as recheck_drain_test.go does) would make the headline numbers reflect the O(m) refresh work itself.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] RecentSnapshot can now escalate from RLock to the exclusive Lock and run the O(m log m) inInclusionOrder sort inline. Since refresh/Reap set snapshotStale, the first /unconfirmed_txs request after each block pays that sort while holding the write lock, blocking Insert/CheckTx/Reap and the consensus Update for roughly the 9–20 ms the PR description measures at 10k–20k txs. Total work is not higher than before (consensus used to pay it unconditionally), but it is now triggerable at an arbitrary moment by an RPC caller rather than at a point consensus controls.

Two cheaper options: keep the snapshot a byproduct of the inInclusionOrder that Reap already runs under the lock, or compute the ordering into a local slice and publish it with a short critical section. Worth at least a comment recording the intended trade-off.

Separately, this makes RecentSnapshot unsafe to call from any context already holding inner.RLock() (sync.RWMutex is not reentrant). No current caller does, but the previous version was safe there.