-
Notifications
You must be signed in to change notification settings - Fork 20.9k
core: implement state database #32084
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: master
Are you sure you want to change the base?
Conversation
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.
I left a few questions, chief of all how you expect the transition pointers to be maintained, how OpenStorageTrie
should be handled.
Once this is clear, I can try to port the transition on top of that and see what breaks.
} | ||
|
||
// stateDB returns the appropriate state database based on the given flag. | ||
func (s *stateDatabase) stateDB(isVerkle bool) *state.CachingDB { |
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.
that should really have another name, we find ourselves calling statedb.stateDB()
which makes a part of the code that has a ton of confusing names even more confusing because now two things are named the same.
Why not keeping caching database as a name?
func (s *stateDatabase) hasState(root common.Hash) bool { | ||
_, err := s.merkle.NodeReader(root) | ||
if err == nil { | ||
return true | ||
} | ||
_, err = s.verkle.NodeReader(root) | ||
return err == nil | ||
} | ||
|
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 should be the opposite, verkle primes over merkle, otherwise the transition won't work.
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.
k, I see that this is just testing whether the state exists, and a same state won't be present in both dbs because they don't have the same hash. I think it's confusing, owing to the transition process, but it makes sense because we don't want to take the slower path until we know it's needed.
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.
exactly
if root == types.EmptyRootHash { | ||
return &reader{db: db}, nil | ||
} |
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.
I don't see why this is needed?
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.
Because the root node of empty merkle trie won't be persisted in the database.
triedb.NodeReader(types.EmptyRootHash) will return nil without this condition.
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.
iiuc, the pointers / TransitionState
should also be managed at this level? But then I still need to have OpenTrie
knowing which epoch I'm at, to figure out which kind of tree needs to be returned.
This pull request introduces a new structure: stateDatabase, acting as the entrypoint of state data
and provides another layer of abstraction between Merkle and Verkle data.