Skip to content

Conversation

@Rjected
Copy link
Member

@Rjected Rjected commented Mar 19, 2025

This splits out the branch node insertion logic in reveal_node, into a dedicated function. Also adds some more docs related to reveal_node_or_hash

@Rjected Rjected added the A-trie Related to Merkle Patricia Trie implementation label Mar 19, 2025
@rkrasiuk
Copy link
Member

@Rjected are we planning to reuse this function anywhere? otherwise, seems like a minor misdirection

@Rjected
Copy link
Member Author

Rjected commented Mar 20, 2025

@Rjected are we planning to reuse this function anywhere? otherwise, seems like a minor misdirection

No, maybe skill issue but when reading the code previously it was not immediately clear to me that this is what the match was doing. As well as the branches for the other node types. Open to just adding docs, but I think if the match in reveal_node looked like the following:

        match node {
            TrieNode::EmptyRoot => {
                debug_assert!(path.is_empty());
                self.nodes.insert(path, SparseNode::Empty);
            }
            TrieNode::Branch(branch) => {
                // first few lines
                self.insert_branch_node(branch, path, masks)?;
            }
            TrieNode::Extension(ext) => {
                self.insert_extension_node(ext, path, masks)?;
            }
            TrieNode::Leaf(leaf) => {
                self.insert_leaf_node(leaf, path, masks)?;
            }
        }

it would have been easier to read / understand

@jenpaff jenpaff moved this from Todo to In Review in Reth Tracker Apr 9, 2025
@jenpaff jenpaff added the C-perf A change motivated by improving speed, memory usage or disk footprint label Apr 22, 2025
@shekhirin shekhirin closed this Apr 23, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in Reth Tracker Apr 23, 2025
@jenpaff jenpaff moved this from Done to Out of Scope in Reth Tracker Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-trie Related to Merkle Patricia Trie implementation C-perf A change motivated by improving speed, memory usage or disk footprint

Projects

Status: Out of Scope

Development

Successfully merging this pull request may close these issues.

5 participants