erigontech / erigontech/erigon

execution/state/genesiswrite: a block-fork conflict at block 0 is written silently

Open
#23,563 1 comment 0 reactions 1 assignee Claimed by @awskii View on GitHub
tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

`WriteGenesisBlock` drops a block-fork compatibility error whose rewind target is 0, so a fork scheduled at block 0 or 1 can be rescheduled on a chain that is already past it and the new config is written over the stored one.

The guard (`execution/state/genesiswrite/genesis_write.go:264`):

```go
compatibilityErr := storedCfg.CheckCompatible(newCfg, *height, headTime)
if compatibilityErr != nil &&
(compatibilityErr.RewindTo != 0 || compatibilityErr.HasTimestampConflict()) {
```

`newCompatError` only sets `RewindTo` when the earlier of the two schedules is `> 0` (`execution/chain/chain_config.go:847-854`), so a fork at block 0 or 1 yields `RewindTo == 0` and a real conflict reads as none.

The reachable case is the chain ID. `checkCompatibleBlocks` builds `newCompatError("EIP155 chain ID", c.SpuriousDragonBlock, newcfg.SpuriousDragonBlock)` (`chain_config.go:755`), and `eip155Block: 0` ships on sepolia, hoodi, gnosis and chiado — so on any of them the error is always `RewindTo == 0`. Re-running `erigon init` against a config with a different `chainId` on a synced datadir is accepted, `rawdb.WriteChainConfig` persists it, and the node runs replay protection its own executed history contradicts.

`homesteadBlock: 0` moved forward on a private chain has the same shape.

## Why it is not fixed in #23466

That PR is the timestamp axis. The block-axis tolerance predates it and is unchanged there; @yperbasis kept it out of scope explicitly when approving. It matters more now than before, because that PR also made a compatibility error abort startup instead of being swallowed — so tightening this would newly refuse to boot on datadirs that start today, which deserves its own change rather than riding along.

## Shape of the fix

```go
if compatibilityErr != nil &&
(compatibilityErr.HasBlockConflict() || compatibilityErr.HasTimestampConflict()) {
```

`HasBlockConflict()` already exists (added in #23466) and is currently used only by `Error()` and tests. The numeric tolerance has no remaining purpose: the genesis-only database it was protecting is already excluded by the enclosing `*height != 0`.

Two callers deliberately swallow `ConfigCompatError` and are worth checking first, since they suggest zero-rewind conflicts occur routinely in those flows:

- `cmd/integration/commands/stages.go:1185`
- `execution/execmodule/execmoduletester/exec_module_tester.go:574`

Wants a regression test that changes the stored chain ID at a non-zero head and asserts both an error and that the stored `ChainID` is unchanged.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.