-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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
trie: iterate values pre-order and fix seek behavior #27838
Conversation
i think it's a valid fix, could you please add more relevant unit tests? |
c41b8d4
to
149951b
Compare
In order to properly change the order of traversal, I added a fix that will jump to the full node's leaf first, then back to iterate its child nodes normally, and skip the leaf at the end. It's a little funky, but seems to work fine. I updated the unit tests to reflect the new ordering, which should sufficiently cover the behavior change. I also added an explicit case to check that seeking does not produce a leaf whose key prefixes the target key. |
Is there anything else this needs? |
Wrt the path comparison, I think for ethereum state trie, this issue will never occur because of the same key length. |
Right, I tried to acknowledge that in the issue (#27837 (comment)) - it won't show up in normal Geth operations, but it creates this inconsistency between a pre-order traversal of nodes vs. post-order traversal of values. I think it's at least worth documenting if not fixing. |
59c555e
to
bbc0c64
Compare
This pull request fixes the pre-order trie traversal by defining a more accurate iterator order and path comparison rule. Co-authored-by: Gary Rong <garyrong0905@gmail.com>
This pull request fixes the pre-order trie traversal by defining a more accurate iterator order and path comparison rule. Co-authored-by: Gary Rong <garyrong0905@gmail.com>
Proposed fix for #27837