Requested changes for PR #6

eth_trie_utils/src/debug_tools/common.rs

@@ -60,17 +60,27 @@ pub(super) fn get_segment_from_node_and_key_piece<T: PartialTrie>(
     }
 }
 
-pub(super) fn get_key_piece_from_node<T: PartialTrie>(n: &Node<T>, curr_key: &Nibbles) -> Nibbles {
+/// Get the key piece from the given node if applicable.
+///
+/// Note that there is no specific [`Nibble`] associated with a branch like
+/// there are [`Nibbles`] with [Extension][`Node::Extension`] and
+/// [Leaf][`Node::Leaf`] nodes, and the only way to get the `Nibble`
+/// "associated" with a branch is to look at the next `Nibble` in the current
+/// key as we traverse down it.
+pub(super) fn get_key_piece_from_node_pulling_from_key_for_branches<T: PartialTrie>(
+    n: &Node<T>,
+    curr_key: &Nibbles,
+) -> Nibbles {
     match n {
         Node::Empty | Node::Hash(_) => Nibbles::default(),
         Node::Branch { .. } => curr_key.get_next_nibbles(1),
         Node::Extension { nibbles, child: _ } | Node::Leaf { nibbles, value: _ } => *nibbles,
     }
 }
 
-// It might seem a bit weird to say a branch has no key piece, but this function
-// is used to detect two nodes of the same type that have different keys.
-pub(super) fn get_key_piece_from_node_no_branch_key<T: PartialTrie>(n: &Node<T>) -> Nibbles {
+/// Get the key piece from the given node if applicable. Note that
+/// [branch][`Node::Branch`]s have no [`Nibble`] directly associated with them.
+pub(super) fn get_key_piece_from_node<T: PartialTrie>(n: &Node<T>) -> Nibbles {
     match n {
         Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(),
         Node::Extension { nibbles, child: _ } | Node::Leaf { nibbles, value: _ } => *nibbles,

eth_trie_utils/src/debug_tools/diff.rs

@@ -30,9 +30,7 @@ use std::{fmt::Display, ops::Deref};
 
 use ethereum_types::H256;
 
-use super::common::{
-    get_key_piece_from_node_no_branch_key, get_segment_from_node_and_key_piece, NodePath,
-};
+use super::common::{get_key_piece_from_node, get_segment_from_node_and_key_piece, NodePath};
 use crate::{
     nibbles::Nibbles,
     partial_trie::{HashedPartialTrie, Node, PartialTrie},
@@ -97,8 +95,8 @@ impl DiffPoint {
         parent_k: Nibbles,
         path: NodePath,
     ) -> Self {
-        let a_key = parent_k.merge_nibbles(&get_key_piece_from_node_no_branch_key(child_a));
-        let b_key = parent_k.merge_nibbles(&get_key_piece_from_node_no_branch_key(child_b));
+        let a_key = parent_k.merge_nibbles(&get_key_piece_from_node(child_a));
+        let b_key = parent_k.merge_nibbles(&get_key_piece_from_node(child_b));
 
         DiffPoint {
             depth: 0,
@@ -298,8 +296,8 @@ fn find_diff_point_where_tries_begin_to_diff_depth_rec(
     let a_type: TrieNodeType = state.a.deref().into();
     let b_type: TrieNodeType = state.b.deref().into();
 
-    let a_key_piece = get_key_piece_from_node_no_branch_key(state.a);
-    let b_key_piece = get_key_piece_from_node_no_branch_key(state.b);
+    let a_key_piece = get_key_piece_from_node(state.a);
+    let b_key_piece = get_key_piece_from_node(state.b);
 
     // Note that differences in a node's `value` will be picked up by a hash
     // mismatch.

eth_trie_utils/src/debug_tools/query.rs

@@ -6,18 +6,30 @@ use std::fmt::{self, Display};
 use ethereum_types::H256;
 
 use super::common::{
-    get_key_piece_from_node, get_segment_from_node_and_key_piece, NodePath, PathSegment,
+    get_key_piece_from_node_pulling_from_key_for_branches, get_segment_from_node_and_key_piece,
+    NodePath, PathSegment,
 };
 use crate::{
     nibbles::Nibbles,
     partial_trie::{Node, PartialTrie, WrappedNode},
 };
 
 /// Params controlling how much information is reported in the query output.
+///
+/// By default, the node type along with its key piece is printed out per node
+/// (eg. "Leaf(0x1234)"). Additional node specific information can be printed
+/// out by enabling `include_node_specific_values`.
 #[derive(Clone, Debug)]
 pub struct DebugQueryParams {
+    /// Include (if applicable) the piece of the key that is contained by the
+    /// node (eg. ("0x1234")).
     include_key_piece_per_node: bool,
+
+    /// Include the type of node (eg "Branch").
     include_node_type: bool,
+
+    /// Include additional data that is specific to the node type (eg. The mask
+    /// of a `Branch` or the hash of a `Hash` node).
     include_node_specific_values: bool,
 }
 
@@ -79,6 +91,8 @@ impl From<Nibbles> for DebugQuery {
     }
 }
 
+/// Extra data that is associated with a node. Only used if
+/// `include_node_specific_values` is `true`.
 #[derive(Clone, Debug)]
 enum ExtraNodeSegmentInfo {
     Hash(H256),
@@ -119,10 +133,8 @@ impl ExtraNodeSegmentInfo {
 fn create_child_mask_from_children<T: PartialTrie>(children: &[WrappedNode<T>; 16]) -> u16 {
     let mut mask: u16 = 0;
 
-    // I think the clippy lint actually makes it a lot less readable in this case.
-    #[allow(clippy::needless_range_loop)]
-    for i in 0..16 {
-        if !matches!(children[i].as_ref(), Node::Empty) {
+    for (i, child) in children.iter().enumerate().take(16) {
+        if !matches!(child.as_ref(), Node::Empty) {
             mask |= (1 << i) as u16;
         }
     }
@@ -255,7 +267,7 @@ fn get_path_from_query_rec<T: PartialTrie>(
     curr_key: &mut Nibbles,
     query_out: &mut DebugQueryOutput,
 ) {
-    let key_piece = get_key_piece_from_node(node, curr_key);
+    let key_piece = get_key_piece_from_node_pulling_from_key_for_branches(node, curr_key);
     let seg = get_segment_from_node_and_key_piece(node, &key_piece);
 
     query_out.node_path.append(seg);
@@ -271,12 +283,12 @@ fn get_path_from_query_rec<T: PartialTrie>(
             get_path_from_query_rec(&children[nib as usize], curr_key, query_out)
         }
         Node::Extension { nibbles, child } => {
-            get_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles);
+            get_next_nibbles_from_node_key_clamped(curr_key, nibbles.count);
             get_path_from_query_rec(child, curr_key, query_out);
         }
         Node::Leaf { nibbles, value: _ } => {
             let curr_key_next_nibs =
-                get_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles);
+                get_next_nibbles_from_node_key_clamped(curr_key, nibbles.count);
 
             if *nibbles == curr_key_next_nibs {
                 curr_key.pop_nibbles_front(curr_key_next_nibs.count);
@@ -289,12 +301,13 @@ fn get_path_from_query_rec<T: PartialTrie>(
     }
 }
 
-/// If the key lands in the middle of a leaf/extension, then we will assume that
-/// this is a node hit even though it's not precise.
-fn get_nibbles_from_node_key_and_avoid_underflow(
-    curr_key: &mut Nibbles,
-    node_key: &Nibbles,
-) -> Nibbles {
-    let num_nibs_to_get = node_key.count.min(curr_key.count);
-    curr_key.get_next_nibbles(num_nibs_to_get)
+/// Gets the next `n` [`Nibbles`] from the key and clamps it in the case of it
+/// going out of range.
+fn get_next_nibbles_from_node_key_clamped(key: &Nibbles, n_nibs: usize) -> Nibbles {
+    let num_nibs_to_get = n_nibs.min(key.count);
+    key.get_next_nibbles(num_nibs_to_get)
 }
+
+// TODO: Create some simple tests...
+#[cfg(test)]
+mod tests {}

eth_trie_utils/src/nibbles.rs

@@ -607,8 +607,6 @@ impl Nibbles {
         let hex_string_raw = hex_encode_f(&byte_buf[(64 - count_bytes)..64]);
         let hex_char_iter_raw = hex_string_raw.chars();
 
-        // We need this skip to make both match arms have the same type.
-        #[allow(clippy::iter_skip_zero)]
         let mut hex_string = String::from("0x");
         match is_even(self.count) {
             false => hex_string.extend(hex_char_iter_raw.skip(1)),

eth_trie_utils/src/utils.rs

@@ -35,13 +35,15 @@ impl<N: PartialTrie> From<&Node<N>> for TrieNodeType {
 
 impl Display for TrieNodeType {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            TrieNodeType::Empty => write!(f, "Empty"),
-            TrieNodeType::Hash => write!(f, "Hash"),
-            TrieNodeType::Branch => write!(f, "Branch"),
-            TrieNodeType::Extension => write!(f, "Extension"),
-            TrieNodeType::Leaf => write!(f, "Leaf"),
-        }
+        let s = match self {
+            TrieNodeType::Empty => "Empty",
+            TrieNodeType::Hash => "Hash",
+            TrieNodeType::Branch => "Branch",
+            TrieNodeType::Extension => "Extension",
+            TrieNodeType::Leaf => "Leaf",
+        };
+
+        write!(f, "{}", s)
     }
 }