Skip to content
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

core, trie: extend the state reader interface with StorageExists #30218

Closed
wants to merge 2 commits into from

Conversation

rjl493456442
Copy link
Member

This branch is mostly for verkle experiment

Copy link
Member

@gballet gballet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me, I will rebase and see if that works for me. I left a few questions but it's not critical for now.

Comment on lines +298 to +308
// mustCopyTrie returns a deep-copied trie.
func mustCopyTrie(t Trie) Trie {
switch t := t.(type) {
case *trie.StateTrie:
return t.Copy()
case *trie.VerkleTrie:
return t.Copy()
default:
panic(fmt.Errorf("unknown trie type %T", t))
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need this? would it not be simpler to just call t.Copy()?

Comment on lines +130 to +134
// StorageExists implements state.Trie, returning a flag indicating whether the
// requested storage slot is existent or not.
func (t *VerkleTrie) StorageExists(addr common.Address, key []byte) (bool, error) {
k := utils.StorageSlotKeyWithEvaluatedAddress(t.cache.Get(addr.Bytes()), key)
val, err := t.root.Get(k, t.nodeResolver)
if err != nil {
return false, err
}
return len(val) != 0, nil
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it not be faster to directly do t.reader.StorageExists ? because if I call Get I will jump from node to node until I find the target - whereas if they went straight to the right key, we would get this information. Since it can not be deleted, if it's found in the difflayers or the db, it exists.

Comment on lines +130 to +134
// StorageExists implements state.Trie, returning a flag indicating whether the
// requested storage slot is existent or not.
func (t *VerkleTrie) StorageExists(addr common.Address, key []byte) (bool, error) {
k := utils.StorageSlotKeyWithEvaluatedAddress(t.cache.Get(addr.Bytes()), key)
val, err := t.root.Get(k, t.nodeResolver)
if err != nil {
return false, err
}
return len(val) != 0, nil
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it not be faster to directly do t.reader.StorageExists ? because if I call Get I will jump from node to node until I find the target - whereas if they went straight to the right key, we would get this information. Since it can not be deleted, if it's found in the difflayers or the db, it exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants