Conversation
…context Admission and recheck shared baseapp's checkState as the pending-nonce store. Give the app mempool its own CacheMultiStore, branched off the committed store and refreshed inside the existing admission-mutex span at Commit, and pass it as RunTx's txMultiStore at all three call sites (admit, CheckTxHandler, runRecheck). All three must move together: the branch is the sole nonce authority, so a split would leave one path reading state reset at every Commit. A generation counter lets an in-flight recheck pass abandon candidates validated against a superseded branch; the unreached candidates' senders are re-merged into staging so the next pass re-covers them.
…gap evictions runRecheck took stateMu once per candidate, so an admission could land between two txs of the same sender and make evictions timing-dependent. Bucket candidates by the signer the mempool orders by and take stateMu once per group: a sender's nonce chain now advances atomically against other senders' admissions, while the hold time stays bounded by that sender's queue depth instead of the whole batch. Encoding moves out of the lock, and the generation check now runs under stateMu before each group, so a group is never split mid-flight. On a nonce failure, evict the remaining higher-nonce siblings without a RunTx each. Only when the gap is provable: an earlier tx in the group passed this pass, so lastOK+1 is the expected nonce, and the failing nonce is strictly above it. A wrong-sequence failure can also mean a stale (already committed) nonce, whose successor may be valid — cascading there would evict good txs. Disabled for any group that isn't the signer's contiguous ascending view.
Manager grew into one struct holding admission, recheck staging, selection, and the shared execution state. Split it along the boundary the branched context made explicit: - exec.go: txExec owns the admission mutex, mempoolState, the generation counter, and the codecs. Both halves run txs through it, so this state belongs to neither alone. - admitter.go: admit, InsertTx/CheckTx handlers, cacheTx. - scheduler.go: sender staging, candidate selection, TTL/timeout eviction, recheck grouping, and the async worker. Manager is now a facade over the three, so app.go and the proposal handler call sites are unchanged. Lock order is unchanged: recheckMu > txExec.mu > stagingMu, with mempoolState.mu innermost.
The fast path (mempool.type=app with the encoder cache) trusts admission and recheck, so it only encodes each pooled tx instead of re-running the ante like the default handler does. Nothing captured what that buys or costs. Run both handlers over identically seeded pools and assert the boundary: - all-valid pool and a same-sender nonce gap: identical selections and pools, since the gap guard lives in the shared DefaultProposalHandler sequence tracking. - stale nonce, recheck backlog, timeout height: the fast path proposes txs the ante rejects and leaves them pooled for recheck instead of evicting them mid-proposal. - baseFee drift: selections match because the proposal gate replaces the ante's fee check; only the pool differs, as a gated tx stays pooled. Each divergent case also runs the real ProcessProposal over the fast path's proposal with a non-empty blocklist and asserts ACCEPT: cronos ProcessProposal is blocklist-only, so an ante-invalid tx cannot make peers reject the block. Pooled txs are a local diffTx carrying its own signer, nonce, fee, gas, and timeout, so no account keeper or real codec is needed.
Sort recheck groups ascending by seq and disable cascade for co-signers of a multi-signer tx, whose nonces the keyed group cannot see.
The flat batch cap could hand a sender's higher-nonce txs to a later cycle without their prefix, so they failed wrong-sequence against a freshly rebranched base and were evicted while valid. Cap whole groups instead, run each group in bounded chunks so a deep queue can't stall Commit, and read the generation counter after the pool scan rather than before it.
The deferred carry is keyed on tx identity, so a fee bump replacing a carried tx at the same nonce dropped it from the next cycle's group and took the live tail down as a false wrong-sequence failure; carry the senders too. Also keep cascade eviction inside the chunked mutex hold, and tighten the recheck test runner to reject stale nonces like the real ante does.
cascadeChunkLocked now spends one RunTx on a chunk's head before blind- evicting the rest, since the lock releases between chunks and a same- sender admission can fill a gap proven in an earlier chunk. Also guard unordered txs out of cascadable grouping, and carry unreached senders into deferred on a gen-abort so a low-priority tail can't be starved by sustained aborts.
Both eviction paths remove a tx from the pool without spending a RunTx, so the EVM ante's per-(sender, nonce) admission cache never learns the slot is free and skips nonce verification on a resubmit at that nonce. Add an eviction hook the scheduler fires with (sender, nonce) on every eviction; wire it to the ante cache's Delete in app.go. Also: cascadeChunkLocked no longer assumes any head failure proves the gap survived — only a nonce error does; other failures (e.g. funds) fall through to per-candidate rechecking for the rest of the chunk.
…ames evict fired the ante nonce-cache hook only for the group's key signer. A multi-MsgEthereumTx tx stages one ante-cache entry per msg, so a second-and-later signer's entry leaked on cascade/TTL eviction the same way round 5 fixed for the key signer. evict now enumerates all signers via GetSigners when the evicted tx is multi-signer.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: JayT106 <JayT106@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+98
to
+100
| for sg := range senders { | ||
| s.recheckSenders[sg] = struct{}{} | ||
| } |
| // Pass 1: evictions. Collect senders of evicted txs so their remaining pool txs | ||
| // (e.g. higher-nonce siblings) are rechecked — they become invalid after the gap. | ||
| var evictedSet map[sdk.Tx]struct{} // nil until first eviction; nil-map read is safe | ||
| now := time.Now() |
Drop mempoolState and the generation counter: the branched store did not change what recheck interleaves with (admission still shares one state under one mutex), and it made Simulate stop seeing pending nonces, since baseapp runs Simulate on checkState and the sequence check is not gated on simulate. All three RunTx sites run on checkState again; CheckTxHandler uses the runTx closure baseapp passes in instead of re-deriving the exec mode. A nonce gap is now only provable against a nonce accepted under the same lock hold. The old cross-chunk cursor and cascadeChunkLocked treated any nonce error at a chunk head as the gap surviving, but a stale nonce (same-sender admissions or a Commit landing between chunks) reports the same error, so they could blind-evict valid siblings.
Resolve app.go and manager.go against main. Adopt main's SetAnteCache (#2177) in place of the eviction hook, since it deletes exactly the per-MsgEthereumTx entries the EVM ante stages, and wire main's pendingTxCache (#2156) into the split txExec so admission and recheck both invalidate it. Rename the design doc to match the final architecture.
JayT106
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Groups app-mempool (
mempool.type=app) recheck candidates per signer, runs each group in bounded chunks under the admission mutex, and cascade-evicts higher-nonce siblings only behind a nonce gap proven in the same lock hold. SplitsManagerintotxExec/admitter/recheckScheduler(app/mempool/exec.go,admitter.go,scheduler.go). Addsapp/proposal_diff_test.go, a differential test between the fast and defaultPrepareProposalpaths.Issue
Recheck released the admission mutex between candidates, so a same-sender
InsertTx/CheckTxcould land mid-chain and make evictions timing-dependent (#2109 item 3). An eviction in the middle of a sender's chain also left its higher-nonce siblings pooled as orphans until TTL (item 4).An earlier draft moved all
RunTxcalls onto a mempool-owned branchedCacheMultiStore. That was dropped: it did not change what recheck interleaves with (admission and recheck still shared one state under one mutex), andBaseApp.Simulateruns oncheckStatewith a sequence check that is not gated on simulate, so pipelined cosmosSimulatecalls stopped seeing pending nonces.checkStatestays the single pending-nonce view, as in the default SDK.Solution
groupCandidatesbuckets by first signer and sorts each group by nonce, so a chain never runs out of order.maxRecheckBatchcaps at group boundaries only; overflow groups carry forward whole, with their senders re-staged so a fee bump replacing a carried tx is still re-picked.recheckChunkSize(256) bounds each mutex hold, so one sender's deep queue cannot stallCommit.lastOK+1with a nonce error. Nothing carries across chunks: after the lock is released, a same-sender admission or a Commit can move the account's nonce, and a nonce error at the next chunk's head then means stale, not gap. Blind-evicting on it would drop valid replacements.evictalso drops ethermint's ante nonce-cache entries (via main'sSetAnteCache, fix(mempool): clear the ante nonce cache on eviction and bound it independently #2177), so a cascade/TTL eviction that never runs the ante cannot leave a stale entry that lets a resubmit skip nonce verification.checkState.Test
go test -tags objstore -mod=mod -race -count=1 ./app/mempool/ ./app/,golangci-lint runclean onapp/mempool,go build -tags objstore -mod=mod ./...clean.New/updated unit tests cover: grouping by signer and nonce order; nonce-gap cascade vs stale-nonce and non-nonce failures; the three chunk-boundary stale-vs-gap cases (table-driven,
TestRunGroup_ChunkBoundaryNeverCarriesGapProof); cascade stopping at a chunk boundary; batch cap carrying a whole group without splitting it; deferred carry surviving a head fee bump; multi-signer / co-signer / unordered / duplicate-seq cascade guards; ante-cache eviction on TTL, recheck failure, and multi-msg txs; pending-cache invalidation on admission, staging, and recheck.app/proposal_diff_test.goseeds two identical pools and asserts where the fast path and the default full-ante handler agree (all-valid, same-sender gap) and where they diverge (stale nonce, recheck backlog, timeout height, baseFee drift), and thatProcessProposalstill accepts every divergent block.Design notes:
docs/architecture/mempool-recheck-grouping.md.