refactor(params,core): centralize fork activation in ChainConfig#2329
refactor(params,core): centralize fork activation in ChainConfig#2329gzliudan wants to merge 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 |
gzliudan
changed the title
gzliudan
changed the title
gzliudan
force-pushed
the
move-common-global-var
branch
from
c0ca61f to
b87a72c
Compare
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
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.
gzliudan
force-pushed
the
move-common-global-var
branch
3 times, most recently
from
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.
gzliudan
force-pushed
the
move-common-global-var
branch
3 times, most recently
from
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.
gzliudan
force-pushed
the
move-common-global-var
branch
4 times, most recently
from
f6109e8 to
15b851a
Compare
gzliudan
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gzliudan
force-pushed
the
move-common-global-var
branch
6 times, most recently
from
c456a50 to
f110367
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
gzliudan
force-pushed
the
move-common-global-var
branch
2 times, most recently
from
2b45acd to
82c6af4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 51 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.
gzliudan
force-pushed
the
move-common-global-var
branch
from
82c6af4 to
4a45dbb
Compare
gzliudan
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 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 74 out of 74 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
core/state_transition.go:186
- TransactionToMessage dereferences chainConfig.TIPTRC21FeeBlock when balanceFee != nil. If chainConfig is nil or TIPTRC21FeeBlock is nil, this will panic. Consider validating inputs (returning a descriptive error) or ensuring a non-nil default is applied before comparing blockNumber against TIPTRC21FeeBlock.
💡 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 74 out of 74 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
core/state_transition.go:186
- TransactionToMessage dereferences chainConfig.TIPTRC21FeeBlock when balanceFee != nil (token-fee path). Since TransactionToMessage is exported and chainConfig is a pointer parameter, passing a nil chainConfig (or a config with nil TIPTRC21FeeBlock) will panic at runtime. Please add an explicit guard (returning an error) when balanceFee != nil and chainConfig/TIPTRC21FeeBlock is nil, so callers get a deterministic error instead of a panic.
💡 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 83 out of 83 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.
💡 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 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.
Replace common-level fork switch globals with ChainConfig as the single source of truth for fork activation checks across core paths. Canonicalize built-in network configs by genesis hash during setup/load, keep explicit custom genesis schedules, and backfill legacy stored custom configs only for compatibility. Add regression coverage for cloned config isolation, built-in fork declarations, startup validation, compatibility checks, and built-in config persistence to prevent missing-field regressions on restart.
Proposed changes
Replace common-level fork switch globals with ChainConfig as the single source of truth for fork activation checks across core paths.
Canonicalize built-in network configs by genesis hash during setup/load, keep explicit custom genesis schedules, and backfill legacy stored custom configs only for compatibility.
Add regression coverage for cloned config isolation, built-in fork declarations, startup validation, compatibility checks, and built-in config persistence to prevent missing-field regressions on restart.
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