fix(anvil): remove replaced tx on queued-pool replacement - #16638
Open
gomesalexandre wants to merge 1 commit into
Open
fix(anvil): remove replaced tx on queued-pool replacement#16638gomesalexandre wants to merge 1 commit into
gomesalexandre wants to merge 1 commit into
Conversation
…d tx on anvil_dropTransaction PendingTransactions::add_transaction (the queued/future-nonce path) only checked for an underpriced replacement and returned an error - on the accept path it never removed the tx it was replacing, unlike ReadyTransactions::add_transaction which correctly does. Replacing a queued transaction with a higher-priced one at the same (sender, nonce) therefore stacked transactions indefinitely instead of releasing the old one, only one of which could ever be mined. The fix captures the replaced tx's hash before inserting the new tx's own markers, then removes the old entry via the existing remove() path. Order matters: removing after insertion would delete the new tx's own waiting_markers entry instead (both share the same `provides` key for the same nonce slot), silently disabling the underpriced check for the next replacement at that slot. anvil_dropTransaction had the same asymmetry: it only touched ready_transactions, silently no-op'ing on a queued transaction, unlike its siblings remove_invalid/remove_transactions_by_address which already touch both pools. Live-verified against a running anvil node (txpool_status/ eth_getTransactionByHash before and after), plus two new integration tests covering repeated replacement, the underpriced-rejection-after- replacement trap, and mark_and_unlock's required_markers bookkeeping after a nonce-gap fill. Co-Authored-By: Claude <noreply@anthropic.com>
gomesalexandre
marked this pull request as ready for review
September 4, 2026 18:56
gomesalexandre
requested review from
0xrusowsky,
DaniPopes,
figtracer,
grandizzy,
mablr,
mattsse and
stevencartavia
as code owners
September 4, 2026 18:56
Contributor
✅ Changelog foundThe deterministic check will validate the changed entry. |
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
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.
PendingTransactions::add_transaction(the QUEUED/future-nonce tx path) only checked for an underpriced replacement and returned an error - on the accept path it never removed the tx it was replacing. Compare toReadyTransactions::add_transaction, which correctly callsreplaced_transactions(...)to remove the old tx.Consequence: submitting three transactions at the same
(sender, nonce)with escalating gas price while the sender has a pending earlier nonce (so they land in the queued pool) stacks all three instead of releasing the earlier ones.txpool_statusreportsqueued: 3where geth would report1, and two of the three can never be mined - unbounded pool growth for any gas-bumping/fee-escalation workflow.Companion bug:
anvil_dropTransactiononly touchedready_transactions, silently no-op'ing ({"result":null}, read as "no such tx") on a queued transaction - unlike its siblingsremove_invalid/remove_transactions_by_address, which already touch both pools.The fix
Capture the replaced tx's hash from the immutable-borrow scope, then call
self.remove(vec![replaced_hash])after that borrow ends but before inserting the new tx's ownwaiting_markers/waiting_queueentries.Order matters here: removing after insertion would delete the new tx's own
waiting_markersentry instead of the old one, since both share the sameprovideskey for the same nonce slot (remove()'swaiting_markers.remove(&tx.provides)is a plain key removal, not hash-checked) - silently disabling the underpriced-replacement check for the next replacement at that slot. Caught this via a synchronous Codex adversarial review before shipping.anvil_dropTransactionnow mirrorsremove_invalid's existing both-pools removal pattern.Testing
crates/anvil/tests/it/txpool.rs:queued_tx_replacement_removes_old_tx- replaces a queued tx twice in a row, confirms the pool never stacks, confirms an underpriced replacement attempt is still correctly rejected after two prior replacements (the specific marker-corruption trap above), and fills the preceding nonce gap + mines to confirmmark_and_unlock'srequired_markersbookkeeping wasn't left in a bad state by the removal.anvil_drop_transaction_removes_queued_tx- confirms dropping a queued tx actually removes it.crates/anvil/tests/it/txpool.rssuite (8 tests) passing, plus the existinganvil_api::instant_mining_reselects_from_live_pool_after_failure(which already exercisesanvil_dropTransactionon a ready tx) unaffected.cargo clippy -p anvil --all-targets -- -D warningsclean,cargo fmt --check -p anvilclean (stable only - nightly, used by this repo's CI, wasn't available in this environment).add_transaction), verifiedremove()'s marker cleanup is complete under normal invariants, and flagged therequired_markerscoverage gap addressed above.receipts
Live-verified against a real running anvil node (not just the test suite), reproducing the exact scenario: three transactions submitted at nonce 5 with escalating gas price via raw JSON-RPC.
Before fix would show
queued: 3. After fix: