perf(core/txpool,miner): speed up pending transaction ordering with uint256 #29008#2159
perf(core/txpool,miner): speed up pending transaction ordering with uint256 #29008#2159gzliudan wants to merge 4 commits intoXinFinOrg:dev-upgradefrom
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 |
5b668fb to
8e70fad
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to speed up “pending” transaction handling (miner selection and txpool retrieval) by pre-filtering pending txs by fee/tip constraints and by switching fee comparisons to uint256 to reduce allocations/overhead.
Changes:
- Update the txpool
PendingAPI to accept(minTip, baseFee *uint256.Int)and propagate the new signature through ETH protocol syncing, RPC backends, miner, and tests. - Convert
txpool.LazyTransactionfee fields and miner ordering logic to*uint256.Intand add tests around min-tip/base-fee filtering behavior. - Refactor mining configuration into
ethconfig.Config.Miner(typeminer.Config), add miner-side gas tip setting/validation, and adjust gas-limit defaults.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| params/protocol_params.go | Adjusts XDC gas-limit constant and removes TargetGasLimit global. |
| miner/worker_test.go | Adds unit tests for worker gas tip validation and copy semantics. |
| miner/worker.go | Wires miner config vs chain config, adds setGasTip, uses uint256 fees in tx ordering inputs, and uses new txpool Pending filter. |
| miner/ordering_test.go | Updates ordering tests to populate LazyTransaction fee fields with uint256. |
| miner/ordering.go | Switches miner fee/tip computations and comparisons to uint256 and pre-converts TRC21 gas price. |
| miner/miner.go | Introduces miner.Config + defaults, updates miner constructor signature, and adds SetGasTip. |
| eth/sync.go | Updates peer tx sync path to use new Pending(nil, nil) signature. |
| eth/protocol.go | Updates txPool interface Pending signature to uint256 params. |
| eth/helper_test.go | Updates test txpool mock to satisfy new Pending signature and LazyTransaction fee types. |
| eth/ethconfig/gen_config.go | Moves miner-related TOML marshalling to Miner miner.Config field. |
| eth/ethconfig/config.go | Moves mining options under Config.Miner and updates defaults / gencodec generation settings. |
| eth/backend.go | Sanitizes miner config, wires miner creation with config.Miner, and updates GPO initialization. |
| eth/api_miner.go | Updates miner_setGasPrice flow to set txpool + miner gas tip and copy gas price internally. |
| eth/api_backend.go | Updates pending tx retrieval to new Pending(nil, nil) signature. |
| core/txpool/txpool.go | Updates TxPool.Pending signature and forwards filter params to subpools. |
| core/txpool/subpool.go | Updates LazyTransaction fee fields to uint256 and updates SubPool.Pending signature. |
| core/txpool/legacypool/legacypool_test.go | Adds tests for min-tip/base-fee pending filtering behavior. |
| core/txpool/legacypool/legacypool.go | Implements new Pending(minTip, baseFee) filtering and converts tx fee fields to uint256. |
| core/chain_makers.go | Replaces removed target gas-limit usage with XDCGenesisGasLimit. |
| core/bench_test.go | Replaces removed target gas-limit usage with XDCGenesisGasLimit. |
| console/console_test.go | Updates test config wiring to new ethconfig.Config.Miner struct. |
| cmd/utils/flags.go | Routes miner flags into cfg.Miner (and adds setMiner). |
| cmd/XDC/main.go | Removes runtime mutation of removed params.TargetGasLimit. |
| accounts/abi/bind/v2/util_test.go | Adjusts gas price in deployment test to be safely above base fee. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| eth.miner = miner.New(eth, &config.Miner, eth.blockchain.Config(), eth.EventMux(), eth.engine, stack.Config().AnnounceTxs) | ||
| eth.miner.SetExtra(makeExtraData(config.Miner.ExtraData)) |
There was a problem hiding this comment.
config.Miner.ExtraData is hexutil.Bytes, but makeExtraData expects []byte. This call won’t compile without converting hexutil.Bytes to []byte (and ideally copying to avoid aliasing if the underlying slice might be reused).
| if ctx.IsSet(MinerExtraDataFlag.Name) { | ||
| cfg.ExtraData = []byte(ctx.String(MinerExtraDataFlag.Name)) | ||
| } |
There was a problem hiding this comment.
miner.Config.ExtraData is hexutil.Bytes, so assigning a []byte directly won’t compile. Convert the CLI string to hexutil.Bytes (or change the config field type back to []byte) so the miner extra-data flag wiring builds correctly.
| GenesisGasLimit uint64 = 4712388 // Gas limit of the Genesis block. | ||
| XDCGenesisGasLimit uint64 = 84000000 | ||
| XDCGenesisGasLimit uint64 = 42000000 | ||
|
|
There was a problem hiding this comment.
This PR changes params.XDCGenesisGasLimit from 84,000,000 to 42,000,000, which feeds into miner defaults (miner.DefaultConfig.GasCeil) and other helpers (e.g., core.CalcGasLimit targets). That’s a significant behavioral/default change but it’s not described in the PR metadata/title; please confirm it’s intentional and add rationale (or revert if unrelated to the blobpool pending speedup work).
Miner configuration is unified under [Eth.Miner] (GasCeil/GasPrice/Etherbase/ExtraData), replacing legacy top-level [Eth] miner keys. Operational impact: existing config files using [Eth].GasPrice/[Eth].Etherbase/[Eth].ExtraData must be migrated before upgrade. Behavior update: gasprice=0 remains valid; only negative gas prices are sanitized at startup. Default change: XDCGenesisGasLimit is reduced to 42,000,000 and now feeds miner default GasCeil (including default --miner-gaslimit), so nodes relying on defaults should review capacity expectations.
Synchronize miner gas tip updates across subsystems when RPC updates gas price. - update eth miner API to apply gas tip changes to both txpool and miner - add miner/worker SetGasTip path and initialize worker tip from config - adjust bind util test to use params.GWei over base fee Ref: ethereum#28933
…ethereum#29005 Introduce dynamic-fee pre-filtering in txpool pending retrieval to reduce allocations and downstream processing work during mining and tx propagation. What changed: - Change the `Pending` API from `Pending(enforceTips bool)` to `Pending(minTip *uint256.Int, baseFee *uint256.Int)` in txpool interfaces. - Implement pre-filtering in `legacypool.Pending` by comparing effective tip against the provided `minTip/baseFee` for non-local, non-special txs. - Update call sites to the new API: - miner work assembly (`miner/worker.go`) - tx sync (`eth/sync.go`) - pool transaction retrieval (`eth/api_backend.go`) - protocol/test interfaces (`eth/protocol.go`, `eth/helper_test.go`). Tests: - Add targeted coverage for pending filtering semantics in `core/txpool/legacypool/legacypool_test.go`, including: - minTip threshold boundary behavior - baseFee-aware effective tip filtering - local/special transaction exemption behavior - dynamic-fee boundary behavior when baseFee is nil. Impact: - Preserves existing behavior while making pending selection cheaper for downstream consumers. - Improves confidence in edge-case behavior through dedicated tests.
8e70fad to
b526996
Compare
…int256 ethereum#29008 Switch LazyTransaction gas caps from *big.Int to *uint256.Int and convert once at pending retrieval time. In miner ordering, keep fee comparisons on uint256 and precompute TRC21 gas price in uint256 form to avoid repeated big-int conversions in heap comparisons. Also update affected tests and call sites in legacypool/worker/helper paths. Compatibility: LazyTransaction exported field types changed (GasFeeCap/GasTipCap).
b526996 to
6d33068
Compare
|
@gzliudan conflict |
Proposed changes
Switch LazyTransaction gas caps from *big.Int to *uint256.Int and convert once at pending retrieval time.
In miner ordering, keep fee comparisons on uint256 and precompute TRC21 gas price in uint256 form to avoid repeated big-int conversions in heap comparisons.
Also update affected tests and call sites in legacypool/worker/helper paths.
Compatibility: LazyTransaction exported field types changed (GasFeeCap/GasTipCap).
Ref: ethereum#29008
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