rawdb: consolidate genesis and config writes#20441
Conversation
Introduces the rawdb-layer contract for "bytes that make a valid chain DB at block 0": a GenesisBundle struct (Genesis, Config, Block, TD) with WriteGenesisBundle / ReadGenesisBundle. Fresh-DB mode writes every key (genesis JSON, block, TD, canonical hash, head hashes, chain config); config-only mode rewrites just the chain config for an existing block. Includes a round-trip test, a config-only rewrite test, an empty-DB read test, and a nil-bundle rejection test. This is Task 1 of the genesis consolidation refactor — no callers yet.
Introduces the policy-layer consolidation for genesis writing. The new CommitGenesisTx owns stored-vs-fresh branching, config compat checks, overrides, chain-name fallback, and the repair-missing-chain-config path, and composes the rawdb.GenesisBundle primitives from Task 1 so fresh-DB writes can no longer skip TxNums[0] or the ConfigTable[GenesisKey] entry. Also fixes the ordering bug where the old WriteGenesisBlock persisted the genesis JSON before validating Config != nil. New tests cover (a) fresh-DB key presence (the regression net), (b) the repair branch, (c) the ordering bug fix, and (d) the mismatch error path. CommitGenesisTxWithPrecomputedBlock is added for the aura_test.go use case where the block must be computed outside this package.
Thin wrapper around genesiswrite.CommitGenesis that test code uses in place of the soon-to-be-deleted genesiswrite.MustCommitGenesis. Accepts a nil tb so non-test callers (cmd/evm/runner) can still get fail-fast semantics via panic.
Replace the manual ReadGenesis + ReadCanonicalHash + "pass nil if canonical exists" dance with ReadGenesisBundle + genesiswrite.CommitGenesisTx. The old policy lived both in backend.go and inside WriteGenesisBlock; it now lives in one place.
Replaces the legacy CommitGenesisBlock/MustCommitGenesis entry points
with the consolidated genesiswrite.CommitGenesis(Options{...}) API in
init_cmd.go, stages.go, and runner.go.
…Genesis Migrates the engineapitester and execmoduletester test harnesses from the old genesiswrite.CommitGenesisBlock API to the new Options-struct-based CommitGenesis entry point. Also fixes CommitGenesisTx (and CommitGenesisTxWithPrecomputedBlock) to derive TD from block.Difficulty() rather than g.Difficulty.ToBig(): some test genesis specs pass nil Difficulty, which the old WriteTd path stored silently but the new WriteGenesisBundle rejects with an explicit nil check. The block header's difficulty is always populated by GenesisWithoutStateToBlock (defaulted from params.GenesisDifficulty), so this is the correct source.
Replaces the direct WriteGenesisBesideState call with the new CommitGenesisTxWithPrecomputedBlock entry point. The test still pre-computes the block via GenesisToBlock so the IBS lands in the real temporal domains; CommitGenesisTxWithPrecomputedBlock persists the pre-computed block without invoking GenesisToBlock internally.
Replaces direct genesiswrite.MustCommitGenesis calls (soon to be deleted) with the genesistest test helper introduced earlier in the genesis consolidation series. Swaps the genesiswrite import for genesistest since that was the only remaining use in the file.
Replace all CommitGenesisBlock / WriteGenesisBlock / MustCommitGenesis call sites in genesis_test.go with the new Options-based API. Adds explicit GasLimit / Difficulty to customg (matching the defaults that GenesisWithoutStateToBlock would fill in, so customghash is unchanged) because CommitGenesis now persists the Genesis JSON via WriteGenesisIfNotExist, and types.Genesis JSON decode rejects missing gasLimit / difficulty. The old MustCommitGenesis silently skipped the JSON persist, hiding this invariant.
Removes the five overlapping legacy entry points that were superseded by CommitGenesis / CommitGenesisTx / CommitGenesisTxWithPrecomputedBlock in tasks 2-9: CommitGenesisBlock, CommitGenesisBlockWithOverride, WriteGenesisBlock, MustCommitGenesis, and WriteGenesisBesideState (plus the private write() helper). All call sites were migrated in earlier tasks and grep confirms no live callers remain. The new writeFreshGenesisDB helper composes rawdb.WriteGenesisBundle directly, so WriteGenesisBesideState has no internal users either and is deleted rather than unexported.
Verified grep cleanliness, make erigon integration build, make test-short, make test-all (excluding a pre-existing LFS-blocked test unrelated to this refactor), double-pass make lint, and the Task 2 regression-net self-check (confirmed fresh-DB key-presence test catches a deliberately-broken WriteGenesisBundle before reverting).
- node/eth/backend: restore the old genesisSpec=nil idiom on existing DBs. The refactor had started passing config.Genesis unconditionally, forcing CommitGenesisTx to run GenesisToBlock on every restart (perf regression) and rendering the "preserve stored config" branch unreachable for erigon-init-style private chains. - genesiswrite.CommitGenesisTx: hoist the g.Config==nil check above the fresh-vs-existing branch so callers that pass a Config-less Genesis on an existing DB get ErrGenesisNoConfig instead of a later nil dereference. Drop the dead localOpts copy. - rawdb.ReadGenesisBundle: return an explicit error when the canonical hash at height 0 is set but the block body is missing, so the policy layer cannot silently treat a partially-corrupt DB as fresh and overwrite the remaining block-zero metadata. - genesistest.MustCommitGenesis: drop the dead nil-tb branch — no caller passes nil now that cmd/evm/runner inlines its own panic. - Tests: add HeaderNumber[hash] to the fresh-DB key-presence regression net; expand the WriteGenesisBundle nil-rejects test to cover every validation branch including the FreshDB=false happy path.
The prior review pass added an explicit error when the canonical hash at height 0 is present but the block body is missing, preventing CommitGenesisTx from silently treating a corrupt DB as fresh and overwriting its remaining block-zero metadata. Adding the regression test that protects this fix against future refactors.
awskii
requested review from
AskAlexSharov,
Giulio2002,
anacrolix,
bloxster,
mh0lt,
sudeepdino008,
taratorio and
yperbasis
as code owners
There was a problem hiding this comment.
Pull request overview
This PR consolidates genesis + chain config persistence by introducing a single “genesis bundle” write/read path in rawdb and a unified policy layer in genesiswrite, then migrates production and test call sites to the new APIs.
Changes:
- Add
rawdb.GenesisBundlewithWriteGenesisBundle/ReadGenesisBundleto centralize block-0 DB metadata persistence. - Replace the previous multiple genesis-writing entry points with
genesiswrite.CommitGenesis/CommitGenesisTx(+ a precomputed-block variant) driven by anOptionsstruct. - Migrate node startup, CLI tools, and multiple test harnesses to the new consolidated APIs; add regression tests for key presence, repair paths, and ordering.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
db/rawdb/accessors_metadata.go |
Adds GenesisBundle persistence/read APIs used by higher-level genesis commit logic. |
db/rawdb/genesis_bundle_test.go |
Adds tests for bundle round-trip, config-only rewrite, empty DB, and partial DB error handling. |
execution/state/genesiswrite/genesis_write.go |
Introduces Options, CommitGenesis/CommitGenesisTx, fresh-vs-existing semantics, repair path, and precomputed-block commit. |
execution/state/genesiswrite/commit_genesis_test.go |
Adds regression tests ensuring block-0 metadata keys exist and repair/mismatch/ordering behaviors hold. |
execution/state/genesiswrite/genesistest/genesistest.go |
Adds a test-only helper wrapper for committing genesis in tests. |
execution/state/genesiswrite/genesis_test.go |
Migrates existing tests to new commit APIs; adjusts custom genesis fixture fields. |
node/eth/backend.go |
Switches node startup genesis initialization to ReadGenesisBundle + CommitGenesisTx. |
cmd/utils/app/init_cmd.go |
Migrates erigon init genesis commit to the new CommitGenesis API. |
cmd/integration/commands/stages.go |
Migrates integration sync setup genesis commit to the new CommitGenesis API. |
cmd/evm/runner.go |
Replaces removed MustCommitGenesis usage with CommitGenesis + error handling. |
execution/execmodule/execmoduletester/exec_module_tester.go |
Updates test harness genesis initialization to CommitGenesis. |
execution/engineapi/engineapitester/engine_api_tester.go |
Updates engine API tester genesis initialization to CommitGenesis. |
execution/protocol/rules/aura/aura_test.go |
Migrates from WriteGenesisBesideState to CommitGenesisTxWithPrecomputedBlock. |
p2p/sentry/sentry_grpc_server_test.go |
Replaces removed genesiswrite.MustCommitGenesis with genesistest.MustCommitGenesis. |
docs/plans/completed/20260408-genesis-consolidation.md |
Adds a completed plan documenting the consolidation work and rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := WriteGenesisIfNotExist(tx, b.Genesis); err != nil { | ||
| return err | ||
| } | ||
| if err := WriteBlock(tx, b.Block); err != nil { | ||
| return err | ||
| } | ||
| if err := WriteTd(tx, hash, b.Block.NumberU64(), b.TD); err != nil { | ||
| return err | ||
| } | ||
| if err := WriteCanonicalHash(tx, hash, b.Block.NumberU64()); err != nil { |
There was a problem hiding this comment.
WriteGenesisBundle is documented as persisting the keys that make the DB valid at block 0, but in fresh-DB mode it uses b.Block.NumberU64() when writing TD and the canonical hash. This allows a caller to accidentally persist a non-genesis block number and create an invalid/ambiguous DB state. Consider enforcing b.Block.NumberU64()==0 in FreshDB mode (and/or always writing number=0 regardless of the header) to match the contract and how ReadGenesisBundle reads height 0.
| if err := WriteGenesisIfNotExist(tx, b.Genesis); err != nil { | |
| return err | |
| } | |
| if err := WriteBlock(tx, b.Block); err != nil { | |
| return err | |
| } | |
| if err := WriteTd(tx, hash, b.Block.NumberU64(), b.TD); err != nil { | |
| return err | |
| } | |
| if err := WriteCanonicalHash(tx, hash, b.Block.NumberU64()); err != nil { | |
| number := b.Block.NumberU64() | |
| if number != 0 { | |
| return fmt.Errorf("WriteGenesisBundle: bundle.Block number must be 0 in fresh-DB mode, got %d", number) | |
| } | |
| if err := WriteGenesisIfNotExist(tx, b.Genesis); err != nil { | |
| return err | |
| } | |
| if err := WriteBlock(tx, b.Block); err != nil { | |
| return err | |
| } | |
| if err := WriteTd(tx, hash, 0, b.TD); err != nil { | |
| return err | |
| } | |
| if err := WriteCanonicalHash(tx, hash, 0); err != nil { |
| return nil, err | ||
| } | ||
| if block == nil { | ||
| return nil, fmt.Errorf("ReadGenesisBundle: canonical hash %x is set at height 0 but block body is missing", hash) |
There was a problem hiding this comment.
ReadGenesisBundle returns an error saying the "block body is missing" when ReadBlockWithSenders returns nil. That nil can also happen when the header is missing (or other block components are missing), so the message is misleading for diagnosis. Consider changing the wording to something like "genesis block/header/body is missing" (or include more detail about what was attempted) so operators can distinguish corruption cases.
| return nil, fmt.Errorf("ReadGenesisBundle: canonical hash %x is set at height 0 but block body is missing", hash) | |
| return nil, fmt.Errorf("ReadGenesisBundle: canonical hash %x is set at height 0 but genesis block/header/body is missing", hash) |
| if stored.Block == nil { | ||
| g := opts.Genesis | ||
| custom := true | ||
| if genesis == nil { | ||
| if g == nil { | ||
| logger.Info("Writing main-net genesis block") | ||
| genesis = chainspec.MainnetGenesisBlock() | ||
| g = chainspec.MainnetGenesisBlock() | ||
| custom = false | ||
| } | ||
| applyOverrides(genesis.Config) | ||
| block, _, err1 := write(tx, genesis, dirs, logger) | ||
| if err1 != nil { | ||
| return genesis.Config, nil, err1 | ||
| applyOverridesTo(g.Config, opts) | ||
| if err := g.Config.CheckConfigForkOrder(); err != nil { | ||
| return g.Config, nil, err | ||
| } | ||
| block, _, err := GenesisToBlock(nil, g, opts.Dirs, logger) | ||
| if err != nil { | ||
| return g.Config, nil, err | ||
| } | ||
| // Derive TD from the block header, not g.Difficulty: GenesisWithoutStateToBlock | ||
| // defaults the header difficulty from params.GenesisDifficulty when g.Difficulty | ||
| // is nil, so block.Difficulty is always valid while g.Difficulty.ToBig() may be nil. | ||
| blockDifficulty := block.Difficulty() | ||
| if err := writeFreshGenesisDB(tx, &rawdb.GenesisBundle{ | ||
| Genesis: g, | ||
| Config: g.Config, | ||
| Block: block, | ||
| TD: blockDifficulty.ToBig(), | ||
| }); err != nil { |
There was a problem hiding this comment.
On the fresh-DB path, CommitGenesisTx persists the raw genesis JSON via rawdb.WriteGenesisBundle(…Genesis: g…). However types.Genesis JSON encoding omits the required difficulty field when g.Difficulty == nil, which means CommitGenesisTx can successfully initialize a DB that then fails to reopen because rawdb.ReadGenesis / ReadGenesisBundle will fail JSON decoding. To prevent creating an unreopenable DB, either (a) normalize g before persisting (e.g., if g.Difficulty==nil set it from the computed block header difficulty; likewise consider syncing g.GasLimit if it was 0 and defaults were applied), or (b) return a validation error when required fields for JSON persistence are missing.
| // applyOverridesTo copies any fork-time overrides from opts onto cfg. | ||
| func applyOverridesTo(cfg *chain.Config, opts Options) { | ||
| if opts.OverrideOsakaTime != nil { | ||
| cfg.OsakaTime = opts.OverrideOsakaTime | ||
| } | ||
| if chainName != "" { | ||
| spec, err := chainspec.ChainSpecByName(chainName) | ||
| if err == nil && spec.GenesisHash == genesisHash { | ||
| return spec.Config | ||
| } | ||
| if opts.OverrideAmsterdamTime != nil { | ||
| cfg.AmsterdamTime = opts.OverrideAmsterdamTime | ||
| } |
There was a problem hiding this comment.
applyOverridesTo mutates the passed *chain.Config in-place. In the existing-DB path, configOrDefault can return shared singleton configs (e.g. chainspec.Spec.Config from execution/chain/spec/config.go or chain.AllProtocolChanges from execution/chain/chain_config.go). Mutating those shared pointers can leak override values across calls and can introduce data races. Consider cloning the config before applying overrides when the source may be shared (chain.Config docs note it must be copied with jinzhu/copier due to sync.Once), or change configOrDefault to always return a per-call copy.
| // CommitGenesisTxWithPrecomputedBlock is the fresh-DB path only, using a | ||
| // caller-provided block instead of invoking GenesisToBlock internally. Used by | ||
| // aura_test.go where the block is computed outside this package so its | ||
| // IntraBlockState can be wired into the real temporal domains (running | ||
| // GenesisToBlock here would create a disposable temporal DB and defeat that). | ||
| // | ||
| // The caller is responsible for passing a block that matches opts.Genesis and | ||
| // whose state root was computed against the intended domains. | ||
| func CommitGenesisTxWithPrecomputedBlock(tx kv.RwTx, opts Options, block *types.Block) (*chain.Config, *types.Block, error) { | ||
| logger := opts.Logger | ||
| if logger == nil { | ||
| logger = log.New() | ||
| } | ||
| g := opts.Genesis | ||
| if g == nil { | ||
| return nil, nil, errors.New("CommitGenesisTxWithPrecomputedBlock: opts.Genesis must not be nil") | ||
| } | ||
| if block == nil { | ||
| return nil, nil, errors.New("CommitGenesisTxWithPrecomputedBlock: block must not be nil") | ||
| } | ||
| if g.Config == nil { | ||
| return chain.AllProtocolChanges, nil, types.ErrGenesisNoConfig | ||
| } | ||
| applyOverridesTo(g.Config, opts) | ||
| if err := g.Config.CheckConfigForkOrder(); err != nil { | ||
| return g.Config, nil, err | ||
| } | ||
| blockDifficulty := block.Difficulty() | ||
| if err := writeFreshGenesisDB(tx, &rawdb.GenesisBundle{ | ||
| Genesis: g, | ||
| Config: g.Config, | ||
| Block: block, | ||
| TD: blockDifficulty.ToBig(), | ||
| }); err != nil { | ||
| return g.Config, nil, err | ||
| } |
There was a problem hiding this comment.
CommitGenesisTxWithPrecomputedBlock is documented as "fresh-DB path only", but it does not verify the DB is actually fresh (e.g., canonical hash at height 0 is empty) and it does not enforce block.NumberU64()==0. As written, a caller could accidentally overwrite an existing DB’s block-zero metadata or persist a non-zero block as “genesis”. Consider adding an explicit freshness guard (e.g., rawdb.ReadGenesisBundle/ReadCanonicalHash(0) check) and rejecting non-zero block numbers before calling writeFreshGenesisDB; also consider the same genesis JSON normalization/validation around Difficulty as in CommitGenesisTx.
|
|
||
| - [x] create package `genesistest` in `execution/state/genesiswrite/genesistest/` | ||
| - [x] implement `MustCommitGenesis(tb testing.TB, g *types.Genesis, db kv.RwDB, dirs datadir.Dirs, logger log.Logger) *types.Block`: | ||
| wraps `genesiswrite.CommitGenesis(context.Background(), db, genesiswrite.Options{Genesis: g, Dirs: dirs, Logger: logger})`, calls `tb.Fatal(err)` on error, returns the block. `tb` may be nil for non-test callers (cmd/evm/runner-style) — in that case, panic on error. |
There was a problem hiding this comment.
The plan says genesistest.MustCommitGenesis allows tb == nil and will panic on error, but the implemented helper (execution/state/genesiswrite/genesistest/genesistest.go) unconditionally calls tb.Helper()/tb.Fatal() and requires a non-nil TB. Please update this plan text to match the actual behavior (or adjust the helper to support nil TB as described).
| wraps `genesiswrite.CommitGenesis(context.Background(), db, genesiswrite.Options{Genesis: g, Dirs: dirs, Logger: logger})`, calls `tb.Fatal(err)` on error, returns the block. `tb` may be nil for non-test callers (cmd/evm/runner-style) — in that case, panic on error. | |
| wraps `genesiswrite.CommitGenesis(context.Background(), db, genesiswrite.Options{Genesis: g, Dirs: dirs, Logger: logger})`, calls `tb.Helper()` and `tb.Fatal(err)` on error, and returns the block. This helper is for test callers and requires a non-nil `tb`. |
| gspecNoFork = &types.Genesis{Config: configNoFork} | ||
| gspecProFork = &types.Genesis{Config: configProFork} |
There was a problem hiding this comment.
These tests commit a Genesis that only sets Config. With the new CommitGenesis/WriteGenesisBundle path, the genesis JSON is persisted and types.Genesis JSON decoding requires difficulty (and uses GasLimit defaults in the block header). A Config-only genesis can therefore create a DB that cannot be reopened because rawdb.ReadGenesis will fail JSON decoding. Either set Difficulty (and consider setting GasLimit) in gspecNoFork/gspecProFork, or make CommitGenesis normalize these defaults before persisting the genesis JSON.
| gspecNoFork = &types.Genesis{Config: configNoFork} | |
| gspecProFork = &types.Genesis{Config: configProFork} | |
| gspecNoFork = &types.Genesis{ | |
| Config: configNoFork, | |
| Difficulty: big.NewInt(1), | |
| GasLimit: 5000, | |
| } | |
| gspecProFork = &types.Genesis{ | |
| Config: configProFork, | |
| Difficulty: big.NewInt(1), | |
| GasLimit: 5000, | |
| } |
No description provided.