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

trie: fix del stack operation key formatting #3378

Merged
merged 9 commits into from
Apr 28, 2024
Merged

Conversation

gabrocheleau
Copy link
Contributor

@gabrocheleau gabrocheleau commented Apr 27, 2024

This PR addresses #3333.

This uncovered an issue where we were typecasting EmbeddedNodes as Uint8Array (whereas they could also be Uint8Array[]) before trie/database operations. This resulted in issues when the node was < 32 bytes, as in MPTs, nodes are not serialized when they can directly fit in < 32 bytes. This only occurs in the context of non-hashed tries (as hashing normalizes everything to 32 bytes anyways), so I assume this is why this did not get caught.

Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

LGTM, left one small question regarding the linter.

}
lastRoot = this._formatNode(node, stack.length === 0, opStack) as Uint8Array
}

if (lastRoot) {
if (lastRoot !== undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, interesting, why did this not get caught by our linter before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure, this linting sometimes behaves in ways that I don't understand, for e.g. by flagging straight booleans (e.g. isActivatedEIP) and not flagging instances like this.

@gabrocheleau gabrocheleau merged commit c4a9f00 into master Apr 28, 2024
33 of 34 checks passed
@holgerd77 holgerd77 deleted the trie/issue-3333 branch April 29, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants