refactor(miner): polish miner configuration #19480#2135
refactor(miner): polish miner configuration #19480#2135AnilChinchawale merged 1 commit 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 |
There was a problem hiding this comment.
Pull request overview
Refactors mining-related runtime configuration to be instance-scoped under ethconfig.Config.Miner, removing the global params.TargetGasLimit dependency and rewiring initialization/CLI/test paths accordingly.
Changes:
- Introduces
miner.Config(Etherbase/ExtraData/GasCeil/GasPrice) and threads it throughethservice creation and miner/worker construction. - Removes
params.TargetGasLimitand updates mining gas limit selection to use the miner config (GasCeil) while updating chain maker/bench code to useparams.XDCGenesisGasLimit. - Updates TOML marshaling/unmarshaling, CLI flag wiring, and tests to use the nested miner config.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| params/protocol_params.go | Removes global TargetGasLimit. |
| miner/worker.go | Splits miner runtime config from chain config; uses GasCeil from miner config for block gas limits. |
| miner/miner.go | Adds miner.Config/DefaultConfig and updates Miner constructor signature. |
| eth/ethconfig/config.go | Moves mining options into Config.Miner and updates defaults/gencodec directive. |
| eth/ethconfig/gen_config.go | Updates TOML encoding/decoding to include nested Miner config. |
| eth/backend.go | Validates/sanitizes config.Miner fields; wires miner + GPO to nested miner config. |
| core/chain_makers.go | Replaces removed target gas limit usage with params.XDCGenesisGasLimit. |
| core/bench_test.go | Updates benchmark gas limit target constant. |
| console/console_test.go | Updates test config initialization to set Config.Miner.Etherbase. |
| cmd/utils/flags.go | Rewires CLI flags into cfg.Miner and adds setMiner. |
| cmd/XDC/main.go | Removes mutation of the deleted global params.TargetGasLimit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type Config struct { | ||
| Genesis *core.Genesis `toml:",omitempty"` | ||
| NetworkId *uint64 | ||
| SyncMode *downloader.SyncMode | ||
| NoPruning *bool | ||
| Prefetch *bool | ||
| LightServ *int `toml:",omitempty"` | ||
| LightPeers *int `toml:",omitempty"` | ||
| SkipBcVersionCheck *bool `toml:"-"` | ||
| DeleteAllBadBlocks *bool `toml:"-"` | ||
| DatabaseHandles *int `toml:"-"` | ||
| DatabaseCache *int | ||
| TrieCleanCache *int | ||
| TrieDirtyCache *int | ||
| TrieTimeout *time.Duration | ||
| Preimages *bool | ||
| FilterLogCacheSize *int | ||
| Miner *miner.Config | ||
| LogQueryLimit *int | ||
| Etherbase *common.Address `toml:",omitempty"` | ||
| MinerThreads *int `toml:",omitempty"` | ||
| ExtraData *hexutil.Bytes `toml:",omitempty"` | ||
| GasPrice *big.Int | ||
| MinerThreads *int `toml:",omitempty"` | ||
| TxPool *legacypool.Config | ||
| GPO *gasprice.Config | ||
| EnablePreimageRecording *bool |
There was a problem hiding this comment.
This TOML unmarshal shape no longer recognizes the legacy top-level mining keys (etherbase, extraData, gasPrice) that previously existed on ethconfig.Config. Existing config files using the old keys will now silently ignore them and fall back to defaults, which is a backward-compatibility break. Consider supporting both the legacy keys and the new nested [Miner] table during a deprecation window (e.g., by mapping legacy fields into c.Miner when present).
fc8af14 to
5aa4c2c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4f8c1d5 to
093b171
Compare
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.
Proposed changes
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.
Ref: ethereum#19480
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