fix(core/txpool/legacypool): fix data race of pricedList access #31758#2203
Merged
AnilChinchawale merged 1 commit intoXinFinOrg:dev-upgradefrom Mar 24, 2026
Merged
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 transaction-pool “local transaction” handling by removing the local flag from the txpool/subpool APIs and introducing a dedicated locals.TxTracker for journaling/resubmission, while also adding a deterministic TxPool.Sync() helper for simulator/tests.
Changes:
- Simplify txpool/subpool
AddAPIs by removing thelocalparameter; addTxPool.AddLocal(s)wrappers that feed a pluggable local tracker. - Introduce
core/txpool/locals.TxTrackerwith journaling + periodic recheck/resubmit, wired frometh.New()when locals are enabled. - Update legacy pool internals/tests to drop local/remote distinctions (pricing/eviction/journaling) and adjust eth protocol/tests call sites.
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 protocol manager’s txpool interface to the new Add(txs, sync) signature. |
| eth/protocol_test.go | Updates protocol tests to use the new txpool Add signature. |
| eth/helper_test.go | Updates the test txpool mock to match the new Add signature. |
| eth/handler.go | Updates p2p tx ingestion to call Add(txs, sync) without a local flag. |
| eth/backend.go | Wires locals.TxTracker into TxPool and node lifecycle; sanitizes Rejournal. |
| eth/api_backend.go | Adjusts RPC tx submission path to track locals + submit into txpool under new API. |
| core/txpool/txpool.go | Adds LocalTracker, SetLocalTracker, AddLocal(s), termination marker, and Sync() for deterministic resets; updates Add/loop logic. |
| core/txpool/txpool_local_test.go | Adds tests ensuring local tracking happens before pool insertion. |
| core/txpool/subpool.go | Updates SubPool interface by removing the local parameter and removing Locals(). |
| core/txpool/locals/tx_tracker.go | Adds local tx tracker with periodic recheck/resubmit and optional journaling. |
| core/txpool/locals/journal.go | Fixes package name and adjusts logging verbosity when journal is empty. |
| core/txpool/legacypool/list.go | Renames sortedMap to SortedMap and removes local-aware pricing/eviction code paths. |
| core/txpool/legacypool/legacypool.go | Removes legacy local-account semantics/journaling; collapses local/remote lookup into a single map; updates add/evict/clear flows accordingly. |
| core/txpool/legacypool/legacypool_test.go | Updates tests/benchmarks to match the new semantics and APIs (no local exemptions). |
| core/txpool/legacypool/journal_shared.go | Adds shared journal helpers for legacypool journals. |
| contracts/utils.go | Switches contract helper submissions to TxPool.AddLocal for local tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eth/api_backend.go
Outdated
Comment on lines
+304
to
+307
| if locals := b.eth.localTxTracker; locals != nil { | ||
| locals.Track(signedTx) | ||
| } | ||
| return b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0] |
| } | ||
| } | ||
| return migrated | ||
| t.slots = 0 |
Comment on lines
619
to
623
| // Discard finds a number of most underpriced transactions, removes them from the | ||
| // priced list and returns them for further removal from the entire pool. | ||
| // If noPending is set to true, we will only consider the floating list | ||
| // | ||
| // Note local transaction won't be considered for eviction. | ||
| func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { | ||
| func (l *pricedList) Discard(slots int) (types.Transactions, bool) { | ||
| drop := make(types.Transactions, 0, slots) // Remote underpriced transactions to drop |
Comment on lines
+1934
to
1938
| // TxsBelowTip finds all remote transactions below the given tip threshold. | ||
| func (t *lookup) TxsBelowTip(threshold *big.Int) types.Transactions { | ||
| found := make(types.Transactions, 0, 128) | ||
| t.Range(func(hash common.Hash, tx *types.Transaction, local bool) bool { | ||
| t.Range(func(hash common.Hash, tx *types.Transaction) bool { | ||
| if tx.GasTipCapIntCmp(threshold) < 0 { |
gzliudan
force-pushed
the
fix-data-race-pricedList-access
branch
2 times, most recently
from
7128100 to
17d0484
Compare
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
anunay-xin
approved these changes
Mar 18, 2026
wanwiset25
approved these changes
Mar 24, 2026
benjamin202410
approved these changes
Mar 24, 2026
gzliudan
force-pushed
the
fix-data-race-pricedList-access
branch
from
17d0484 to
13d5504
Compare
gzliudan
force-pushed
the
fix-data-race-pricedList-access
branch
from
13d5504 to
1bc881d
Compare
AnilChinchawale
approved these changes
Mar 24, 2026
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.
Proposed changes
Ref: ethereum#31758
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