fix(core/txpool/locals): untrack rejected and superseded local txs on resubmit - #2541
fix(core/txpool/locals): untrack rejected and superseded local txs on resubmit#2541gzliudan wants to merge 2 commits into
Conversation
… resubmit
The gas schedule fork sweep (56adc5a97) drops underpriced transactions
from the pool, but the local tracker keeps resubmitting them every
minute and discards the rejection, so a transaction priced out by the
fork turns into a permanent zombie: it stays in the tracked set and in
the journal, and is re-tracked unconditionally on restart.
Consume the resubmit errors the same way AddLocal consumes the initial
admission: a transaction the pool refuses for a non-temporary reason is
untracked, while accepted and temporarily rejected ones stay tracked.
Match the per-address map by hash, not by nonce, so a replacement at the
same nonce is not taken down with the transaction it superseded.
This commit addresses three issues found during review:
* Orphaned transactions: a same-nonce replacement supersedes the
previously tracked transaction, so TrackAll now drops the superseded
hash from the tracked set to keep it consistent with the per-address
map (the journal catches up on the next rotation).
* ErrAlreadyKnown is not a rejection: a resubmission that loses a race
to a concurrent Add returning txpool.ErrAlreadyKnown is already in the
desired state, so untrackRejected treats it like a success and keeps
tracking the transaction.
* Journal resurrection window: the journal rotation is moved out of
recheck and performed only after resubmission rejections are
processed, and fires immediately when a transaction was untracked, so
rejected transactions are removed from disk in the same cycle instead
of surviving until the next scheduled rotation.
|
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: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
There was a problem hiding this comment.
Pull request overview
Fixes local transaction tracking so permanently rejected or superseded transactions are removed from memory and disk.
Changes:
- Classifies resubmission errors and untracks permanent rejections.
- Removes superseded same-nonce transactions.
- Rotates journals after removals and adds regression tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
core/txpool/locals/tx_tracker.go |
Updates resubmission, untracking, and journal rotation logic. |
core/txpool/locals/tx_tracker_test.go |
Tests rejection, replacement, and journal persistence behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
untrackRejected previously called SortedMap.Filter once per rejected transaction. Filter rescans the whole per-address map and rebuilds its heap, so a fork sweep that permanently rejects k tracked transactions from one sender turned into O(k²) work while tracker.mu was held, blocking new local tracking and journal writes. Group permanently rejected hashes by sender and filter each per-address map exactly once, using a hash set for O(1) membership checks. Add a test covering the fork-sweep shape.
d34ee67 to
69d8ff1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
core/txpool/locals/tx_tracker_test.go:165
- Handle the key-generation error instead of passing a potentially nil key to
SignTx; the repository's Go error-handling convention does not allow ignored errors.
otherKey, _ := crypto.GenerateKey()
Proposed changes
Summary
The
core/txpool/localstracker resubmits locally-tracked transactions every recheck interval but discards the errors returned bypool.Add. After the gas schedule fork sweep drops underpriced transactions from the pool, a local transaction priced out by the fork turns into a permanent zombie: it is never untracked, survives in the on-disk journal, and is re-tracked unconditionally on restart. This PR fixes how the tracker classifies resubmit outcomes so the tracked set and the journal stay consistent with the pool's actual state.Root cause
TxTracker.loopcalledpool.Add(resubmits, false)and ignored the returned errors, and there was no path to remove a transaction from the tracked set or journal once the pool permanently rejected it.Changes
Reuse the same error classification that
AddLocalapplies to the initial admission, now on resubmission: untrack permanently rejected txs (removed fromtracker.allandtracker.byAddr, matched by hash not nonce so a same-nonce replacement is not taken down with the tx it superseded); treatErrAlreadyKnownas success (a resubmission losing a race to a concurrentAddreturningtxpool.ErrAlreadyKnownis already in the desired state and stays tracked); drop superseded same-nonce replacements inTrackAll(remove the previously tracked hash at a nonce when a replacement is tracked, keeping the tracked set consistent with the per-address map); and move journal rotation out ofrecheckso it runs only after resubmission rejections are processed and fires immediately when a transaction was untracked, wiping rejected transactions from disk in the same cycle.Risk / notes
Hot path:
untrackRejectedruns only when there are resubmits and acquirestracker.muonce for the whole batch, with no per-transaction pool calls added. The change is confined to the local tracker; no txpool public API changed.Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that