refactor(txpool,eth,miner): use pending filter struct #29026#2160
refactor(txpool,eth,miner): use pending filter struct #29026#2160gzliudan wants to merge 5 commits intoXinFinOrg:dev-upgradefrom
Conversation
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.
…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).
Introduce txpool.PendingFilter and migrate Pending(...) signatures from positional params to a typed struct. - Add core/txpool.PendingFilter with MinTip/BaseFee fields for cheap call-site filtering - Update txpool subpool interface and all call sites in eth/miner to pass the filter struct - Keep behavior unchanged for empty filter (equivalent to previous nil,nil usage) - Refresh legacypool tests to use the new API shape This is a refactor-only change intended to improve API clarity and future extensibility without changing consensus or RPC semantics.
|
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 |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
Refactors transaction selection plumbing by introducing a typed txpool.PendingFilter (instead of positional args) and propagating it through txpool → eth → miner, while also migrating fee-cap fields on txpool.LazyTransaction to uint256.Int for cheaper comparisons.
Changes:
- Add
txpool.PendingFilterand updateTxPool.Pending/SubPool.Pendingsignatures and call sites to accept the struct. - Update miner transaction ordering/selection to use
uint256.Intfee fields and pass aPendingFilter(MinTip/BaseFee) when collecting pending txs for block production. - Move mining runtime options under
ethconfig.Config.Miner(miner.Config) and update CLI/config marshalling + service initialization accordingly.
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 constants and removes TargetGasLimit global. |
| core/txpool/subpool.go | Introduces PendingFilter; changes LazyTransaction fee fields to *uint256.Int. |
| core/txpool/txpool.go | Updates TxPool.Pending to accept PendingFilter and forwards it to subpools. |
| core/txpool/legacypool/legacypool.go | Implements Pending(filter) by translating filter to legacy big.Int comparisons. |
| core/txpool/legacypool/legacypool_test.go | Adds/updates tests for pending filtering (MinTip/BaseFee) behavior. |
| eth/protocol.go | Updates txPool interface to Pending(filter) shape. |
| eth/sync.go | Updates tx sync path to call Pending(txpool.PendingFilter{}). |
| eth/api_backend.go | Updates pool-tx retrieval to call Pending(txpool.PendingFilter{}). |
| eth/helper_test.go | Updates test txpool implementation to the new Pending(filter) signature. |
| miner/worker.go | Adds miner gas tip state + filter-based pending retrieval; switches LazyTx fee fields to uint256. |
| miner/worker_test.go | Updates worker setup and adds setGasTip validation/copying tests. |
| miner/ordering.go | Migrates ordering logic to uint256-based fee comparisons and baseFee conversion. |
| miner/ordering_test.go | Updates tests to populate LazyTx fee fields as uint256. |
| miner/miner.go | Introduces miner.Config + defaults and adds Miner.SetGasTip. |
| eth/backend.go | Sanitizes config.Miner values and wires miner construction to config.Miner. |
| eth/ethconfig/config.go | Moves mining options under Config.Miner and updates defaults/go:generate. |
| eth/ethconfig/gen_config.go | Regenerates TOML marshal/unmarshal using nested Miner config. |
| cmd/utils/flags.go | Routes CLI miner flags into cfg.Miner via new setMiner. |
| cmd/XDC/main.go | Removes runtime mutation of params.TargetGasLimit (now config-driven). |
| console/console_test.go | Updates console test config to set Miner.Etherbase. |
| core/chain_makers.go | Updates header gas-limit targeting to use XDCGenesisGasLimit. |
| core/bench_test.go | Updates benchmark gas-limit targeting to use XDCGenesisGasLimit. |
| accounts/abi/bind/v2/util_test.go | Adjusts deployment gasPrice calculation to be baseFee + 1 gwei. |
| eth/api_miner.go | Updates SetGasPrice to update both txpool and miner gas-tip threshold. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| MinGasLimit uint64 = 5000 // Minimum the gas limit may ever be. | ||
| MaxGasLimit uint64 = 0x7fffffffffffffff // Maximum the gas limit (2^63-1). | ||
| GenesisGasLimit uint64 = 4712388 // Gas limit of the Genesis block. | ||
| XDCGenesisGasLimit uint64 = 84000000 | ||
| XDCGenesisGasLimit uint64 = 42000000 | ||
|
|
There was a problem hiding this comment.
This PR is described as a txpool Pending(...) API refactor with unchanged semantics, but this hunk changes XDCGenesisGasLimit from 84,000,000 to 42,000,000 and removes the global TargetGasLimit. That alters default gas limit behavior (e.g., default mining/config/bench helpers) and should be explicitly justified or split into a separate PR to avoid accidentally changing network defaults.
| // DefaultConfig contains default settings for miner. | ||
| var DefaultConfig = Config{ | ||
| GasCeil: params.XDCGenesisGasLimit, | ||
| GasPrice: big.NewInt(1), | ||
| } |
There was a problem hiding this comment.
miner.DefaultConfig sets GasPrice to 1 wei, which is far below the chain’s configured minimum gas price (common.MinGasPrice defaults to 250,000,000). Since this value feeds both txpool tip enforcement and the miner’s pending-tx filter, it changes which transactions are mined by default. Consider preserving the previous default (or setting it to common.MinGasPrice) to keep default mining/txpool behavior consistent.
| type PendingFilter struct { | ||
| MinTip *uint256.Int // Minimum miner tip required to include a transaction | ||
| BaseFee *uint256.Int // Minimum 1559 basefee needed to include a transaction | ||
| } |
There was a problem hiding this comment.
The PendingFilter.BaseFee field comment says "Minimum 1559 basefee needed", but call sites (e.g. mining) pass the current execution base fee to compute effective tips. Consider rewording this to avoid misuse (e.g., treat it as the baseFee context for effectiveTip = min(tipCap, feeCap-baseFee)).
| // This is the number of blocks for which logs will be cached in the filter system. | ||
| FilterLogCacheSize int | ||
|
|
||
| // Mining options | ||
| Miner miner.Config | ||
|
|
||
| // This is the maximum number of addresses or topics allowed in filter criteria | ||
| // for eth_getLogs. | ||
| LogQueryLimit int | ||
|
|
||
| // Mining-related options | ||
| Etherbase common.Address `toml:",omitempty"` | ||
| MinerThreads int `toml:",omitempty"` | ||
| ExtraData []byte `toml:",omitempty"` | ||
| GasPrice *big.Int | ||
|
|
||
| // Transaction pool options | ||
| TxPool legacypool.Config |
There was a problem hiding this comment.
Moving Etherbase/ExtraData/GasPrice into the nested Miner struct changes the TOML schema and will break existing config files that still use the old top-level keys. If backward compatibility is required, consider supporting both shapes during TOML unmarshalling (e.g., accept the legacy fields and map them into Config.Miner when Miner is unset).
|
@gzliudan conflict |
Proposed changes
Introduce txpool.PendingFilter and migrate Pending(...) signatures from positional params to a typed struct.
This is a refactor-only change intended to improve API clarity and future extensibility without changing consensus or RPC semantics.
Ref: ethereum#29026
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