fix(core/txpool/legacypool): fix data race of txlookup access #31641#2194
Conversation
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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
This PR refactors local transaction handling in the txpool by removing the local flag from the Add APIs and introducing a dedicated local transaction tracker for journaling/resubmission, alongside a new TxPool.Sync() helper for deterministic simulator/test behavior.
Changes:
- Remove
localparameter fromtxpool.TxPool.Addandtxpool.SubPool.Add, updating eth protocol plumbing and tests accordingly. - Introduce
core/txpool/locals.TxTrackerand wire it intoeth.Newfor local tx tracking/journaling/resubmission. - Add
TxPool.AddLocal(s)helpers and a newTxPool.Sync()method to support deterministic resets in tests/simulations.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| eth/protocol.go | Updates txPool interface to match new Add(txs, sync) signature. |
| eth/protocol_test.go | Adjusts protocol tests for updated txpool Add signature. |
| eth/helper_test.go | Updates test txpool stub to match new Add signature. |
| eth/handler.go | Updates p2p tx import path to call Add(txs, sync) without local. |
| eth/backend.go | Wires up locals.TxTracker lifecycle and journal interval sanitization. |
| eth/api_backend.go | Updates RPC tx submission path for new local-tracking flow. |
| core/txpool/txpool.go | Adds local tracking hooks, AddLocal(s), and deterministic Sync() + reset-loop improvements. |
| core/txpool/txpool_local_test.go | Adds unit tests asserting local tracking call order vs add. |
| core/txpool/subpool.go | Updates SubPool interface: removes local parameter and locals API. |
| core/txpool/locals/tx_tracker.go | New local transaction tracker (recheck/resubmit + journaling). |
| core/txpool/locals/journal.go | Fixes package name and tweaks logging verbosity on empty rotation. |
| core/txpool/legacypool/list.go | Exposes SortedMap, removes local-aware pricing behaviors, updates eviction/pricing plumbing. |
| core/txpool/legacypool/legacypool.go | Removes local exemptions/journaling from LegacyPool; adapts to new shared txpool API. |
| core/txpool/legacypool/legacypool_test.go | Updates tests/benchmarks to reflect removal of local exemptions and new semantics. |
| core/txpool/legacypool/journal_shared.go | Adds shared journal helpers for legacypool journals. |
| contracts/utils.go | Switches local tx insertion to TxPool.AddLocal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resubmits, rejournal := tracker.recheck(checkJournal) | ||
| if len(resubmits) > 0 { | ||
| tracker.pool.Add(resubmits, false) | ||
| } | ||
| if checkJournal { | ||
| // Lock to prevent journal.rotate <-> journal.insert (via TrackAll) conflicts | ||
| tracker.mu.Lock() | ||
| lastJournal = time.Now() | ||
| if err := tracker.journal.rotate(rejournal); err != nil { | ||
| log.Warn("Transaction journal rotation failed", "err", err) | ||
| } | ||
| tracker.mu.Unlock() |
d9959e5 to
8937f39
Compare
2349a83 to
95546f5
Compare
95546f5 to
c9ef0aa
Compare
Proposed changes
Ref: ethereum#31641
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