fix(params): reject chain configs that skip a fork their rules depend on #20169

[gzliudan]

Proposed changes

CheckConfigForkOrder accepted a chain config that schedules a standard fork while leaving an earlier fork — the one the later forks' rules are selected by — unset. {"chainId": 50, "osakaBlock": 0} passed with Prague, Cancun, EIP1559 and everything before them missing; issue #2771 records the shape and its consequences.

This PR adds a second scan over the standard fork slots in forkOrder order. Forks that decide a whole precompile bag or instruction set, or whose own flag a cross-cutting rule reads directly (Homestead, EIP150, EIP155, EIP158, Byzantium, Petersburg, Istanbul, EIP1559, Prague, Osaka), must be declared before any later standard fork is scheduled; the other standard slots only trigger the check. Every member of that set is demanded unconditionally, Petersburg included: reading its SSTORE gate IsPetersburg || !IsConstantinople alone might suggest the slot can be skipped when ConstantinopleBlock is unset, but upstream does not allow that — its petersburgBlock and istanbulBlock entries are both non-optional, and geth ethereum#21473 states the intent as "fork ordering requirements which state that Petersburg fork be set if Constantinople fork is set". XDC-specific forks stay out of both sets, so the built-in networks are unaffected, and TIPXDCXCancellationFeeBlock remains the effective activation of both Istanbul and Petersburg exactly as IsIstanbul and IsPetersburg define it. A schedule that stops at ConstantinopleBlock is rejected by a separate pair check (chainConfigForkSequencePetersburgPair), because the scan reports a gap only when a later slot is scheduled: IsConstantinople is then true while IsPetersburg stays false, so the net-metered EIP-1283 SSTORE rule runs although no fork declares it. Upstream accepts that shape only because its existence branch needs a later non-optional slot to carry a value, which is the one place where this check is stricter. Both sets are data-driven (config_backfill_fields.json, generator-validated) and the required list is pinned by a test. The rationale, the per-member audit, the Petersburg requirement and the deliberate rejection of a miner-disable-without-Istanbul config are in docs/adr/0002-require-standard-fork-sequence.md.

Fixes #2771.

Summary

 CheckConfigForkOrder (params/config.go)
   ChainID / XDC field requirements
   linear fork comparison                 # unchanged
+  standard fork sequence scan            # new
+    ForEachChainConfigForkSequenceBlock  # params/config_backfill.go
+  petersburg/constantinople pair check   # new
+    chainConfigForkSequencePetersburgPair
   forkOrderSpecialCaseRules              # unchanged

 params/config_backfill_fields.json
+  forkOrderStandard    on the 16 standard fork slots
+  forkSequenceRequired on the 10 whose rules depend on them
+    Petersburg required unconditionally, the way upstream requires it

The scan reports the nearest required predecessor rather than the earliest gap, checks each slot before it updates its state, and never treats the current slot as its own predecessor. The field named as scheduled is the one the value comes from, so a rejection under the cancellation-fee fallback names TIPXDCXCancellationFeeBlock — the field the operator actually wrote — instead of the IstanbulBlock or PetersburgBlock slot it stands in for:

declared = true; lastRequired = ""
for slot in standard slots ordered by forkOrder:
    required = slot.required
    (value, source) = required ? effective(slot) : (rawValue(slot), slot)
    if value != nil and not declared
        error "<lastRequired> is unset, but <source> <value> is scheduled"
    if required
        declared = value != nil
        lastRequired = slot

# the scan only reports a gap when a later slot is scheduled, so this pair
# needs its own check
if ConstantinopleBlock != nil and effective(PetersburgBlock) == nil
    error "PetersburgBlock is unset, but ConstantinopleBlock <value> is scheduled"

Evidence

• Before (dev-upgrade @ 22a40a58f): {"chainId": 50, "osakaBlock": 0} validates; a config with Cancun or Prague scheduled and EIP158Block/EIP1559Block unset runs with no code-size cap (CheckMaxCodeSize gates on IsEIP158).
  After (this branch): invalid chain config: wrong fork switch order: PragueBlock is unset, but OsakaBlock 0 is scheduled, and the uncapped shapes can no longer start.
