TxMempool rewrite (CON-305)

[pompon0]

Addressed a bunch of issues:

• diverging state between various indices in TxMempool
• \Omega(n) complexity of inserting a transaction above capacity
• delayed promotion of transactions from pending to ready (happens only on Update)
• removed mechanism which was skipping 1 high priority tx in case it did not fit into the current limit - this mechanism was ad hoc and it did not respect nonces

Not addressed:

• UnconfirmedTxs rpc call is still inefficient and hogs mempool
• poor behavior under autobahn: it uses ReapTxs(remove=true) so that producers do not include the same transactions in multiple blocks, but it also makes non-reaped higher nonce transactions pending (lower nonce txs have been reaped, but are not executed yet, so the account nonce did not move). For autobahn we will implement a simpler but more tighlty integrated mempool (because it needs no gossip and all proposals are roughly guaranteed to be sequenced, since every producer has their own lane)

Consistency has been achieved mostly by moving all the data under a single mutex in txStore. Amortized O(log n) updates were achieved by delaying pruning until mempool size reaches 2*capacity (more precisely capacity is counted both in number of txs and in bytes, so 1 enormous transaction can also trigger recomputation of the mempool, in which case we have amortized O(log n) per tx byte instead)

Additional changes:

• PendingSize and Size capacity limits have been merged into 1 limit = PendingSize + Size, same for byte size capacity limits
• removed tracking of peers that a given transaction was received from in favor of just gossiping to everyone. It is expected to increase the actual network traffic just by ~1/p where p is the number of peers.
• fixed node hanging introduced in Removed callbacks from TxMempool #3410

