feat: centralize chain config and harden genesis startup - #2329
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 |
c0ca61f to
b87a72c
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors fork activation to be fully params.ChainConfig-driven (removing reliance on common fallback globals), normalizes XDC network configs to explicit per-network fork block values (using nil for unscheduled forks), and updates multiple call sites/tests to operate without global fallback behavior.
Changes:
- Move Berlin/London/Merge/Shanghai/EIP-1559/Cancun/Prague/Osaka/DynamicGasLimit activation to
params.ChainConfigand removecommonfallback usage. - Normalize XDC chain configs (mainnet/testnet/devnet/localnet) with explicit fork block declarations and update signer/genesis/state logic to rely on
ChainConfigonly. - Update and extend tests to assert explicit fork declarations and avoid relying on implicit global fallback activation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| params/config.go | Adds explicit fork block fields to XDC configs; removes fallback logic in Description() and fork checks; introduces Localnet config and Localnet V2 config map. |
| params/config_test.go | Adds tests ensuring XDC configs explicitly declare fork blocks and that activation ignores removed common fallbacks. |
| eth/filters/filter_test.go | Adjusts test chain config to avoid Cancun/Prague/Osaka activation via config defaults. |
| eth/downloader/testchain_test.go | Introduces a local test chain config with Cancun/Prague/Osaka disabled; uses it in genesis/signer creation. |
| eth/downloader/downloader_test.go | Uses the shared testChainConfig for downloader tests. |
| eth/backend.go | Switches common.CopyConstants call to use chainConfig.ChainID directly. |
| core/vm/gas_table_test.go | Forces legacy fork behavior by nil-ing post-London fork blocks in test config copy. |
| core/types/transaction_signing.go | Makes LatestSigner selection depend solely on ChainConfig fork pointers (no global fallback). |
| core/state_processor.go | Removes fallback to common.PragueBlock when checking Prague activation. |
| core/genesis.go | Canonicalizes localnet config by chain ID and expands logging around resolved configs; removes fallback-related comment text. |
| core/genesis_test.go | Adds test asserting localnet config normalization and persistence. |
| contracts/randomize/randomize_test.go | Moves to NewXDCSimulatedBackend with a legacy config to preserve legacy-tx signing path. |
| common/constants.all.go | Removes fork block fields/globals (Berlin/London/Merge/Shanghai/EIP-1559/Cancun/Prague/Osaka/DynamicGasLimit) from constant set/copy paths. |
| common/constants.mainnet.go | Removes fork block constants from mainnet constant set. |
| common/constants.testnet.go | Removes fork block constants from testnet constant set. |
| common/constants.devnet.go | Removes fork block constants from devnet constant set. |
| common/constants.local.go | Removes fork block constants from local constant set. |
| cmd/XDC/chaincmd.go | Adds nil-check for genesis.Config; canonicalizes localnet config when chain ID matches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
298f45c to
a732dbb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0efd748 to
32c289c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f6109e8 to
15b851a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 94 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 94 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
core/token_validator.go:112
- CallContractWithState now relies on StateDB.GetTRC21FeeCapacityFromState(), which in turn uses StateDB.TRC21IssuerSMC() (derived from StateDB.chainConfig). CallContractWithState never sets statedb's chain config, so if the caller passes a StateDB without SetChainConfig having been invoked, feeCapacity will be read from the zero address and token-fee behavior becomes incorrect. Consider setting statedb.SetChainConfig(chain.Config()) at the start of CallContractWithState (or otherwise ensuring the chain config is always attached before accessing TRC21 fee state).
eth/tracers/api.go:285 - TransactionToMessage can now return an error (e.g., missing token-fee fork fields in chainConfig, signer/sender issues). In traceChain the error is ignored (msg, _ := ...), but msg is then passed to traceTx without nil checks. If an error occurs this can lead to a nil dereference/panic inside traceTx/ApplyTransactionWithEVM. Please handle the error and record it into task.results[i] (similar to the traceTx error path), or abort tracing the block gracefully.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 94 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 103 out of 103 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 103 out of 104 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
core/state_transition.go:1
- This can panic if
st.evm.ChainConfig()is nil or ifTIPTRC21FeeBlockis nil (nil pointer dereference during.Cmp). Consider guarding the comparison (e.g., checkcfg != nil && cfg.TIPTRC21FeeBlock != nil) or using aChainConfighelper method that encapsulates the nil-safe fork check.
core/state_transition.go:1 - These are newly introduced error strings that callers/tests appear to match via substring checks. To make this easier to handle robustly across the codebase, consider defining package-level sentinel errors (similar to
txpool.ErrMissingChainConfig) and wrapping them (so callers can useerrors.Is) rather than relying on string matching.
core/genesis.go:1 - This adds an unconditional
Infolog on everySetupGenesisBlockcall (including tests and any code paths that call config loading/setup multiple times). Consider lowering this toDebug, or logging only when the config is written/changed or whencompatErr/erris non-nil, to avoid noisy startup logs and large log volume in repeated setup/load scenarios.
contracts/trc21issuer/trc21issuer_test.go:1 - This test constructs and uses a cloned
chainConfig(with at leastTRC21IssuerSMCoverridden) when creating the simulated backend, but computes expected fees usingparams.TestXDPoSMockChainConfiginstead of the actualchainConfigin use. To avoid false positives/negatives if fee fork blocks differ in future edits, compute fees using the same config instance passed into the backend.
contracts/trc21issuer/trc21issuer_test.go:1 - This test constructs and uses a cloned
chainConfig(with at leastTRC21IssuerSMCoverridden) when creating the simulated backend, but computes expected fees usingparams.TestXDPoSMockChainConfiginstead of the actualchainConfigin use. To avoid false positives/negatives if fee fork blocks differ in future edits, compute fees using the same config instance passed into the backend.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 109 out of 110 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
core/state/trc21_reader.go:161
- UpdateTRC21Fee writes issuer token-capacity state and subtracts balance from the TRC21 issuer address. If TRC21IssuerSMC resolves to the zero address (e.g., chain config not set on StateDB), this will mutate account(0) and corrupt state. Add a guard to no-op or error when issuer is the zero address (or when chain config is missing).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 112 out of 115 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
eth/tracers/api.go:545
core.TransactionToMessagenow returns an error in some cases. Ignoring it here (msg, _ := ...) can result in a nilmsgand a panic inApplyMessagewhen tracing blocks whose chain config is missing required fork fields. Capture and handle the error (return it, or short-circuit with roots collected so far).
eth/tracers/api.go:628core.TransactionToMessagecan fail now, but the error is ignored here. If it returns an error,msgwill be nil andtraceTxwill likely panic. Please handle the error and return it (or store it inresults[i]like other trace failures).
eth/tracers/api.go:683core.TransactionToMessagenow returns an error for some chain-config dependent cases; this worker path ignores it. A nilmsgcan panic insidetraceTx. Handle the error and setresults[task.index]to an error result (or propagate the error to abort tracing).
eth/tracers/api.go:726core.TransactionToMessageerrors are ignored here (msg, _ := ...). If it returns an error, the subsequentApplyMessagewill dereference a nilmsgand panic. Handle the error and abort the trace loop cleanly (e.g., setfailedand break).
eth/state_accessor.go:250core.TransactionToMessagenow returns an error in some cases. This call ignores the error, which can leavemsgnil and causeApplyMessageto panic while replaying transactions to reachtxIndex. Please handle and return the error (or wrap it with tx/hash context).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 125 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
ethclient/simulated/backend.go:1037
state.Newcan fail, but the returned error is ignored and the newly addedSetChainConfigcall dereferencesb.pendingStateunconditionally. Handle the error before using the StateDB soSendTransactionreturns an error instead of panicking if the pending state cannot be reopened.
ethclient/simulated/backend.go:1172- This ignores the error from
state.Newand then immediately dereferencesb.pendingState. If rebuilding the adjusted pending state fails,AdjustTimewill panic rather than returning the failure to the caller. Check and return the error before setting the chain config.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 132 out of 135 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
core/state/statedb.go:265
- The comment says this accessor panics when the state/config/address is missing, but the implementation returns an error in those cases. Please update the doc comment so callers know they must handle the returned error rather than expect a panic.
Consolidate per-network constants into common/constants.go and
common/constants.shared.go, dropping the build-tag constant matrix
(constants.{mainnet,testnet,devnet,local,all}.go).
Move gas pricing helpers from common to params and make them
ChainConfig-driven, so fork-aware pricing is derived from the active
config instead of process-wide globals.
Split params/config.go into focused files (config_networks,
config_forks, config_xdpos, config_backfill, config_compat) and
extend ChainConfig with XDC fork switches and system-contract
addresses. Add clone and semantic-equality helpers, and preserve
explicit-zero JSON fields via presence-aware marshal/unmarshal.
Introduce core/startup with an explicit decision flow, and rework
genesis loading/recovery to:
- hydrate and validate config by network class,
- backfill only missing fields,
- preserve compatibility errors,
- allow same-hash custom override only in guarded recovery mode.
Refresh bundled genesis JSON files to the new schema and thread
ChainConfig through consensus, core, eth, miner, txpool, tracers,
the simulated backend and related helpers.
Add regression coverage for built-in / local / custom / same-hash
startup paths and denylist validation, and document the operator
upgrade/repair workflow in docs/upgrade.md.
Proposed changes
Consolidate per-network constants into common/constants.go and common/constants.shared.go, dropping the build-tag constant matrix (constants.{mainnet,testnet,devnet,local,all}.go).
Move gas pricing helpers from common to params and make them ChainConfig-driven, so fork-aware pricing is derived from the active config instead of process-wide globals.
Split params/config.go into focused files (config_networks, config_forks, config_xdpos, config_backfill, config_compat) and
extend ChainConfig with XDC fork switches and system-contract addresses. Add clone and semantic-equality helpers, and preserve explicit-zero JSON fields via presence-aware marshal/unmarshal.
Introduce core/startup with an explicit decision flow, and rework genesis loading/recovery to:
Refresh bundled genesis JSON files to the new schema and thread ChainConfig through consensus, core, eth, miner, txpool, tracers, the simulated backend and related helpers.
Add regression coverage for built-in / local / custom / same-hash startup paths and denylist validation, and document the operator upgrade/repair workflow in docs/upgrade.md.
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