• New cases in params/config_test.go pin the exact strings for nine rejections (osaka without prague, cancun without eip1559, prague without eip1559, eip1559 without istanbul, berlin without istanbul, the issue's Shanghai-prefix reproduction, petersburg absent without constantinople, petersburg skipped while constantinople is declared and petersburg skipped with nothing later scheduled) and four acceptance boundaries (cancun skipped between eip1559 and prague, petersburg declared after constantinople, petersburg activated through the cancellation fee fork, istanbul activated through the cancellation fee fork), plus the deliberate rejection of a miner-disable-without-Istanbul config. Two more cases guard the scan's own properties: one pins that a config which both inverts a pair and skips a required fork keeps the linear comparison's error, i.e. that the sequence scan stays after it (moving the scan earlier reddens exactly this case), and one pins that the fallback rejection names TIPXDCXCancellationFeeBlock. TestChainConfigForkSequenceRequiredSet pins the required list — Petersburg included, which is what keeps its forkSequenceRequired flag from being dropped — and TestBuiltInChainConfigsPassCheckConfigForkOrder keeps covering all eleven built-in configs.
• Mutation-checked: marking every slot non-required reddens the required-set pin and the rejection cases; disabling the Istanbul effective value reddens exactly XDCMainnetChainConfig, TestnetChainConfig, DevnetChainConfig and LocalnetChainConfig, while MainnetChainConfig still passes. Dropping Petersburg's forkSequenceRequired flag reddens TestChainConfigForkSequenceRequiredSet and petersburg absent without constantinople, so the flag has its own anchor now that the conditional narrowing is gone. Deleting the pair check reddens exactly petersburg skipped with nothing later scheduled, and dropping its ConstantinopleBlock guard reddens TestChainConfigForkSequencePetersburgPairInvariant's constantinople absent shape and MainnetChainConfig in the built-in-config guard. TestChainConfigForkSequencePetersburgPairInvariant pins the four shapes of the pair, and TestChainConfigForkSequenceFallbackMirrorsRuntimeFlags pins that the slots for which the validation substitutes the cancellation-fee fork are exactly the slots whose runtime flag consults it, so the two copies of that rule cannot drift apart.
• Local runs on the commit: gofmt/goimports clean, make all, make quick-test (no failures), go test ./params/... ./core/... ./eth/... without -short (no failures), make tidy (no module change), make generate (no stale files).

Merge Danger

Door: two-way for the repository, one-way for an operator whose private chain config is incomplete.

A custom or legacy chain config that schedules a standard fork without declaring the required prefix — or that declares no standard forks at all while relying on a migrated TIPXDCXCancellationFeeBlock — now fails at startup. All five built-in networks pass unchanged: XDCMainnet (50), Mainnet (1), Testnet (51), Devnet (551) and Localnet (5151) all leave constantinopleBlock, petersburgBlock and istanbulBlock unset, and the first four rely on TIPXDCXCancellationFeeBlock (38383838 / 23779191 / 0 / 0) as the effective Petersburg and Istanbul cutoff; MainnetChainConfig has no fallback. IsConstantinople has no fallback and stays false on those networks, so Petersburg's condition never reaches them.

The one-way part is the restart, not just the first init: a stored config is validated on every startup as well. SetupGenesisBlockWithOverride resolves the stored config (with no --genesis, chainConfigOrDefault returns it) and resolveSetupStoredConfigResult calls CheckConfigForkOrder on it, XDPoS.New checks the same config again, and the read-only LoadChainConfigWithCompatWithOverride path rejects it too — upstream's config-resolution helper ends in cfg.CheckConfigForkOrder() as well. An operator of such a chain therefore cannot restart after upgrading, and the repair is a rule change rather than a config detail: declaring HomesteadBlock changes the signer MakeSigner picks (Frontier versus Homestead) and the contract-creation intrinsic gas (21000 versus 53000), and declaring EIP158Block flips deleteEmptyObjects, so the state root of every block changes. Accepting that as a rule change for the chain, or staying on the previous release, are the two honest options.

Blast Radius: params startup validation plus eight test fixtures, thirteen fixture sites, and one more fixture field for Petersburg's unconditional requirement (the pair check added none).

The fixtures had to gain the earlier standard forks — the check rejects the shapes they were written as — and two of them, the ones that execute blocks or EVM code, end up running under an upgraded rule set: TestEIP161AccountRemoval gains EIP150Block, which turns rules.IsEIP150 on for every block it imports, and TestApplyTransactionWithEVMKeepsCoinbaseFeeRecipientAtTIPTRC21ActivationBlock gains PetersburgBlock and IstanbulBlock, which turn rules.IsIstanbul on at block 0, the height at which it runs the EVM. Scheduling those completions beyond the blocks each fixture reaches is not an alternative: the linear comparison runs first and rejects it, measured on 2026-10-13 as IstanbulBlock 1000000000 > TIPTRC21FeeBlock 0 and EIP150Block 1000000000 > EIP155Block 0. ExampleGenerateChain completes its prefix at futureFork and stays under the rule set it always ran, and the remaining subjects (DAO fork ranges, gas-tier rules, EIP-161 removal, worker pre-commit handling, the XDPoS V2 rejection cases) are unchanged. Those thirteen additions across eight files are written out per fixture rather than shared: the earlier forks are part of each fixture's own setup, and a cross-package helper would need an exported test-only API in params. #2756 / #2757 touch the same files from the same base, so whichever lands second needs a trivial rebase of params/config*.go. Petersburg's unconditional requirement touches one fixture: the core/state_processor_test.go config that schedules istanbulBlock from genesis now declares petersburgBlock beside it, because the shape upstream rejects is rejected here too. The shapes the check separates are pinned as test cases as well. The pair check adds no fixture either — measured with an overlay prototype across ./core/... ./miner/... ./cmd/XDC/... ./consensus/... ./eth/... ./internal/ethapi/... (42 packages, zero failures); the one fixture that declares constantinopleBlock without petersburgBlock (core/genesis_setup_test.go) never reaches the check, because its chain ID resolves to the built-in Localnet profile which backfills TIPXDCXCancellationFeeBlock first.

ADR numbering: this PR ships docs/adr/0002-require-standard-fork-sequence.md; 0001 belongs to #2748 (evict-executed-marker) and 0003 to the gap ADR that lives outside this branch, so a lone 0002 is expected until #2748 lands.

Types of changes

What types of changes does your code introduce to XDC network?
Put an ✅ in the boxes that apply

• build: Changes that affect the build system or external dependencies
• ci: Changes to CI configuration files and scripts
• chore: Changes that don't change source code or tests
• docs: Documentation only changes
• feat: A new feature
• fix: A bug fix
• perf: A code change that improves performance
• refactor: A code change that neither fixes a bug nor adds a feature
• revert: Revert something
• style: Changes that do not affect the meaning of the code
• test: Adding missing tests or correcting existing tests

Impacted Components

Which parts of the codebase does this PR touch?
Put an ✅ in the boxes that apply

• Consensus
• Account
• Network
• Geth
• Smart Contract
• External components
• Not sure (Please specify below)

Checklist

Put an ✅ in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

• This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
• Tested on a private network from the genesis block and monitored the chain operating correctly for multiple epochs. N/A: startup validation of the chain config only, no runtime or consensus path changes.
• Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet. N/A: see above.
• Tested the backwards compatibility. Built-in and stored configs keep resolving; the rejection fires only on configs that skip a required fork.
• Tested with XDC nodes running this version co-exist with those running the previous version. N/A: no wire, consensus or state change.
• Relevant documentation has been updated as part of this PR (docs/adr/0002-require-standard-fork-sequence.md)
• N/A

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 03692463-16f3-451a-82d6-ff31f33f1f6a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

🟡 Changes recommended

The Petersburg policy contradicts its stated upstream behavior, and dual activation fields can produce misleading validation errors.

2 open findings

• Istanbul exemption diverges from upstream fork validation · New
• Report earliest effective activation block for conflicting fork fields · New

What changed in this PR

Adds stricter chain-config validation to reject standard fork schedules that omit required predecessors.

Changes:

• Adds generated metadata and sequence validation for standard forks.
• Handles Petersburg/Istanbul fallback activation and Petersburg pairing.
• Expands tests, fixtures, and migration documentation.

File	Description
params/​config.go	Runs the new fork-sequence validation.
params/​config_test.go	Tests accepted and rejected schedules.
params/​config_backfill.go	Implements sequence and fallback helpers.
params/​config_backfill_test.go	Pins required forks and fallback behavior.
params/​config_backfill_generator.go	Generates sequence metadata.
params/​config_backfill_generated.go	Contains generated fork classifications.
params/​config_backfill_fields.json	Defines standard and required forks.
docs/​adr/​0002-require-standard-fork-sequence.md	Documents rationale and migration impact.
miner/​worker_test.go	Completes a worker fixture’s fork prefix.
core/​state_processor_test.go	Adds Istanbul to a transaction fixture.
core/​genesis_setup_test.go	Updates genesis-validation fixtures.
core/​genesis_load_test.go	Updates stored-config fixture setup.
core/​genesis_compat_test.go	Updates compatibility fixtures.
core/​chain_makers_test.go	Completes the example fork schedule.
core/​blockchain_test.go	Completes the EIP-161 fixture.
cmd/​XDC/​dao_test.go	Adds Homestead to the DAO fixture.

Files not reviewed (1)

• params/config_backfill_generated.go: Generated file

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

It changes startup acceptance for stored consensus configurations and still contains inaccurate behavior and migration documentation.

2 open findings

• Incorrectly describes Berlin instruction set as a strict superset · New
• Misstates XDC fork fields' effect on sequence validation · New

2 resolved since last review

• Istanbul exemption diverges from upstream fork validation
• Report earliest effective activation block for conflicting fork fields

Files not reviewed (1)

• params/config_backfill_generated.go: Generated file

🧠 Review effort: Balanced