-
Notifications
You must be signed in to change notification settings - Fork 69
all: move main transaction pool into a subpool #27463 #1890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-upgrade
Are you sure you want to change the base?
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 You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements a major architectural refactoring to move the main transaction pool into a subpool system, supporting specialization and modularity in transaction handling.
Summary: The PR restructures the transaction pool to use a coordinator pattern where a main TxPool manages multiple specialized SubPool implementations. The legacy transaction pool logic is moved to core/txpool/legacypool/, and a new wrapper type txpool.Transaction is introduced for better encapsulation.
Key Changes:
- Introduced subpool architecture with
SubPoolinterface and mainTxPoolcoordinator - Moved legacy pool implementation to dedicated
legacypoolpackage - Added transaction wrapper type (
txpool.Transaction) for better type safety - Updated API signatures across the codebase (e.g.,
AddLocal→Add,Stop()→Close())
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/txpool/txpool.go | New main transaction pool coordinator implementing subpool aggregation |
| core/txpool/subpool.go | Defines SubPool interface for specialized transaction pools |
| core/txpool/legacypool/legacypool.go | Refactored legacy pool as a SubPool implementation |
| core/txpool/errors.go | Centralized error definitions for transaction pool |
| event/multisub.go | New utility for joining multiple event subscriptions |
| eth/backend.go | Updated to use new pool initialization pattern |
| miner/worker.go | Type signature updates for transaction maps |
| internal/ethapi/backend.go | Interface signature updates for consistency |
| eth/protocol.go | Updated txPool interface with new method signatures |
| Multiple test files | Updated tests to use new API (e.g., addLocal, addRemote, Close) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6c9a27a to
1098ef4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1098ef4 to
55e24a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
083953d to
1869827
Compare
…inFinOrg#1850 Previously, vote verification would log errors and fail when the referenced block header was not yet available locally, especially when nodes in the same round processed votes before receiving the block header. This commit changes the logic to defer verification and log at debug level if the header is missing, preventing unnecessary error logs and aligning with upstream geth behavior for out-of-order message arrival during consensus voting.
- Refactor NewTransactionsByPriceAndNonce to properly filter and separate special transactions (IsSpecialTransaction) - Remove account from txs map if it has no normal transactions left - Update comments for clarity
Previously, the code used binary.BigEndian.Uint64(data[8:40]), which incorrectly read the index and only extracted the highest 8 bytes of the 32-byte left-padded block number, resulting in wrong values (often zero). Now, the code uses new(big.Int).SetBytes(data[4:36]).Uint64() to correctly extract the block number from the proper 32-byte field. This change fixes both the incorrect index and the parsing logic, ensuring accurate block number extraction and correct validation for special transactions.
7c7e85e to
4553c0c
Compare
This commit updates the Txs field in NewTxsEvent to use []*types.Transaction instead of types.Transactions. This ensures type consistency for event broadcasting and handling, improving reliability of transaction propagation across the network. The change also removes unnecessary goroutine usage for txFeed.Send, making event delivery synchronous and safer for downstream consumers.
4553c0c to
66167c3
Compare
66167c3 to
614bab0
Compare
Proposed changes
Ref: ethereum#27463
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
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