Conversation
…udget
The revalidation walk applied one rolling sender budget across every MorphTx in
the pool, pending and queued alike. A transaction sitting behind a nonce gap was
therefore charged against whatever the sender's executable transactions had left
over — but the transactions filling the gap are not in the pool, so how much of
the balance is actually still owed by the time the gapped one executes is
unknown. An unrelated block was enough to evict a future-nonce transaction that
had passed admission on its own.
Stop the walk at the first nonce discontinuity, which is what upstream's
`AllTransactions::update` does ("If there's a nonce gap, we can shortcircuit,
because there's nothing to update yet"). go-ethereum reaches the same place from
the other direction: `promoteExecutables` only ever applies a per-transaction
cost check to the queue and discards `FilterF`'s `invalids`.
Nothing is lost by leaving those transactions alone: without `NO_NONCE_GAPS`
they sit in the queued sub-pool, which is exactly what reth's own stale eviction
reaps.
Claude-Session: https://claude.ai/code/session_01PiUjd47Da71WG2BFkDQz9q
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks 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 |
Stacked on #202 — review that one first; this PR's base is its branch, so the diff here is just the nonce-gap change.
The problem
The revalidation walk applies one rolling sender budget across every MorphTx in the pool, pending and queued alike. A transaction behind a nonce gap is therefore charged against whatever the sender's executable transactions left over.
That number is meaningless. The transactions filling the gap are not in the pool, so how much of the balance is still owed by the time the gapped transaction executes is unknown — it could be more (the gap transactions spend) or less (they arrive with a refund, or never arrive at all and the gap closes with a cheaper replacement). An unrelated, otherwise empty block was enough to evict a future-nonce transaction that had passed admission on its own merits.
The fix
Stop the walk at the first nonce discontinuity.
This is what upstream already does —
AllTransactions::update,pool/txpool.rs:go-ethereum arrives at the same place from the other direction:
promoteExecutablesonly ever applies a per-transaction cost check to the queue (core/tx_pool.go:1622) and discardsFilterF'sinvalidsreturn value — it never runs a cumulative budget over future nonces.Nothing is lost by leaving these transactions alone. Without
NO_NONCE_GAPSthey live in the queued sub-pool, which is precisely what reth's own stale eviction reaps aftermax_tx_lifetime.Tests
Both confirmed to fail before this change:
a_transaction_behind_a_nonce_gap_is_not_charged_to_the_budget— nonce 0 reserves the sender's whole token balance, nonce 10 is behind a gap; previously nonce 10 was removed.a_sender_holding_only_future_nonces_is_left_alone— a sender with no executable front is not evaluated at all, even with a zero token balance.make lint,cargo test --allandcargo test --docpass.Why the other half of the review finding is not here
The external review also suggested not removing a transaction that is merely unaffordable right now, leaving that to the payload builder (which already skips it via
mark_invalid, correctly and for that block only).That was not done, deliberately. Upstream's stale eviction only scans
queued_transactions()— the basefee and queued sub-pools. A transaction in pending is never time-evicted, so a token-fee transaction whose sender has spent their balance would occupy a pool slot forever, keep being gossiped, and keep skewingeth_getTransactionCount(pending).reth's own answer for the ETH equivalent is park, then reap:
update_accountsclearsENOUGH_BALANCE, the transaction moves to queued, and stale eviction takes it from there. There is no public API to park a specific transaction, so removing the first unaffordable transaction and letting the pool park its descendants — which #202 makes it do — is the closest available approximation, and matches go-ethereum'sdemoteUnexecutablesoutcome.https://claude.ai/code/session_01PiUjd47Da71WG2BFkDQz9q