-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fix iterator from t8n #434
Conversation
func nodeToDBKey(n verkle.VerkleNode) []byte { | ||
ret := n.Commitment().Bytes() | ||
return ret[:] | ||
} | ||
|
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.
this function is no longer in use, as it was still using the "hash-based" storage model
@@ -97,9 +97,9 @@ func (it *verkleNodeIterator) Next(descend bool) bool { | |||
it.current = it.stack[len(it.stack)-1].Node | |||
it.stack[len(it.stack)-1].Index++ | |||
return it.Next(descend) | |||
case *verkle.HashedNode: |
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.
this code was so old that we were still using the version of HashedNode
that was storing commitments (and a potential stem as well)
// resolve the node | ||
data, err := it.trie.db.diskdb.Get(nodeToDBKey(node)) |
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.
(incorrect) remnants of the hash-based period.
@@ -147,7 +147,7 @@ func (it *verkleNodeIterator) Path() []byte { | |||
var path []byte | |||
for i, state := range it.stack { | |||
// skip the last byte | |||
if i <= len(it.stack)-1 { |
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.
this would always return an empty path
@@ -112,7 +112,7 @@ func (it *verkleNodeIterator) Next(descend bool) bool { | |||
it.stack[len(it.stack)-1].Node = it.current | |||
parent := &it.stack[len(it.stack)-2] | |||
parent.Node.(*verkle.InternalNode).SetChild(parent.Index, it.current) | |||
return 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.
if a node was resolved, the function needs to be called recursively so that the iteration covers the child nodes
* various verkle iterator fixes * remove unused nodeToDBKey
* various verkle iterator fixes * remove unused nodeToDBKey
* various verkle iterator fixes * remove unused nodeToDBKey
* various verkle iterator fixes * remove unused nodeToDBKey
While working on a verkle-enabled
evm t8n
, I found out that the iterator was broken. This PR merges the fixes back intokaustinen-with-shapella
so that I can keep the t8n PR minimal.