-
Notifications
You must be signed in to change notification settings - Fork 20.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd, core, miner: rework genesis setup #30907
Conversation
I think we should extract the chain config setup into its own function like I did in my branch: https://github.com/fjl/go-ethereum/tree/genesis-verkle-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few questions / suggestions but LGTM overall.
core/genesis.go
Outdated
} | ||
} | ||
// Get the existing chain configuration. | ||
// Construct the new chain config either from the supplied genesis, or | ||
// the stored chain config if nothing specified. | ||
newcfg := genesis.configOrDefault(stored) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a better name now, because it's no longer returning any "default". Maybe getUpdatedConfig
?
7bccc27
to
2204e0e
Compare
2204e0e
to
d1460c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems OK. I have tested a bit and didn't find any obvious bugs. Maybe the EnableVerkleAtGenesis
field wouldn't have been necessary, since we also have the VerkleTime
in the config, and could compare that to the genesis time. But I see why you did it this way, it makes life a bit easier in BlockChain init phase. And the field is a workaround anyway.
This pull request refactors the genesis setup function, the major changes are highlighted here: **(a) Triedb is opened in verkle mode if `EnableVerkleAtGenesis` is configured in chainConfig or the database has been initialized previously with `EnableVerkleAtGenesis` configured**. A new config field `EnableVerkleAtGenesis` has been added in the chainConfig. This field must be configured with True if Geth wants to initialize the genesis in Verkle mode. In the verkle devnet-7, the verkle transition is activated at genesis. Therefore, the verkle rules should be used since the genesis. In production networks (mainnet and public testnets), verkle activation always occurs after the genesis block. Therefore, this flag is only made for devnet and should be deprecated later. Besides, verkle transition at non-genesis block hasn't been implemented yet, it should be done in the following PRs. **(b) The genesis initialization condition has been simplified** There is a special mode supported by the Geth is that: Geth can be initialized with an existing chain segment, which can fasten the node sync process by retaining the chain freezer folder. Originally, if the triedb is regarded as uninitialized and the genesis block can be found in the chain freezer, the genesis block along with genesis state will be committed. This condition has been simplified to checking the presence of chain config in key-value store. The existence of chain config can represent the genesis has been committed.
This pull request refactors the genesis setup function, the major changes are highlighted here:
(a) Triedb is opened in verkle mode if
EnableVerkleAtGenesis
is configured in chainConfigor the database has been initialized previously with
EnableVerkleAtGenesis
configured.A new config field
EnableVerkleAtGenesis
has been added in the chainConfig. This field mustbe configured with True if Geth wants to initialize the genesis in Verkle mode.
In the verkle devnet-7, the verkle transition is activated at genesis. Therefore, the verkle rules
should be used since the genesis. In production networks (mainnet and public testnets), verkle
activation always occurs after the genesis block. Therefore, this flag is only made for devnet
and should be deprecated later. Besides, verkle transition at non-genesis block hasn't been
implemented yet, it should be done in the following PRs.
(b) The genesis initialization condition has been simplified
There is a special mode supported by the Geth is that: Geth can be initialized with an existing
chain segment, which can fasten the node sync process by retaining the chain freezer folder.
Originally, if the triedb is regarded as uninitialized and the genesis block can be found in the
chain freezer, the genesis block along with genesis state will be committed. This condition
has been simplified to checking the presence of chain config in key-value store. The existence
of chain config can represent the genesis has been committed.