feat(trie): Proof V2: retain proof nodes which match targets #19941
+506
−150
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mediocregopher
commented
Nov 24, 2025
| } | ||
| Self::Branch(branch_node) => Ok(TrieNode::Branch(branch_node)), | ||
| } | ||
| // TODO store trie masks on branch |
mediocregopher
changed the title
mediocregopher
commented
Nov 24, 2025
Comment on lines
+63
to
+64
| /// ever be modified, and therefore all children besides the last are expected to be | ||
| /// [`ProofTrieBranchChild::RlpNode`]s. |
Collaborator
Author
There was a problem hiding this comment.
Given this property, it might make more sense to split this into something like:
child_stack: Vec<RlpNode>
uncommitted_child: Option<ProofTrieBranchChild<VE::DeferredEncoder>>
I'm not sure that this would give us any performance gains or anything, but it might make the code more understandable and easier to validate as correct. Would be better done as a follow-up PR though.
mediocregopher
commented
Nov 24, 2025
Comment on lines
-220
to
-239
| "Stack is missing necessary children" | ||
| ); | ||
| let children = self.child_stack.drain(self.child_stack.len() - num_children..); | ||
|
|
||
| // We will be pushing the branch onto the child stack, which will require its parent | ||
| // extension's short key (if it has a parent extension). Calculate this short key from the | ||
| // `branch_path` prior to modifying the `branch_path`. | ||
| let short_key = trim_nibbles_prefix( | ||
| &self.branch_path, | ||
| self.branch_path.len() - branch.ext_len as usize, | ||
| "Stack is missing necessary children ({num_children:?})" | ||
| ); | ||
|
|
||
| // Update the branch_path. If this branch is the only branch then only its extension needs | ||
| // to be trimmed, otherwise we also need to remove its nibble from its parent. | ||
| let new_path_len = self.branch_path.len() - | ||
| branch.ext_len as usize - | ||
| if self.branch_stack.is_empty() { 0 } else { 1 }; | ||
|
|
||
| debug_assert!(self.branch_path.len() >= new_path_len); | ||
| self.branch_path = self.branch_path.slice_unchecked(0, new_path_len); |
Collaborator
Author
There was a problem hiding this comment.
This logic didn't change, it only got re-ordered in the method a bit. We cannot modify the branch_path until after commit_child has potentially been called on the popped branch, in the case that the branch has an extension node parent.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #19512
This PR gives the new
proof_v2::ProofCalculatorthe ability to retain theProofTrieNodes for all nodes in the trie which match the given proof targets. The legacy proof calculator retains proofs by checking each node against all targets, whereas V2 checks each node against only a single target. See theshould_retainmethod for details on how this works.To support proof retention the idea of "committing" a child is introduced. A child is "committed" to when there can be no further changes to it, such as an extension or a leaf being split to insert a new branch. When a child is committed it is converted to an RlpNode, checked to see if its proof should be retained, and pushed back onto the child stack as a
ProofTrieBranchChild::RlpNode. This is done all as one step in order to best reuse buffers during RLP-encoding.