feat(core/txpool): reject stale transaction for local tracking #31473#2182
Open
gzliudan wants to merge 6 commits intoXinFinOrg:dev-upgradefrom
Open
feat(core/txpool): reject stale transaction for local tracking #31473#2182gzliudan wants to merge 6 commits intoXinFinOrg:dev-upgradefrom
gzliudan wants to merge 6 commits intoXinFinOrg:dev-upgradefrom
Conversation
…thereum#28837 Improve txpool loop synchronization around background resets. This change: - adds an explicit termination channel to signal pool shutdown - tracks forced-reset intent and a waiter channel inside the reset loop - ensures reset waiters are notified on completion or on pool termination - allows an explicit sync request path to trigger an additional reset round when needed Scope is limited to internal txpool concurrency control in core/txpool/txpool.go, with no protocol or RPC behavior change.
…ereum#29083 * core/txpool: no need to run rotate if no local txs Signed-off-by: jsvisa <delweng@gmail.com> * Revert "core/txpool: no need to run rotate if no local txs" This reverts commit 17fab17. Signed-off-by: jsvisa <delweng@gmail.com> * use Debug if todo is empty Signed-off-by: jsvisa <delweng@gmail.com> --------- Signed-off-by: jsvisa <delweng@gmail.com>
|
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)
📝 Coding Plan
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 |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
This PR refactors “local transaction” handling across the txpool stack by removing the local flag from txpool/subpool APIs and introducing a dedicated locals.TxTracker for tracking/journaling/resubmitting local submissions. It wires the tracker into the Ethereum service startup and updates callers/tests accordingly.
Changes:
- Remove
local boolfromTxPool.Add/SubPool.Add(and related interfaces/call sites), keeping only thesyncflag. - Introduce
core/txpool/localstracker + journaling and wire it ineth.NewwhenTxPool.NoLocalsis false. - Simplify
legacypoolby removing local-account exemptions/journaling paths and update tests/benchmarks for the new semantics.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eth/protocol.go | Updates txpool interface to new Add(txs, sync) signature. |
| eth/protocol_test.go | Updates protocol tests for new txpool Add signature. |
| eth/helper_test.go | Updates test txpool implementation signature. |
| eth/handler.go | Updates inbound-tx handling to call Add(txs, sync) without local flag. |
| eth/backend.go | Wires locals.TxTracker into node lifecycle and txpool when locals enabled. |
| eth/api_backend.go | Updates RPC submission path to track locals and call txpool Add. |
| core/txpool/txpool.go | Adds local-tracker plumbing (AddLocal(s)), pool termination/sync helper, and internal state tracking. |
| core/txpool/txpool_local_test.go | Adds tests asserting local tracking happens before add and propagates sync. |
| core/txpool/subpool.go | Updates subpool interface to new Add signature; removes Locals() API. |
| core/txpool/locals/tx_tracker.go | New local transaction tracker (track/recheck/resubmit + journaling). |
| core/txpool/locals/tx_tracker_test.go | Adds a test harness for locals tracker integration tests. |
| core/txpool/locals/journal.go | Fixes package name and adjusts regen logging verbosity when empty. |
| core/txpool/legacypool/list.go | Exports SortedMap/constructor; removes local-aware pricing/eviction hooks. |
| core/txpool/legacypool/legacypool.go | Removes legacy local-account exemptions/journaling and unifies tx lookup handling. |
| core/txpool/legacypool/legacypool_test.go | Updates tests/benchmarks for unified (non-local-exempt) semantics. |
| core/txpool/legacypool/journal_shared.go | Adds shared journal helpers for legacypool journals. |
| contracts/utils.go | Switches contract helpers to TxPool.AddLocal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+304
to
309
| if locals := b.eth.localTxTracker; locals != nil { | ||
| locals.Track(signedTx) | ||
| } | ||
| return b.eth.txPool.Add([]*types.Transaction{signedTx}, false)[0] | ||
| } | ||
|
|
|
|
||
| // RemotesBelowTip finds all remote transactions below the given tip threshold. | ||
| func (t *lookup) RemotesBelowTip(threshold *big.Int) types.Transactions { | ||
| // TxsBelowTip finds all remote transactions below the given tip threshold. |
Comment on lines
+500
to
+503
| sync := make(chan error) | ||
| select { | ||
| case p.sync <- sync: | ||
| return <-sync |
| // Sync is a helper method for unit tests or simulator runs where the chain events | ||
| // are arriving in quick succession, without any time in between them to run the | ||
| // internal background reset operations. This method will run an explicit reset | ||
| // operation to ensure the pool stabilises, thus avoiding flakey behavior. |
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 |
benjamin202410
approved these changes
Mar 14, 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#31473
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