fix(vara.eth/mempool): seed head + grace-window unresolved refs#5503
fix(vara.eth/mempool): seed head + grace-window unresolved refs#5503grishasobol wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses three critical bugs in the InjectedTxMempool lifecycle related to transaction expiry and deduplication. By implementing a grace window for unresolved reference blocks and properly seeding the initial chain head height, the changes prevent flaky behavior where transactions are prematurely purged or incorrectly accepted during node startup and observer lag. These improvements ensure that the mempool maintains strict validity invariants while remaining tolerant of expected network and database synchronization delays. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a grace window for mempool and 'seen' entries to handle scenarios where a transaction's reference block has not yet been replicated to the local database. It adds PoolEntry and SeenEntry structs to track the head height at the time of insertion or commitment and seeds the initial head height from the database to ensure the expiry logic is active during cold starts. Comprehensive regression tests were added to verify these changes. I have no feedback to provide.
Three related bugs in the mempool's `latest_head_height` / `purge_expired` lifecycle (corner-case hunt #11, #4, #7): - Cold-start: `is_expired` in `insert` was guarded on `latest_head_height.is_some()` — skipped between process boot and the observer's first `set_chain_head` tick. RPC returned `Accept` for arbitrarily-old txs that the very next chain-head advance silently purged. Fix: seed `latest_head_height` from `db.globals().latest_synced_eb` at construction so the gate is active from boot. - Insert→purge race: `insert` tolerated not-yet-replicated ref_blocks (the producer's EB lags the observer by O(seconds)) but `purge_expired` evicted them on the very next chain-head advance. The local RPC's `Accept` was orphaned. - Forget→purge dedup bypass: when a committed tx's ref_block hadn't replicated to this node's DB, `purge_expired`'s seen-retain loop evicted the seen entry — letting the same network-committed tx re-enter the local pool. Fix for the latter two: keep entries with unresolved ref_block within a `VALIDITY_WINDOW`-block grace period, evicting only once that grace expires. Pool entries carry `inserted_at_head_height`; seen entries carry `seen_at_head_height`. Satisfies both "tolerate observer lag" and the existing `pool_retains_unresolved_ref_block_indefinitely` invariant on bounded capacity exhaustion. Three regression tests added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8f805f0 to
e82387d
Compare
Summary
Closes three related bugs in
InjectedTxMempool'slatest_head_height/purge_expiredlifecycle (surfaced by the corner-case hunt — see PR #5491, log entries iter #11, #4, #7).Cold-start expiry bypass (iter #11)
insert'sis_expiredcheck was guarded onlatest_head_height.is_some():Between process boot and the observer's first
set_chain_headtick,latest_head_heightisNone, so the entire chain short-circuits and expired txs slip through. RPC returnsAccept; the very next chain-head advance silently purges the tx. Users see flaky behaviour every restart.Fix: seed
latest_head_heightfromdb.globals().latest_synced_eb.header.heightinwith_capacity. The DB's last-synced EB is a sound proxy for the chain head while the observer catches up.Insert→purge race on a lagging observer (iter #4)
insertis intentionally tolerant ofref_blocks that haven't yet replicated to this validator's DB (the producer's EB lags the observer by O(seconds) in normal operation):But
purge_expiredwas intolerant — it evicted unknown ref_blocks on the very next chain-head advance via the_ => falsearm. So the local RPC'sAcceptwas immediately orphaned.Forget→purge dedup bypass (iter #7)
Same shape on the
seentable:forget()stamps every committed tx intoseen. When the committed tx's ref_block hadn't replicated to this node's DB, the nextpurge_expiredevicted the seen entry — letting the same network-committed tx re-enter the local pool. Inflates pool occupancy with already-committed work.Fix for #4 and #7: grace-window
Naive "always keep unknown ref_block" breaks the existing
pool_retains_unresolved_ref_block_indefinitelyinvariant (a stream of bogus ref_block txs would permanently exhaust capacity).Real semantics: keep unresolved-ref_block entries for a bounded grace window, evict afterwards:
Pool entries gain
inserted_at_head_height; seen entries gainseen_at_head_height. Both satisfied: insert→purge race tolerated for a window long enough for any sane observer lag; bounded back-pressure for txs whose ref_block never lands.Test plan
#[ignore]:cold_start_insert_rejects_expired_ref_block_using_latest_synced_ebpurge_expired_keeps_tx_with_unresolved_ref_block_within_graceforget_then_purge_keeps_seen_for_unresolved_ref_block_within_gracepool_retains_unresolved_ref_block_indefinitelycontinues to pass (proves the grace expiry actually fires pastVALIDITY_WINDOW).ethexe-malachitetest suite green (59/59 passing).cargo fmt --check -p ethexe-malachiteclean.cargo clippy -p ethexe-malachite --testsclean.Out of scope
VALIDITY_WINDOW. Existing fetch ancestor-filter still keeps it from being included while unwinnable.🤖 Generated with Claude Code