refactor(all): move genesis initialization to blockchain #25523#2018
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 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
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
There was a problem hiding this comment.
Pull request overview
Moves genesis initialization responsibilities into core.BlockChain (instead of callers), updating related APIs and tests accordingly.
Changes:
- Updated
core.NewBlockChain/core.NewBlockChainExto accept a*core.Genesisand runSetupGenesisBlockinternally. - Refactored
core.Genesiscreation/commit paths (newToBlock()behavior, persisted genesis state spec) and adjusted call sites. - Updated many tests/helpers/CLIs to pass genesis specs (vs chain config) and to use
MustCommitwhere needed.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/state_test_util.go | Updates genesis block creation to new ToBlock() signature. |
| tests/block_test_util.go | Passes genesis spec to NewBlockChain instead of chain config. |
| params/config.go | Comment tweak for MainnetChainConfig. |
| eth/tracers/api_test.go | Updates test chain creation to use genesis spec in NewBlockChain. |
| eth/helper_test.go | Updates test blockchain creation to pass genesis spec. |
| eth/handler_test.go | Refactors DAO challenge test setup to use genesis spec for blockchain init. |
| eth/gasprice/gasprice_test.go | Uses MustCommit and passes genesis spec to NewBlockChain. |
| eth/filters/filter_test.go | Refactors genesis setup/commit and NewBlockChain calls for new API. |
| eth/filters/filter_system_test.go | Updates blockchain creation to pass genesis spec. |
| eth/fetcher/fetcher_test.go | Replaces GenesisBlockForTesting usage with explicit genesis spec + commit. |
| eth/downloader/testchain_test.go | Replaces GenesisBlockForTesting with explicit genesis spec + commit; adjusts init. |
| eth/downloader/queue_test.go | Removes local genesis DB setup and reuses shared test genesis/DB. |
| eth/backend.go | Switches to LoadChainConfig before blockchain creation; passes genesis spec to NewBlockChainEx. |
| core/state_processor_test.go | Updates test blockchain initialization to pass genesis spec. |
| core/rawdb/accessors_metadata.go | Changes ReadChainConfig to return *ChainConfig without error; adds WriteGenesisStateSpec. |
| core/genesis_test.go | Updates genesis ToBlock() calls and blockchain init signature in tests. |
| core/genesis.go | Introduces alloc hashing/flushing helpers, adds LoadChainConfig, changes ToBlock/Commit flow, persists genesis alloc spec. |
| core/dao_test.go | Refactors DAO fork tests to build blockchains from genesis specs. |
| core/chain_makers_test.go | Updates example to pass genesis spec to NewBlockChain. |
| core/chain_makers.go | Updates GenerateChainWithGenesis to use new ToBlock() signature. |
| core/blockchain_test.go | Updates all test blockchain construction paths to pass genesis specs. |
| core/blockchain.go | Changes NewBlockChain/NewBlockChainEx signatures; moves genesis setup + config logging inside blockchain init. |
| core/block_validator_test.go | Updates test chain generation and blockchain init for new API. |
| core/bench_test.go | Updates benchmarks to pass genesis spec; adapts bench DB setup for chain config storage. |
| cmd/utils/flags.go | Updates MakeChain to use LoadChainConfig and passes genesis spec to NewBlockChain. |
| cmd/evm/runner.go | Switches genesis handling from ToBlock(db) to MustCommit(db) for state initialization. |
| cmd/XDC/dao_test.go | Updates for ReadChainConfig signature change (nil-check instead of error). |
| accounts/abi/bind/backends/simulated.go | Updates simulated backend creation to pass genesis spec to NewBlockChain. |
Comments suppressed due to low confidence (1)
cmd/utils/flags.go:1775
- Same config divergence risk as in
eth/backend.go:MakeChainbuilds the consensus engine fromcore.LoadChainConfig, butcore.NewBlockChainwill runSetupGenesisBlockand may apply a different (upgraded) config. This can leave the engine using a stale config while the blockchain uses the updated one. Prefer obtaining the effective config viaSetupGenesisBlock(or equivalent) before constructing the engine.
func MakeChain(ctx *cli.Context, stack *node.Node, readonly bool) (*core.BlockChain, ethdb.Database) {
var (
gspec = MakeGenesis(ctx)
chainDb = MakeChainDatabase(ctx, stack, readonly)
)
config, _, err := core.LoadChainConfig(chainDb, gspec)
if err != nil {
Fatalf("%v", err)
}
var engine consensus.Engine
if config.XDPoS != nil {
engine = XDPoS.New(config, chainDb)
} else {
Fatalf("Only support XDPoS consensus")
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| proDb := rawdb.NewMemoryDatabase() | ||
| gspec.MustCommit(proDb) | ||
| proConf := *params.TestChainConfig | ||
| proConf.DAOForkBlock = forkBlock | ||
| proConf.DAOForkSupport = true | ||
| proBc, _ := NewBlockChain(proDb, nil, &proConf, ethash.NewFaker(), vm.Config{}) | ||
| progspec := &Genesis{ | ||
| BaseFee: big.NewInt(params.InitialBaseFee), | ||
| Config: &proConf, | ||
| } | ||
| gspec.MustCommit(proDb) | ||
|
|
||
| proBc, _ := NewBlockChain(proDb, nil, progspec, ethash.NewFaker(), vm.Config{}) |
There was a problem hiding this comment.
In this test setup, the genesis is committed to proDb using gspec (the non-fork config) and it’s done twice. This is redundant (since NewBlockChain(..., progspec, ...) will run SetupGenesisBlock anyway) and can accidentally exercise config-compat upgrade paths instead of starting the DB with the intended pro-fork proConf. Prefer removing these commits, or commit progspec once if you want the DB pre-seeded.
| conDb := rawdb.NewMemoryDatabase() | ||
| gspec.MustCommit(conDb) | ||
| conConf := *params.TestChainConfig | ||
| conConf.DAOForkBlock = forkBlock | ||
| conConf.DAOForkSupport = false | ||
| conBc, _ := NewBlockChain(conDb, nil, &conConf, ethash.NewFaker(), vm.Config{}) | ||
| congspec := &Genesis{ | ||
| BaseFee: big.NewInt(params.InitialBaseFee), | ||
| Config: &conConf, | ||
| } | ||
| gspec.MustCommit(conDb) | ||
|
|
||
| conBc, _ := NewBlockChain(conDb, nil, congspec, ethash.NewFaker(), vm.Config{}) |
There was a problem hiding this comment.
Same issue as the pro-fork DB: conDb is pre-seeded by committing gspec twice (non-fork config) even though NewBlockChain(..., congspec, ...) will initialize genesis. This is redundant and can mask mistakes by turning this into a config upgrade scenario. Remove these commits or commit congspec once if needed.
| return nil, genesisErr | ||
| // Here we determine genesis hash and active ChainConfig. | ||
| // We need these to figure out the consensus parameters and to set up history pruning. | ||
| chainConfig, _, err := core.LoadChainConfig(chainDb, config.Genesis) |
There was a problem hiding this comment.
core.LoadChainConfig returns the stored chain config without running the same compatibility/upgrade logic as SetupGenesisBlock. Since core.NewBlockChainEx will still call SetupGenesisBlock internally and may update/rewind to a different config, eth.chainConfig (and the consensus engine created from it) can diverge from eth.blockchain.Config(). For XDPoS this is dangerous because the engine is initialized with the pre-SetupGenesisBlock config. Consider calling SetupGenesisBlock here (as before), or enhancing LoadChainConfig to return the effective config that SetupGenesisBlock would apply.
| chainConfig, _, err := core.LoadChainConfig(chainDb, config.Genesis) | |
| chainConfig, _, _, err := core.SetupGenesisBlock(chainDb, config.Genesis) |
gzliudan
force-pushed
the
move-genesis-state-init
branch
from
2ca266d to
3893a4f
Compare
gzliudan
changed the title
gzliudan
force-pushed
the
move-genesis-state-init
branch
from
3893a4f to
5753e89
Compare
gzliudan
force-pushed
the
move-genesis-state-init
branch
from
5753e89 to
1984320
Compare
Proposed changes
This PR is the minimal version for moving genesis initialization logic into core.Blockchain.
Rerf: ethereum#25523
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
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