[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	May 21, 2026, 6:58 PM

[cursor]

Stale readyEl entries persist in gossip list after compact

Medium Severity

When compact resets account state and re-inserts transactions, previously-ready EVM txs that lose readiness (e.g., due to reduced balance) retain a stale readyEl pointing into the readyTxs CList. The insert function's promotion loop at line 327 checks !wtx.readyEl.IsPresent() and skips re-adding, but it also never removes the stale element when the tx is no longer promoted. This leaves readyTxs containing entries for pending txs, causing them to be gossiped to all peers and creating an inconsistency between readyTxs.Len() and state.ready.count.

Additional Locations (1)

• sei-tendermint/internal/mempool/tx.go#L317-L330

 

^{Reviewed by Cursor Bugbot for commit 94bbd6c. Configure here.}

[pompon0]

this WAI, readyTxs list accumulates transactions of mempool that were ready at any point in the past. Reinserting them to the list would cause regossip.

[cursor]

Recheck only covers ready txs, misses pending ones

Medium Severity

The Update recheck loop iterates only over txmp.txStore.ReadyTxs(), so pending (not-yet-ready) EVM transactions are never rechecked. If a pending transaction becomes invalid after a block (e.g. the sender's balance dropped), it won't be detected and will remain in the mempool indefinitely until it either expires or becomes ready and gets rechecked later.

 

^{Reviewed by Cursor Bugbot for commit d9b1814. Configure here.}

[pompon0]

that's a backward compatible behavior.

[cursor]

CList.Clear deadlocks calling Next under write lock

High Severity

Clear holds l.mtx.Lock() and calls el.Next(), which acquires el.mtx.RLock(). Meanwhile, another goroutine calling Remove(e) holds l.mtx.Lock() and calls e.setRemoved() which acquires e.mtx.Lock(). But the real issue is that setRemoved() is called within Clear() while l.mtx is already held — and setRemoved also acquires e.mtx.Lock(). If a concurrent broadcastTxRoutine is calling NextWait() which holds e.mtx.RLock() and then tries to call some list operation that needs l.mtx, a lock-ordering deadlock occurs.

 

^{Reviewed by Cursor Bugbot for commit d9b1814. Configure here.}

[pompon0]

that's plain bullshit

[cursor]

Ready count double-incremented for newly promoted replacement tx

Medium Severity

In insert(), when replacing a tx with the same nonce where the old tx was NOT ready (oldReady is false) but the new tx satisfies the balance requirement, the ready count is only incremented once by the promotion loop at line 339. This is correct. However, when the old tx WAS ready and evm.nonce equals account.nextNonce - 1, the replacement code at lines 324–326 increments state.ready for the new tx. Then the promotion loop at lines 332–343 uses a shadowed wtx variable (line 334) and re-finds the same just-inserted tx at its nonce if account.nextNonce was decremented during the Dec/Inc — but account.nextNonce is not modified in the replacement path, so the promotion loop starts past this nonce. This is actually safe, so the ready count is correct for this path. The actual issue: when the old tx was NOT ready (pending) and is being replaced, the old tx's bytes are NOT decremented from state.ready (correctly, since it wasn't ready). But the old tx is deleted from byHash at line 317 without decrementing byNonce-related ready state. Then the promotion loop may find the new tx and promote it — but state.total was already decremented for the old tx at line 319 and re-incremented at line 329. The byNonce map only has one entry (replaced in place at line 330). Actually, on closer re-analysis, the counts appear correct. Disregard the double-count concern.

 

^{Reviewed by Cursor Bugbot for commit b4d042b. Configure here.}

[pompon0]

wtf

[cursor]

Compact pre-checks total without accounting for insert's effect

Medium Severity

In compact(), the limit check at line 436–437 creates a local total copy, increments it by wtx.Size(), and checks against softLimit. But this prospective check does not account for the possibility that insert() might replace an existing tx (nonce collision), which would decrease the total. During compact, byNonce is cleared so nonce collisions shouldn't occur. However, there's a more subtle issue: the prospective total.Inc(wtx.Size()) includes the tx's size, but if insert() fails (e.g., errOldNonce after re-fetching nonces), the actual state is never incremented. Yet the eviction path removes the tx from cache regardless. This means a tx that fails insert due to stale nonces is evicted AND removed from cache, even though the limit check (which assumed the tx would be inserted) was the wrong criterion for eviction.

 

^{Reviewed by Cursor Bugbot for commit b4d042b. Configure here.}

[pompon0]

that's incorrect

[cursor]

Compact evicts valid txs using wrong limit criterion

Medium Severity

In compact(), the prospective limit check (total.Inc(wtx.Size()) then total.LessEqual(&inner.softLimit)) evaluates whether adding this tx would exceed the soft limit. But due to short-circuit evaluation in if !limitOk || s.insert(...) != nil, when limitOk is false, insert() is never called. The eviction path then removes the tx from cache. However, if a previous large tx caused the total to approach the limit, many subsequent smaller high-priority txs could be incorrectly evicted because the prospective check double-counts — it adds the current tx's size to a total that already includes successfully inserted txs, but those txs may have replaced older ones (reducing the actual total). Since compact clears byNonce first, nonce collisions won't occur during reinsertion, so this particular scenario doesn't apply. The real concern is that once a single tx pushes the prospective total over the limit, ALL subsequent txs in wtxs will also fail the check (since the actual state only grows), causing potentially valid lower-priority txs to be evicted unnecessarily.

 

^{Reviewed by Cursor Bugbot for commit b4d042b. Configure here.}

[pompon0]

that's incorrect

[cursor]

Cursor Bugbot has reviewed your changes using default mode and found 1 potential issue.

There are 7 total unresolved issues (including 6 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7a8da94. Configure here.}

[cursor]

Removed context cancel changes shutdown ordering

Low Severity

Removing the second defer cancel() that was placed at the end of the function changes context cancellation timing during shutdown. Previously, cancel() ran first during defer unwinding (LIFO order), signalling all goroutines using goCtx to stop before other resources were torn down. Now, the remaining defer cancel() near the top of the function runs last, meaning gRPC servers, API servers, and the app are closed while goroutines using goCtx may still be running.

 

^{Reviewed by Cursor Bugbot for commit 7a8da94. Configure here.}