-
Couldn't load subscription status.
- Fork 114
chore(l2): refactor get_embedded_node and remove hash cloning
#5001
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: main
Are you sure you want to change the base?
Conversation
Lines of code reportTotal lines added: Detailed view |
Benchmark for ad8c4f4Click to view benchmark
|
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.
Pull Request Overview
This PR refactors trie decoding to eliminate hash cloning and simplify the codebase. The changes optimize stateless execution in zkVM by introducing new methods for efficient hash computation and storage, resulting in a 22% performance improvement in Trie::from_nodes.
Key changes:
- Added
NodeHash::finalize_mutto compute and store finalized hashes in-place, avoiding repeated hashing and clones - Refactored the nested
get_embedded_nodefunction intoNodeRef::resolve_subtriefor better code organization - Introduced
NodeRef::compute_hash_finalizedto obtain hash references without cloning
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/common/trie/trie.rs | Simplified Trie::from_nodes by replacing nested function with resolve_subtrie method |
| crates/common/trie/node_hash.rs | Added finalize_mut method to compute and cache finalized hash in-place |
| crates/common/trie/node.rs | Added compute_hash_finalized and resolve_subtrie methods to support refactored trie construction |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Benchmark for 4bcaa8dClick to view benchmark
|
Benchmark for b5b710fClick to view benchmark
|
|
|
||
| /// Returns a reference to the finalized hash, hashing `self` if it's `NodeHash::Inline`, and storing the result by | ||
| /// changing it to the `NodeHash::Hashed` variant. | ||
| pub fn finalize_mut(&mut self) -> &H256 { |
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 leaves the tree in an inconsistent state, right? If any function expects compute_hash to return the node's MPT hash, they'll receive an incorrect result. We should document that in a big warning message to avoid weird errors going forward.
Motivation
Optimizes trie decoding in zkVM stateless execution by adding
NodeHash::finalize_mutandNodeRef::compute_hash_finalizedto calculate a node's finalized hash, store it and return a reference to it, instead of cloning the hash or even hashing each time if it's an inline node.Also refactors the
get_embedded_nodefunction intoNodeRef::resolve_subtrieto remove the nested function and simplify the code.Flamegraphs
After all other trie zkVM optimizations,
from_nodesis one of the most expensive calls in block execution. This PR reducesTrie::from_nodesfrom 131k to 101k, 22%.before:


after: