-
Couldn't load subscription status.
- Fork 14
core/rawdb, core/state: polish the transition state implementation #544
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
base: rebase-master-add-transition-state
Are you sure you want to change the base?
core/rawdb, core/state: polish the transition state implementation #544
Conversation
consensus/beacon/consensus.go
Outdated
| // This step needs to happen as late as possible to catch all access events. | ||
| if chain.Config().IsVerkle(header.Number, header.Time) { | ||
| // TODO(gballet) move this to the end of the overlay conversion function in a subsequent PR | ||
| statedb.Database().(*state.CachingDB).SaveTransitionState(header.Root, &overlay.TransitionState{Ended: true}) |
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.
&overlay.TransitionState{Ended: true}) is apparently wrong here. Let's remove it for now.
|
|
||
| func GenerateVerkleChainWithGenesis(genesis *Genesis, engine consensus.Engine, n int, gen func(int, *BlockGen)) (common.Hash, ethdb.Database, []*types.Block, []types.Receipts, []*verkle.VerkleProof, []verkle.StateDiff) { | ||
| db := rawdb.NewMemoryDatabase() | ||
| saveVerkleTransitionStatusAtVerlkeGenesis(db) |
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.
Duplicated with the one in genesis.Commit(db, triedb). It's also not correct to persist a transition-state with zero root.
| emptyRoot = types.EmptyVerkleHash | ||
| } | ||
| db := rawdb.NewMemoryDatabase() | ||
| if isVerkle { |
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.
It's unnecessary to persist any transition flag, as this function is supposed to calculate the state root of genesis only.
| if genesis != nil && genesis.Config == nil { | ||
| return nil, common.Hash{}, nil, errGenesisNoConfig | ||
| } | ||
| // In case of verkle-at-genesis, we need to ensure that the conversion |
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.
Duplicated with the one in genesis.Commit(db, triedb). It's also not correct to persist a transition-state with zero root.
c38f272 to
c0578c1
Compare
e3f7af8 to
86183a8
Compare
In this pull request, several changes have been made:
(a) Replaced
CurrentAccountAddress *common.AddresswithCurrentAccountHash common.HashManaging a nullable pointer adds unnecessary complexity. This change simplifies the
logic by replacing the pointer with a value based flag:
CurrentAccountAddress == nilindicated that no account had been migrated yet.CurrentAccountHash == common.Hash{}serves the same purpose.This approach is already used in
CurrentSlotHash, so this change aligns the handling of both fields.(b) Removed
LoadTransitionStatecall sites from the trie readerThe current usage of
LoadTransitionStatein the trie reader is not correct. We should considerfive different scenarios:
However, in your implementation, e.g., in the code below, it's wrong. As you mentioned,
the migration is only started if the base merkle state if finalized.
!ts.InTransition() && !ts.Transitioned()can refer to the situation that Verkle has been activated but the migration is not. In this
case, Overlay tree should be used instead of Merkle here. We should separate this part
of logic into a following PR.