From 5496b5d7550e75fa311e7a020726473fc4da92a4 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 17:57:32 -0700 Subject: [PATCH 01/20] Created some simple debug tools --- src/debug_tools.rs | 400 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/utils.rs | 2 +- 3 files changed, 402 insertions(+), 1 deletion(-) create mode 100644 src/debug_tools.rs diff --git a/src/debug_tools.rs b/src/debug_tools.rs new file mode 100644 index 0000000..199d2d2 --- /dev/null +++ b/src/debug_tools.rs @@ -0,0 +1,400 @@ +use std::fmt::Debug; +use std::{fmt::Display, ops::Deref}; + +use ethereum_types::H256; +use serde::de::value; + +use crate::nibbles::Nibble; +use crate::{ + nibbles::Nibbles, + partial_trie::{HashedPartialTrie, Node, PartialTrie}, + utils::TrieNodeType, +}; + +#[derive(Debug)] +pub struct TrieDiff { + latest_diff_res: Option, + // TODO: Later add a second pass for finding diffs from the bottom up (`earlist_diff_res`). +} + +#[derive(Copy, Clone, Debug)] +enum DiffDetectionState { + NodeTypesDiffer, // Also implies that hashes differ. + HashDiffDetected, + NoDiffDetected, +} + +impl DiffDetectionState { + fn pick_most_significant_state(&self, other: &Self) -> Self { + match self.get_int_repr() > other.get_int_repr() { + false => *other, + true => *self, + } + } + + /// The integer representation also indicates the more "significant" state. + fn get_int_repr(&self) -> usize { + match self { + DiffDetectionState::NodeTypesDiffer => 2, + DiffDetectionState::HashDiffDetected => 1, + DiffDetectionState::NoDiffDetected => 0, + } + } +} + +#[derive(Debug)] +pub struct DiffPoint { + depth: usize, + path: NodePath, + key: Nibbles, + a_info: NodeInfo, + b_info: NodeInfo, +} + +impl DiffPoint { + fn new(child_a: &HashedPartialTrie, child_b: &HashedPartialTrie, parent_k: Nibbles) -> Self { + 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: todo!(), + path: todo!(), + key: parent_k, + a_info: NodeInfo::new(child_a, a_key, get_value_from_node(child_a).cloned()), + b_info: NodeInfo::new(child_b, b_key, get_value_from_node(child_b).cloned()), + } + } +} + +#[derive(Debug)] +struct NodePath { + nodes: Vec<(TrieNodeType, Nibbles)>, +} + +impl NodePath { + fn append(&mut self, n: &Node) { + todo!() + } +} + +impl Display for NodePath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } +} + +#[derive(Clone, Debug)] +pub struct NodeInfo { + key: Nibbles, + + /// The direct value associated with the node (only applicable to `Leaf` & + /// `Branch` nodes). + value: Option>, + node_type: TrieNodeType, + hash: H256, +} + +impl NodeInfo { + fn new(n: &HashedPartialTrie, key: Nibbles, value: Option>) -> Self { + Self { + key, + value, + node_type: n.deref().into(), + hash: n.hash(), + } + } +} + +#[derive(Debug)] +enum DiffType { + NodeType, + Hash, +} + +impl Display for DiffType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DiffType::NodeType => write!(f, "node type"), + DiffType::Hash => write!(f, "hash"), + } + } +} + +pub fn create_diff_between_tries(a: &HashedPartialTrie, b: &HashedPartialTrie) -> TrieDiff { + TrieDiff { + latest_diff_res: find_latest_diff_point_where_tries_begin_to_diff(a, b), + } +} + +// Only support `HashedPartialTrie` due to it being significantly faster to +// detect differences due to hash caching. +fn find_latest_diff_point_where_tries_begin_to_diff( + a: &HashedPartialTrie, + b: &HashedPartialTrie, +) -> Option { + let state = LatestDiffPerCallState::new(a, b, Nibbles::default(), 0); + let mut longest_state = LatestNodeDiffState::default(); + + find_latest_diff_point_where_tries_begin_to_diff_rec(state, &mut longest_state); + + longest_state + .longest_key_node_diff + .or_else(|| longest_state.longest_key_hash_diff) +} + +#[derive(Debug, Default)] +struct LatestNodeDiffState { + longest_key_node_diff: Option, + longest_key_hash_diff: Option, +} + +impl LatestNodeDiffState { + fn try_update_longest_divergence_key_hash(&mut self, state: &LatestDiffPerCallState) { + Self::replace_longest_field_if_our_key_is_larger( + &mut self.longest_key_hash_diff, + &state.curr_key, + state.a, + state.b, + ); + } + + fn try_update_longest_divergence_key_node(&mut self, state: &LatestDiffPerCallState) { + Self::replace_longest_field_if_our_key_is_larger( + &mut self.longest_key_node_diff, + &state.curr_key, + state.a, + state.b, + ); + } + + fn replace_longest_field_if_our_key_is_larger( + field: &mut Option, + parent_k: &Nibbles, + child_a: &HashedPartialTrie, + child_b: &HashedPartialTrie, + ) { + if field + .as_ref() + .map_or(true, |d_point| d_point.key.count < parent_k.count) + { + *field = Some(DiffPoint::new(child_a, child_b, *parent_k)) + } + } +} + +// State that is copied per recursive call. +#[derive(Clone, Debug)] +struct LatestDiffPerCallState<'a> { + a: &'a HashedPartialTrie, + b: &'a HashedPartialTrie, + curr_key: Nibbles, + curr_depth: usize, + + // Horribly inefficient, but these are debug tools, so I think we get a pass. + curr_path: Vec, +} + +#[derive(Clone, Debug)] +enum PathSegment { + Empty, + Hash, + Branch(Nibble), + Extension(Nibbles), + Leaf(Nibbles), +} + +impl<'a> LatestDiffPerCallState<'a> { + /// Exists solely to prevent construction of this type from going over + /// multiple lines. + fn new( + a: &'a HashedPartialTrie, + b: &'a HashedPartialTrie, + curr_key: Nibbles, + curr_depth: usize, + ) -> Self { + Self { + a, + b, + curr_key, + curr_depth, + curr_path: Vec::default(), + } + } + + /// Note: The assumption here is that `a` and `b` are of the same node type + /// and have the key. + fn new_from_parent( + &self, + a: &'a HashedPartialTrie, + b: &'a HashedPartialTrie, + key_piece: &Nibbles, + ) -> Self { + let new_segment = match TrieNodeType::from(a.deref()) { + TrieNodeType::Empty => PathSegment::Empty, + TrieNodeType::Hash => PathSegment::Hash, + TrieNodeType::Branch => { + debug_assert_eq!(key_piece.count, 1); + PathSegment::Branch(key_piece.get_nibble(0)) + } + TrieNodeType::Extension => PathSegment::Extension(*key_piece), + TrieNodeType::Leaf => PathSegment::Leaf(*key_piece), + }; + + let mut new_path = self.curr_path.clone(); + new_path.push(new_segment); + + Self { + a, + b, + curr_key: self.curr_key.merge_nibbles(key_piece), + curr_depth: self.curr_depth + 1, + curr_path: new_path, + } + } +} + +fn find_latest_diff_point_where_tries_begin_to_diff_rec( + state: LatestDiffPerCallState, + longest_state: &mut LatestNodeDiffState, +) -> DiffDetectionState { + 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(&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. + match (a_type, a_key_piece) == (b_type, b_key_piece) { + false => { + longest_state.try_update_longest_divergence_key_node(&state); + DiffDetectionState::NodeTypesDiffer + } + true => { + match (&state.a.node, &state.b.node) { + (Node::Empty, Node::Empty) => DiffDetectionState::NoDiffDetected, + (Node::Hash(a_hash), Node::Hash(b_hash)) => { + create_diff_detection_state_based_from_hashes( + a_hash, + b_hash, + &state.new_from_parent(state.a, state.b, &Nibbles::default()), + longest_state, + ) + } + ( + Node::Branch { + children: a_children, + value: a_value, + }, + Node::Branch { + children: b_children, + value: b_value, + }, + ) => { + let mut most_significant_diff_found = DiffDetectionState::NoDiffDetected; + + for i in 0..16 { + let res = find_latest_diff_point_where_tries_begin_to_diff_rec( + state.new_from_parent( + &a_children[i as usize], + &b_children[i as usize], + &Nibbles::from_nibble(i as u8), + ), + longest_state, + ); + most_significant_diff_found = + most_significant_diff_found.pick_most_significant_state(&res); + } + + match matches!( + most_significant_diff_found, + DiffDetectionState::NoDiffDetected + ) { + false => most_significant_diff_found, + true => { + // Also run a hash check if we haven't picked anything up yet. + create_diff_detection_state_based_from_hash_and_gen_hashes( + &state, + longest_state, + ) + } + } + } + ( + Node::Extension { + nibbles: a_nibs, + child: a_child, + }, + Node::Extension { + nibbles: b_nibs, + child: b_child, + }, + ) => find_latest_diff_point_where_tries_begin_to_diff_rec( + state.new_from_parent(&a_child, &b_child, &a_nibs), + longest_state, + ), + (Node::Leaf { .. }, Node::Leaf { .. }) => { + create_diff_detection_state_based_from_hash_and_gen_hashes( + &state, + longest_state, + ) + } + _ => unreachable!(), + } + } + } +} + +fn create_diff_detection_state_based_from_hash_and_gen_hashes( + state: &LatestDiffPerCallState, + longest_state: &mut LatestNodeDiffState, +) -> DiffDetectionState { + let a_hash = state.a.hash(); + let b_hash = state.b.hash(); + + create_diff_detection_state_based_from_hashes(&a_hash, &b_hash, state, longest_state) +} + +fn create_diff_detection_state_based_from_hashes( + a_hash: &H256, + b_hash: &H256, + state: &LatestDiffPerCallState, + longest_state: &mut LatestNodeDiffState, +) -> DiffDetectionState { + match a_hash == b_hash { + false => { + longest_state.try_update_longest_divergence_key_hash(state); + DiffDetectionState::HashDiffDetected + } + true => DiffDetectionState::NoDiffDetected, + } +} + +fn append_node_key_to_key_buf(curr_key: &Nibbles, n: &Node) -> Nibbles { + match n { + Node::Empty | Node::Hash(_) => *curr_key, + Node::Branch { children, value } => todo!(), + Node::Extension { nibbles, child } => todo!(), + Node::Leaf { nibbles, value } => todo!(), + } +} + +// 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. +fn get_key_piece_from_node(n: &Node) -> Nibbles { + match n { + Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), + Node::Extension { nibbles, child } => *nibbles, + Node::Leaf { nibbles, value } => *nibbles, + } +} + +/// If the node type contains a value (without looking at the children), then +/// return it. +fn get_value_from_node(n: &Node) -> Option<&Vec> { + match n { + Node::Empty | Node::Hash(_) | Node::Extension { .. } => None, + Node::Branch { value, .. } => Some(value), + Node::Leaf { nibbles, value } => Some(value), + } +} diff --git a/src/lib.rs b/src/lib.rs index b7fd861..98a147b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ #![allow(incomplete_features)] +pub mod debug_tools; pub mod nibbles; pub mod partial_trie; mod trie_hashing; diff --git a/src/utils.rs b/src/utils.rs index 4c0ea6b..abc3835 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -5,7 +5,7 @@ use num_traits::PrimInt; use crate::partial_trie::{Node, PartialTrie}; -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] /// Simplified trie node type to make logging cleaner. pub(crate) enum TrieNodeType { Empty, From 984bf1cae63c818b4c5177840a0534e36595ba89 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 21:33:28 -0700 Subject: [PATCH 02/20] Clippy fixes and removed some warnings --- src/nibbles.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nibbles.rs b/src/nibbles.rs index b463feb..730d924 100644 --- a/src/nibbles.rs +++ b/src/nibbles.rs @@ -607,6 +607,8 @@ 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)), From d9a248d266ea9fcda5731b6123e6861b5a934972 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 21:33:59 -0700 Subject: [PATCH 03/20] Filled in some todos and missing methods --- src/debug_tools.rs | 129 +++++++++++++++++++++++++++------------------ 1 file changed, 79 insertions(+), 50 deletions(-) diff --git a/src/debug_tools.rs b/src/debug_tools.rs index 199d2d2..289b8f6 100644 --- a/src/debug_tools.rs +++ b/src/debug_tools.rs @@ -1,8 +1,7 @@ -use std::fmt::Debug; +use std::fmt::{self, Debug}; use std::{fmt::Display, ops::Deref}; use ethereum_types::H256; -use serde::de::value; use crate::nibbles::Nibble; use crate::{ @@ -13,7 +12,7 @@ use crate::{ #[derive(Debug)] pub struct TrieDiff { - latest_diff_res: Option, + pub latest_diff_res: Option, // TODO: Later add a second pass for finding diffs from the bottom up (`earlist_diff_res`). } @@ -42,7 +41,7 @@ impl DiffDetectionState { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct DiffPoint { depth: usize, path: NodePath, @@ -57,8 +56,8 @@ impl DiffPoint { let b_key = parent_k.merge_nibbles(&get_key_piece_from_node(child_b)); DiffPoint { - depth: todo!(), - path: todo!(), + depth: 0, + path: NodePath::default(), key: parent_k, a_info: NodeInfo::new(child_a, a_key, get_value_from_node(child_a).cloned()), b_info: NodeInfo::new(child_b, b_key, get_value_from_node(child_b).cloned()), @@ -66,20 +65,48 @@ impl DiffPoint { } } -#[derive(Debug)] -struct NodePath { - nodes: Vec<(TrieNodeType, Nibbles)>, +impl Display for DiffPoint { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Point Diff {{depth: {}, ", self.depth)?; + write!(f, "Path: ({}), ", self.path)?; + write!(f, "Key: {:x}", self.key)?; + write!(f, "A info: {}", self.a_info)?; + write!(f, "B info: {}", self.b_info) + } } +#[derive(Clone, Debug, Default)] +struct NodePath(Vec); + impl NodePath { - fn append(&mut self, n: &Node) { - todo!() + fn dup_and_append(&self, seg: PathSegment) -> Self { + let mut duped_vec = self.0.clone(); + duped_vec.push(seg); + + Self(duped_vec) + } + + fn write_elem(f: &mut fmt::Formatter<'_>, seg: &PathSegment) -> fmt::Result { + write!(f, "{}", seg) } } impl Display for NodePath { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - todo!() + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let num_elems = self.0.len(); + + // For everything but the last elem. + for seg in self.0.iter().take(num_elems.saturating_sub(1)) { + Self::write_elem(f, seg)?; + write!(f, " --> ")?; + } + + // Avoid the extra `-->` for the last elem. + if let Some(seg) = self.0.last() { + Self::write_elem(f, seg)?; + } + + Ok(()) } } @@ -94,6 +121,21 @@ pub struct NodeInfo { hash: H256, } +impl Display for NodeInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "(key: {:x}", self.key)?; + + write!(f, "value: ")?; + match &self.value { + Some(v) => write!(f, "value: {}, ", hex::encode(v))?, + None => write!(f, "N/A, ")?, + } + + write!(f, "Node type: {}", self.node_type)?; + write!(f, "Trie hash: {:x})", self.hash) + } +} + impl NodeInfo { fn new(n: &HashedPartialTrie, key: Nibbles, value: Option>) -> Self { Self { @@ -105,21 +147,6 @@ impl NodeInfo { } } -#[derive(Debug)] -enum DiffType { - NodeType, - Hash, -} - -impl Display for DiffType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - DiffType::NodeType => write!(f, "node type"), - DiffType::Hash => write!(f, "hash"), - } - } -} - pub fn create_diff_between_tries(a: &HashedPartialTrie, b: &HashedPartialTrie) -> TrieDiff { TrieDiff { latest_diff_res: find_latest_diff_point_where_tries_begin_to_diff(a, b), @@ -139,7 +166,7 @@ fn find_latest_diff_point_where_tries_begin_to_diff( longest_state .longest_key_node_diff - .or_else(|| longest_state.longest_key_hash_diff) + .or(longest_state.longest_key_hash_diff) } #[derive(Debug, Default)] @@ -191,7 +218,7 @@ struct LatestDiffPerCallState<'a> { curr_depth: usize, // Horribly inefficient, but these are debug tools, so I think we get a pass. - curr_path: Vec, + curr_path: NodePath, } #[derive(Clone, Debug)] @@ -203,6 +230,18 @@ enum PathSegment { Leaf(Nibbles), } +impl Display for PathSegment { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + PathSegment::Empty => write!(f, "Empty"), + PathSegment::Hash => write!(f, "Hash"), + PathSegment::Branch(nib) => write!(f, "Branch({})", nib), + PathSegment::Extension(nibs) => write!(f, "Extension({})", nibs), + PathSegment::Leaf(nibs) => write!(f, "Leaf({})", nibs), + } + } +} + impl<'a> LatestDiffPerCallState<'a> { /// Exists solely to prevent construction of this type from going over /// multiple lines. @@ -217,7 +256,7 @@ impl<'a> LatestDiffPerCallState<'a> { b, curr_key, curr_depth, - curr_path: Vec::default(), + curr_path: NodePath::default(), } } @@ -240,8 +279,7 @@ impl<'a> LatestDiffPerCallState<'a> { TrieNodeType::Leaf => PathSegment::Leaf(*key_piece), }; - let mut new_path = self.curr_path.clone(); - new_path.push(new_segment); + let new_path = self.curr_path.dup_and_append(new_segment); Self { a, @@ -260,8 +298,8 @@ fn find_latest_diff_point_where_tries_begin_to_diff_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(&state.a); - let b_key_piece = get_key_piece_from_node(&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. @@ -284,11 +322,11 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( ( Node::Branch { children: a_children, - value: a_value, + value: _a_value, }, Node::Branch { children: b_children, - value: b_value, + value: _b_value, }, ) => { let mut most_significant_diff_found = DiffDetectionState::NoDiffDetected; @@ -326,11 +364,11 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( child: a_child, }, Node::Extension { - nibbles: b_nibs, + nibbles: _b_nibs, child: b_child, }, ) => find_latest_diff_point_where_tries_begin_to_diff_rec( - state.new_from_parent(&a_child, &b_child, &a_nibs), + state.new_from_parent(a_child, b_child, a_nibs), longest_state, ), (Node::Leaf { .. }, Node::Leaf { .. }) => { @@ -370,22 +408,13 @@ fn create_diff_detection_state_based_from_hashes( } } -fn append_node_key_to_key_buf(curr_key: &Nibbles, n: &Node) -> Nibbles { - match n { - Node::Empty | Node::Hash(_) => *curr_key, - Node::Branch { children, value } => todo!(), - Node::Extension { nibbles, child } => todo!(), - Node::Leaf { nibbles, value } => todo!(), - } -} - // 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. fn get_key_piece_from_node(n: &Node) -> Nibbles { match n { Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), - Node::Extension { nibbles, child } => *nibbles, - Node::Leaf { nibbles, value } => *nibbles, + Node::Extension { nibbles, child: _ } => *nibbles, + Node::Leaf { nibbles, value: _ } => *nibbles, } } From c61059eadc13b6d25d32aab7054998b798d69456 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 21:34:24 -0700 Subject: [PATCH 04/20] Planned out test structure --- src/debug_tools.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/debug_tools.rs b/src/debug_tools.rs index 289b8f6..7769111 100644 --- a/src/debug_tools.rs +++ b/src/debug_tools.rs @@ -424,6 +424,57 @@ fn get_value_from_node(n: &Node) -> Option<&Vec> { match n { Node::Empty | Node::Hash(_) | Node::Extension { .. } => None, Node::Branch { value, .. } => Some(value), - Node::Leaf { nibbles, value } => Some(value), + Node::Leaf { nibbles: _, value } => Some(value), + } +} + +#[cfg(test)] +mod tests { + #[test] + #[ignore] + fn latest_single_node_hash_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_single_node_node_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_multi_node_single_node_hash_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_multi_node_single_node_node_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_massive_single_node_diff_tests() { + todo!() + } + + #[test] + #[ignore] + fn latest_multi_node_multi_node_hash_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_multi_node_multi_node_node_diffs_work() { + todo!() + } + + #[test] + #[ignore] + fn latest_massive_multi_node_diff_tests() { + todo!() } } From b27474c91b812e8e6b4af252bb1168484f258386 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 21:46:07 -0700 Subject: [PATCH 05/20] Impled test `latest_single_node_hash_diffs_work` --- src/debug_tools.rs | 65 ++++++++++++++++++++++++++++++++++++---------- src/utils.rs | 2 +- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/debug_tools.rs b/src/debug_tools.rs index 7769111..fac3f9c 100644 --- a/src/debug_tools.rs +++ b/src/debug_tools.rs @@ -10,7 +10,7 @@ use crate::{ utils::TrieNodeType, }; -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] pub struct TrieDiff { pub latest_diff_res: Option, // TODO: Later add a second pass for finding diffs from the bottom up (`earlist_diff_res`). @@ -41,13 +41,13 @@ impl DiffDetectionState { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct DiffPoint { - depth: usize, - path: NodePath, - key: Nibbles, - a_info: NodeInfo, - b_info: NodeInfo, + pub depth: usize, + pub path: NodePath, + pub key: Nibbles, + pub a_info: NodeInfo, + pub b_info: NodeInfo, } impl DiffPoint { @@ -75,8 +75,8 @@ impl Display for DiffPoint { } } -#[derive(Clone, Debug, Default)] -struct NodePath(Vec); +#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] +pub struct NodePath(Vec); impl NodePath { fn dup_and_append(&self, seg: PathSegment) -> Self { @@ -110,7 +110,7 @@ impl Display for NodePath { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct NodeInfo { key: Nibbles, @@ -221,7 +221,7 @@ struct LatestDiffPerCallState<'a> { curr_path: NodePath, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] enum PathSegment { Empty, Hash, @@ -430,10 +430,49 @@ fn get_value_from_node(n: &Node) -> Option<&Vec> { #[cfg(test)] mod tests { + use super::{create_diff_between_tries, DiffPoint, NodeInfo, NodePath}; + use crate::{ + nibbles::Nibbles, + partial_trie::{HashedPartialTrie, PartialTrie}, + utils::TrieNodeType, + }; + #[test] - #[ignore] fn latest_single_node_hash_diffs_work() { - todo!() + // TODO: Reduce duplication once we identify common structures across tests... + let mut a = HashedPartialTrie::default(); + a.insert(0x1234, vec![0]); + let a_hash = a.hash(); + + let mut b = a.clone(); + b.insert(0x1234, vec![1]); + let b_hash = b.hash(); + + let diff = create_diff_between_tries(&a, &b); + + let expected_a = NodeInfo { + key: 0x1234.into(), + value: Some(vec![0]), + node_type: TrieNodeType::Leaf, + hash: a_hash, + }; + + let expected_b = NodeInfo { + key: 0x1234.into(), + value: Some(vec![1]), + node_type: TrieNodeType::Leaf, + hash: b_hash, + }; + + let expected = DiffPoint { + depth: 0, + path: NodePath(vec![]), + key: Nibbles::default(), + a_info: expected_a, + b_info: expected_b, + }; + + assert_eq!(diff.latest_diff_res, Some(expected)); } #[test] diff --git a/src/utils.rs b/src/utils.rs index abc3835..887eca9 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -5,7 +5,7 @@ use num_traits::PrimInt; use crate::partial_trie::{Node, PartialTrie}; -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] /// Simplified trie node type to make logging cleaner. pub(crate) enum TrieNodeType { Empty, From 4db3b10f27f069af7ac8c6eb75f1c756b3287f2f Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 17 Jan 2024 22:03:59 -0700 Subject: [PATCH 06/20] Updated some deps --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5319f09..cc10e5a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ exclude = [ [dependencies] bytes = "1.4.0" -enum-as-inner = "0.5.1" +enum-as-inner = "0.6.0" ethereum-types = "0.14.1" hex = "0.4.3" keccak-hash = "0.10.0" @@ -30,8 +30,8 @@ rlp = "0.5.2" serde = { version = "1.0.160", features = ["derive", "rc"] } [dev-dependencies] -eth_trie = "0.1.0" -pretty_env_logger = "0.4.0" +eth_trie = "0.4.0" +pretty_env_logger = "0.5.0" rand = "0.8.5" rlp-derive = "0.1.0" serde = { version = "1.0.160", features = ["derive"] } From a5242916d0af29ea045cae5bb1439be834611e49 Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 19 Jan 2024 10:02:59 -0700 Subject: [PATCH 07/20] A few fixes & cleanup --- src/debug_tools.rs | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/debug_tools.rs b/src/debug_tools.rs index fac3f9c..4459f97 100644 --- a/src/debug_tools.rs +++ b/src/debug_tools.rs @@ -16,6 +16,16 @@ pub struct TrieDiff { // TODO: Later add a second pass for finding diffs from the bottom up (`earlist_diff_res`). } +impl Display for TrieDiff { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(diff) = &self.latest_diff_res { + write!(f, "{}", diff)?; + } + + Ok(()) + } +} + #[derive(Copy, Clone, Debug)] enum DiffDetectionState { NodeTypesDiffer, // Also implies that hashes differ. @@ -69,9 +79,9 @@ impl Display for DiffPoint { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Point Diff {{depth: {}, ", self.depth)?; write!(f, "Path: ({}), ", self.path)?; - write!(f, "Key: {:x}", self.key)?; - write!(f, "A info: {}", self.a_info)?; - write!(f, "B info: {}", self.b_info) + write!(f, "Key: 0x{:x} ", self.key)?; + write!(f, "A info: {} ", self.a_info)?; + write!(f, "B info: {}}}", self.b_info) } } @@ -123,15 +133,14 @@ pub struct NodeInfo { impl Display for NodeInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "(key: {:x}", self.key)?; + write!(f, "(key: 0x{:x} ", self.key)?; - write!(f, "value: ")?; match &self.value { - Some(v) => write!(f, "value: {}, ", hex::encode(v))?, - None => write!(f, "N/A, ")?, + Some(v) => write!(f, "Value: 0x{}, ", hex::encode(v))?, + None => write!(f, "Value: N/A, ")?, } - write!(f, "Node type: {}", self.node_type)?; + write!(f, "Node type: {} ", self.node_type)?; write!(f, "Trie hash: {:x})", self.hash) } } @@ -295,6 +304,14 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( state: LatestDiffPerCallState, longest_state: &mut LatestNodeDiffState, ) -> DiffDetectionState { + let a_hash = state.a.hash(); + let b_hash = state.b.hash(); + + // We're going to ignore node type differences if they have the same hash (only case I think where this can happen is if one is a hash node?). + if a_hash == b_hash { + return DiffDetectionState::NoDiffDetected; + } + let a_type: TrieNodeType = state.a.deref().into(); let b_type: TrieNodeType = state.b.deref().into(); From 6edfe6ca370fd46b315c2038cab8e45a84a0ac66 Mon Sep 17 00:00:00 2001 From: BGluth Date: Mon, 22 Jan 2024 11:24:15 -0700 Subject: [PATCH 08/20] Moved diff logic into a seperate module - Going to add more debug features that are not related to diffing. --- src/{debug_tools.rs => debug_tools/diff.rs} | 2 +- src/debug_tools/mod.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) rename src/{debug_tools.rs => debug_tools/diff.rs} (99%) create mode 100644 src/debug_tools/mod.rs diff --git a/src/debug_tools.rs b/src/debug_tools/diff.rs similarity index 99% rename from src/debug_tools.rs rename to src/debug_tools/diff.rs index 4459f97..d02ca0b 100644 --- a/src/debug_tools.rs +++ b/src/debug_tools/diff.rs @@ -13,7 +13,7 @@ use crate::{ #[derive(Debug, Eq, PartialEq)] pub struct TrieDiff { pub latest_diff_res: Option, - // TODO: Later add a second pass for finding diffs from the bottom up (`earlist_diff_res`). + // TODO: Later add a second pass for finding diffs from the bottom up (`earliest_diff_res`). } impl Display for TrieDiff { diff --git a/src/debug_tools/mod.rs b/src/debug_tools/mod.rs new file mode 100644 index 0000000..d0e98c5 --- /dev/null +++ b/src/debug_tools/mod.rs @@ -0,0 +1 @@ +pub mod diff; From d6349e547340ad890fc5f086a64e79873c8ff099 Mon Sep 17 00:00:00 2001 From: BGluth Date: Mon, 22 Jan 2024 13:48:16 -0700 Subject: [PATCH 09/20] Impled query debug tools - Also moved stuff around. --- src/debug_tools/common.rs | 104 ++++++++++++++++++++++++ src/debug_tools/diff.rs | 73 +---------------- src/debug_tools/mod.rs | 2 + src/debug_tools/query.rs | 165 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 275 insertions(+), 69 deletions(-) create mode 100644 src/debug_tools/common.rs create mode 100644 src/debug_tools/query.rs diff --git a/src/debug_tools/common.rs b/src/debug_tools/common.rs new file mode 100644 index 0000000..ca3812a --- /dev/null +++ b/src/debug_tools/common.rs @@ -0,0 +1,104 @@ +use std::fmt::{self, Display}; + +use crate::{ + nibbles::{Nibble, Nibbles}, + partial_trie::{Node, PartialTrie}, + utils::TrieNodeType, +}; + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub(super) enum PathSegment { + Empty, + Hash, + Branch(Nibble), + Extension(Nibbles), + Leaf(Nibbles), +} + +impl Display for PathSegment { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + PathSegment::Empty => write!(f, "Empty"), + PathSegment::Hash => write!(f, "Hash"), + PathSegment::Branch(nib) => write!(f, "Branch({})", nib), + PathSegment::Extension(nibs) => write!(f, "Extension({})", nibs), + PathSegment::Leaf(nibs) => write!(f, "Leaf({})", nibs), + } + } +} + +impl PathSegment { + pub(super) fn node_type(&self) -> TrieNodeType { + match self { + PathSegment::Empty => TrieNodeType::Empty, + PathSegment::Hash => TrieNodeType::Hash, + PathSegment::Branch(_) => TrieNodeType::Branch, + PathSegment::Extension(_) => TrieNodeType::Extension, + PathSegment::Leaf(_) => TrieNodeType::Leaf, + } + } + + pub(super) fn get_key_piece_from_seg_if_present(&self) -> Option { + match self { + PathSegment::Empty | PathSegment::Hash => None, + PathSegment::Branch(nib) => Some(Nibbles::from_nibble(*nib)), + PathSegment::Extension(nibs) => Some(*nibs), + PathSegment::Leaf(nibs) => Some(*nibs), + } + } +} + +pub(super) fn get_segment_from_node_and_key_piece( + n: &Node, + k_piece: &Nibbles, +) -> PathSegment { + match TrieNodeType::from(n) { + TrieNodeType::Empty => PathSegment::Empty, + TrieNodeType::Hash => PathSegment::Hash, + TrieNodeType::Branch => { + debug_assert_eq!(k_piece.count, 1); + PathSegment::Branch(k_piece.get_nibble(0)) + } + TrieNodeType::Extension => PathSegment::Extension(*k_piece), + TrieNodeType::Leaf => PathSegment::Leaf(*k_piece), + } +} + +#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] +pub struct NodePath(pub(super) Vec); + +impl NodePath { + pub(super) fn dup_and_append(&self, seg: PathSegment) -> Self { + let mut duped_vec = self.0.clone(); + duped_vec.push(seg); + + Self(duped_vec) + } + + pub(super) fn append(&mut self, seg: PathSegment) { + self.0.push(seg); + } + + fn write_elem(f: &mut fmt::Formatter<'_>, seg: &PathSegment) -> fmt::Result { + write!(f, "{}", seg) + } +} + +impl Display for NodePath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let num_elems = self.0.len(); + + // For everything but the last elem. + for seg in self.0.iter().take(num_elems.saturating_sub(1)) { + Self::write_elem(f, seg)?; + write!(f, " --> ")?; + } + + // Avoid the extra `-->` for the last elem. + if let Some(seg) = self.0.last() { + Self::write_elem(f, seg)?; + } + + Ok(()) + } +} diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index d02ca0b..8deab79 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -3,7 +3,7 @@ use std::{fmt::Display, ops::Deref}; use ethereum_types::H256; -use crate::nibbles::Nibble; +use super::common::{get_segment_from_node_and_key_piece, NodePath}; use crate::{ nibbles::Nibbles, partial_trie::{HashedPartialTrie, Node, PartialTrie}, @@ -85,41 +85,6 @@ impl Display for DiffPoint { } } -#[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] -pub struct NodePath(Vec); - -impl NodePath { - fn dup_and_append(&self, seg: PathSegment) -> Self { - let mut duped_vec = self.0.clone(); - duped_vec.push(seg); - - Self(duped_vec) - } - - fn write_elem(f: &mut fmt::Formatter<'_>, seg: &PathSegment) -> fmt::Result { - write!(f, "{}", seg) - } -} - -impl Display for NodePath { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let num_elems = self.0.len(); - - // For everything but the last elem. - for seg in self.0.iter().take(num_elems.saturating_sub(1)) { - Self::write_elem(f, seg)?; - write!(f, " --> ")?; - } - - // Avoid the extra `-->` for the last elem. - if let Some(seg) = self.0.last() { - Self::write_elem(f, seg)?; - } - - Ok(()) - } -} - #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct NodeInfo { key: Nibbles, @@ -230,27 +195,6 @@ struct LatestDiffPerCallState<'a> { curr_path: NodePath, } -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -enum PathSegment { - Empty, - Hash, - Branch(Nibble), - Extension(Nibbles), - Leaf(Nibbles), -} - -impl Display for PathSegment { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - PathSegment::Empty => write!(f, "Empty"), - PathSegment::Hash => write!(f, "Hash"), - PathSegment::Branch(nib) => write!(f, "Branch({})", nib), - PathSegment::Extension(nibs) => write!(f, "Extension({})", nibs), - PathSegment::Leaf(nibs) => write!(f, "Leaf({})", nibs), - } - } -} - impl<'a> LatestDiffPerCallState<'a> { /// Exists solely to prevent construction of this type from going over /// multiple lines. @@ -277,17 +221,7 @@ impl<'a> LatestDiffPerCallState<'a> { b: &'a HashedPartialTrie, key_piece: &Nibbles, ) -> Self { - let new_segment = match TrieNodeType::from(a.deref()) { - TrieNodeType::Empty => PathSegment::Empty, - TrieNodeType::Hash => PathSegment::Hash, - TrieNodeType::Branch => { - debug_assert_eq!(key_piece.count, 1); - PathSegment::Branch(key_piece.get_nibble(0)) - } - TrieNodeType::Extension => PathSegment::Extension(*key_piece), - TrieNodeType::Leaf => PathSegment::Leaf(*key_piece), - }; - + let new_segment = get_segment_from_node_and_key_piece(a, key_piece); let new_path = self.curr_path.dup_and_append(new_segment); Self { @@ -307,7 +241,8 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( let a_hash = state.a.hash(); let b_hash = state.b.hash(); - // We're going to ignore node type differences if they have the same hash (only case I think where this can happen is if one is a hash node?). + // We're going to ignore node type differences if they have the same hash (only + // case I think where this can happen is if one is a hash node?). if a_hash == b_hash { return DiffDetectionState::NoDiffDetected; } diff --git a/src/debug_tools/mod.rs b/src/debug_tools/mod.rs index d0e98c5..7e49ab1 100644 --- a/src/debug_tools/mod.rs +++ b/src/debug_tools/mod.rs @@ -1 +1,3 @@ +pub mod common; pub mod diff; +pub mod query; diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs new file mode 100644 index 0000000..e5347fd --- /dev/null +++ b/src/debug_tools/query.rs @@ -0,0 +1,165 @@ +use std::fmt::{self, Display}; + +use super::common::{get_segment_from_node_and_key_piece, NodePath, PathSegment}; +use crate::{ + nibbles::Nibbles, + partial_trie::{Node, PartialTrie}, +}; + +#[derive(Clone, Debug)] +pub struct DebugQueryParams { + include_key_piece_per_node: bool, + include_node_type: bool, + // TODO: Look at implementing later... + // include_node_specific_values: bool, +} + +impl Default for DebugQueryParams { + fn default() -> Self { + Self { + include_key_piece_per_node: true, + include_node_type: true, + } + } +} + +#[derive(Debug, Default)] +pub struct DebugQueryParamsBuilder { + params: DebugQueryParams, +} + +impl DebugQueryParamsBuilder { + /// Defaults to `true`. + pub fn print_key_pieces(mut self, enabled: bool) -> Self { + self.params.include_key_piece_per_node = enabled; + self + } + + /// Defaults to `true`. + pub fn print_node_type(mut self, enabled: bool) -> Self { + self.params.include_node_type = enabled; + self + } + + pub fn build(self) -> DebugQueryParams { + self.params + } +} + +#[derive(Debug)] +pub struct DebugQuery { + k: Nibbles, + params: DebugQueryParams, +} + +impl From for DebugQuery { + fn from(_v: Nibbles) -> Self { + todo!() + } +} + +#[derive(Clone, Debug)] +pub struct DebugQueryOutput { + k: Nibbles, + node_path: NodePath, + node_found: bool, + params: DebugQueryParams, +} + +impl Display for DebugQueryOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.fmt_query_header(f)?; + + writeln!(f, "Query path:")?; + for seg in self.node_path.0.iter().take(self.node_path.0.len() - 1) { + Self::fmt_node_based_on_debug_params(f, seg, &self.params)?; + writeln!(f)?; + writeln!(f, "V")?; + } + + if let Some(last_seg) = self.node_path.0.last() { + Self::fmt_node_based_on_debug_params(f, last_seg, &self.params)?; + } + + Ok(()) + } +} + +impl DebugQueryOutput { + fn new(k: Nibbles, params: DebugQueryParams) -> Self { + Self { + k, + node_path: NodePath::default(), + node_found: false, + params, + } + } + + fn fmt_query_header(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "Query Result {{")?; + + writeln!(f, "Queried Key: {}", self.k)?; + writeln!(f, "Node node: {}", self.node_found)?; + + writeln!(f, "}}") + } + + fn fmt_node_based_on_debug_params( + f: &mut fmt::Formatter<'_>, + seg: &PathSegment, + params: &DebugQueryParams, + ) -> fmt::Result { + let node_type = seg.node_type(); + + if params.include_node_type { + write!(f, "{}", node_type)?; + } + + write!(f, "(")?; + + if params.include_key_piece_per_node { + if let Some(k_piece) = seg.get_key_piece_from_seg_if_present() { + write!(f, "key: {} ", k_piece)?; + } + } + + write!(f, ")")?; + + Ok(()) + } +} + +pub fn get_path_from_query(trie: &Node, q: DebugQuery) -> DebugQueryOutput { + let mut out = DebugQueryOutput::new(q.k, q.params); + get_path_from_query_rec(trie, &mut q.k.clone(), &mut out); + + out +} + +fn get_path_from_query_rec( + node: &Node, + curr_key: &mut Nibbles, + query_out: &mut DebugQueryOutput, +) { + let seg = get_segment_from_node_and_key_piece(node, curr_key); + query_out.node_path.append(seg); + + match node { + Node::Empty | Node::Hash(_) => (), + Node::Branch { children, value: _ } => { + let nib = curr_key.pop_next_nibble_front(); + get_path_from_query_rec(&children[nib as usize], curr_key, query_out) + } + Node::Extension { nibbles, child } => { + curr_key.pop_nibbles_front(nibbles.count); + get_path_from_query_rec(child, curr_key, query_out); + } + Node::Leaf { nibbles, value: _ } => { + curr_key.pop_nibbles_front(nibbles.count); + } + } + + if curr_key.is_empty() { + query_out.node_found = true; + } +} From d3096008db57cd7560aa3d02cf3f9e78c24213df Mon Sep 17 00:00:00 2001 From: BGluth Date: Mon, 22 Jan 2024 15:22:11 -0700 Subject: [PATCH 10/20] Made the debug query tools output more info --- src/debug_tools/common.rs | 5 +- src/debug_tools/query.rs | 133 ++++++++++++++++++++++++++++++++++---- 2 files changed, 121 insertions(+), 17 deletions(-) diff --git a/src/debug_tools/common.rs b/src/debug_tools/common.rs index ca3812a..7eeea73 100644 --- a/src/debug_tools/common.rs +++ b/src/debug_tools/common.rs @@ -55,10 +55,7 @@ pub(super) fn get_segment_from_node_and_key_piece( match TrieNodeType::from(n) { TrieNodeType::Empty => PathSegment::Empty, TrieNodeType::Hash => PathSegment::Hash, - TrieNodeType::Branch => { - debug_assert_eq!(k_piece.count, 1); - PathSegment::Branch(k_piece.get_nibble(0)) - } + TrieNodeType::Branch => PathSegment::Branch(k_piece.get_nibble(0)), TrieNodeType::Extension => PathSegment::Extension(*k_piece), TrieNodeType::Leaf => PathSegment::Leaf(*k_piece), } diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs index e5347fd..8db40d7 100644 --- a/src/debug_tools/query.rs +++ b/src/debug_tools/query.rs @@ -1,17 +1,18 @@ use std::fmt::{self, Display}; +use ethereum_types::H256; + use super::common::{get_segment_from_node_and_key_piece, NodePath, PathSegment}; use crate::{ nibbles::Nibbles, - partial_trie::{Node, PartialTrie}, + partial_trie::{Node, PartialTrie, WrappedNode}, }; #[derive(Clone, Debug)] pub struct DebugQueryParams { include_key_piece_per_node: bool, include_node_type: bool, - // TODO: Look at implementing later... - // include_node_specific_values: bool, + include_node_specific_values: bool, } impl Default for DebugQueryParams { @@ -19,6 +20,7 @@ impl Default for DebugQueryParams { Self { include_key_piece_per_node: true, include_node_type: true, + include_node_specific_values: false, } } } @@ -41,8 +43,17 @@ impl DebugQueryParamsBuilder { self } - pub fn build(self) -> DebugQueryParams { - self.params + /// Defaults to `false`. + pub fn print_node_specific_values(mut self, enabled: bool) -> Self { + self.params.include_node_specific_values = enabled; + self + } + + pub fn build>(self, k: K) -> DebugQuery { + DebugQuery { + k: k.into(), + params: self.params, + } } } @@ -53,15 +64,80 @@ pub struct DebugQuery { } impl From for DebugQuery { - fn from(_v: Nibbles) -> Self { - todo!() + fn from(k: Nibbles) -> Self { + Self { + k, + params: DebugQueryParams::default(), + } + } +} + +#[derive(Clone, Debug)] +enum ExtraNodeSegmentInfo { + Hash(H256), + Branch { child_mask: u16 }, + Leaf { value: Vec }, +} + +impl Display for ExtraNodeSegmentInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ExtraNodeSegmentInfo::Hash(h) => write!(f, "Hash: {:x}", h), + ExtraNodeSegmentInfo::Branch { child_mask } => write!( + f, + "mask: {:#018b} (Num Children: {})", + child_mask, + count_non_empty_branch_children_from_mask(*child_mask) + ), + ExtraNodeSegmentInfo::Leaf { value } => write!(f, "Leaf Value: {}", hex::encode(value)), + } + } +} + +impl ExtraNodeSegmentInfo { + pub(super) fn from_node(n: &Node) -> Option { + match n { + Node::Empty | Node::Extension { .. } => None, + Node::Hash(h) => Some(ExtraNodeSegmentInfo::Hash(*h)), + Node::Branch { children, .. } => Some(ExtraNodeSegmentInfo::Branch { + child_mask: create_child_mask_from_children(children), + }), + Node::Leaf { value, .. } => Some(ExtraNodeSegmentInfo::Leaf { + value: value.clone(), + }), + } + } +} + +fn create_child_mask_from_children(children: &[WrappedNode; 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) { + mask |= (1 << i) as u16; + } + } + + mask +} + +fn count_non_empty_branch_children_from_mask(mask: u16) -> usize { + let mut num_children = 0; + + for i in 0..16 { + num_children += ((mask & (1 << i)) > 0) as usize; } + + num_children } #[derive(Clone, Debug)] pub struct DebugQueryOutput { k: Nibbles, node_path: NodePath, + extra_node_info: Vec>, node_found: bool, params: DebugQueryParams, } @@ -71,14 +147,25 @@ impl Display for DebugQueryOutput { self.fmt_query_header(f)?; writeln!(f, "Query path:")?; - for seg in self.node_path.0.iter().take(self.node_path.0.len() - 1) { - Self::fmt_node_based_on_debug_params(f, seg, &self.params)?; + for (i, seg) in self + .node_path + .0 + .iter() + .take(self.node_path.0.len() - 1) + .enumerate() + { + Self::fmt_node_based_on_debug_params(f, seg, &self.extra_node_info[i], &self.params)?; writeln!(f)?; writeln!(f, "V")?; } if let Some(last_seg) = self.node_path.0.last() { - Self::fmt_node_based_on_debug_params(f, last_seg, &self.params)?; + Self::fmt_node_based_on_debug_params( + f, + last_seg, + &self.extra_node_info[self.node_path.0.len() - 1], + &self.params, + )?; } Ok(()) @@ -90,6 +177,7 @@ impl DebugQueryOutput { Self { k, node_path: NodePath::default(), + extra_node_info: Vec::default(), node_found: false, params, } @@ -99,7 +187,7 @@ impl DebugQueryOutput { writeln!(f, "Query Result {{")?; writeln!(f, "Queried Key: {}", self.k)?; - writeln!(f, "Node node: {}", self.node_found)?; + writeln!(f, "Node found: {}", self.node_found)?; writeln!(f, "}}") } @@ -107,6 +195,7 @@ impl DebugQueryOutput { fn fmt_node_based_on_debug_params( f: &mut fmt::Formatter<'_>, seg: &PathSegment, + extra_seg_info: &Option, params: &DebugQueryParams, ) -> fmt::Result { let node_type = seg.node_type(); @@ -119,7 +208,17 @@ impl DebugQueryOutput { if params.include_key_piece_per_node { if let Some(k_piece) = seg.get_key_piece_from_seg_if_present() { - write!(f, "key: {} ", k_piece)?; + write!(f, "key: {}", k_piece)?; + } + } + + if params.include_node_specific_values { + if let Some(extra_seg_info) = extra_seg_info { + if params.include_key_piece_per_node { + write!(f, ", ")?; + } + + write!(f, "Extra Seg Info: {}", extra_seg_info)?; } } @@ -129,7 +228,12 @@ impl DebugQueryOutput { } } -pub fn get_path_from_query(trie: &Node, q: DebugQuery) -> DebugQueryOutput { +pub fn get_path_from_query>( + trie: &Node, + q: Q, +) -> DebugQueryOutput { + let q = q.into(); + let mut out = DebugQueryOutput::new(q.k, q.params); get_path_from_query_rec(trie, &mut q.k.clone(), &mut out); @@ -143,6 +247,9 @@ fn get_path_from_query_rec( ) { let seg = get_segment_from_node_and_key_piece(node, curr_key); query_out.node_path.append(seg); + query_out + .extra_node_info + .push(ExtraNodeSegmentInfo::from_node(node)); match node { Node::Empty | Node::Hash(_) => (), From cfad1636e0edc90e59168fe5b5a1a709f322496d Mon Sep 17 00:00:00 2001 From: BGluth Date: Tue, 23 Jan 2024 12:55:41 -0700 Subject: [PATCH 11/20] Moved trie debug tools behind a feature flag --- Cargo.toml | 3 +++ src/lib.rs | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index cc10e5a..62b2692 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,9 @@ rlp-derive = "0.1.0" serde = { version = "1.0.160", features = ["derive"] } serde_json = "1.0.96" +[features] +trie_debug = [] + [lib] doc-scrape-examples = true diff --git a/src/lib.rs b/src/lib.rs index 98a147b..676e19b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,6 @@ #![allow(incomplete_features)] -pub mod debug_tools; pub mod nibbles; pub mod partial_trie; mod trie_hashing; @@ -20,5 +19,8 @@ pub mod trie_ops; pub mod trie_subsets; mod utils; +#[cfg(feature = "trie_debug")] +pub mod debug_tools; + #[cfg(test)] mod testing_utils; From 756fffe4c449d87e673bbfe5e34541fd60f2cbad Mon Sep 17 00:00:00 2001 From: BGluth Date: Tue, 23 Jan 2024 13:14:33 -0700 Subject: [PATCH 12/20] Fixed path segments for trie diffs not being set properly --- src/debug_tools/diff.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 8deab79..924a662 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -61,13 +61,18 @@ pub struct DiffPoint { } impl DiffPoint { - fn new(child_a: &HashedPartialTrie, child_b: &HashedPartialTrie, parent_k: Nibbles) -> Self { + fn new( + child_a: &HashedPartialTrie, + child_b: &HashedPartialTrie, + parent_k: Nibbles, + path: NodePath, + ) -> Self { 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, - path: NodePath::default(), + path, key: parent_k, a_info: NodeInfo::new(child_a, a_key, get_value_from_node(child_a).cloned()), b_info: NodeInfo::new(child_b, b_key, get_value_from_node(child_b).cloned()), @@ -138,6 +143,11 @@ fn find_latest_diff_point_where_tries_begin_to_diff( find_latest_diff_point_where_tries_begin_to_diff_rec(state, &mut longest_state); + // If there was a node diff, we always want to prioritize displaying this over a + // hash diff. The reasoning behind this is hash diffs can become sort of + // meaningless or misleading if the trie diverges at some point (eg. saying + // there is a hash diff deep in two separate trie structures doesn't make much + // sense). longest_state .longest_key_node_diff .or(longest_state.longest_key_hash_diff) @@ -156,6 +166,7 @@ impl LatestNodeDiffState { &state.curr_key, state.a, state.b, + state.curr_path.clone(), ); } @@ -165,6 +176,7 @@ impl LatestNodeDiffState { &state.curr_key, state.a, state.b, + state.curr_path.clone(), ); } @@ -173,12 +185,13 @@ impl LatestNodeDiffState { parent_k: &Nibbles, child_a: &HashedPartialTrie, child_b: &HashedPartialTrie, + path: NodePath, ) { if field .as_ref() .map_or(true, |d_point| d_point.key.count < parent_k.count) { - *field = Some(DiffPoint::new(child_a, child_b, *parent_k)) + *field = Some(DiffPoint::new(child_a, child_b, *parent_k, path)) } } } From f7ecaea2f667b49445f3551984d4ba8a507ead67 Mon Sep 17 00:00:00 2001 From: BGluth Date: Wed, 24 Jan 2024 11:40:56 -0700 Subject: [PATCH 13/20] Cleanup and a few fixes --- Cargo.toml | 1 + src/debug_tools/diff.rs | 72 ++++++++++++++++++++-------------------- src/debug_tools/query.rs | 12 +++++-- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 62b2692..d86f964 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ serde = { version = "1.0.160", features = ["derive"] } serde_json = "1.0.96" [features] +default = ["trie_debug"] trie_debug = [] [lib] diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 924a662..6ecfb46 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -138,10 +138,10 @@ fn find_latest_diff_point_where_tries_begin_to_diff( a: &HashedPartialTrie, b: &HashedPartialTrie, ) -> Option { - let state = LatestDiffPerCallState::new(a, b, Nibbles::default(), 0); - let mut longest_state = LatestNodeDiffState::default(); + let state = DepthDiffPerCallState::new(a, b, Nibbles::default(), 0); + let mut longest_state = DepthNodeDiffState::default(); - find_latest_diff_point_where_tries_begin_to_diff_rec(state, &mut longest_state); + find_diff_point_where_tries_begin_to_diff_depth_rec(state, &mut longest_state); // If there was a node diff, we always want to prioritize displaying this over a // hash diff. The reasoning behind this is hash diffs can become sort of @@ -154,13 +154,13 @@ fn find_latest_diff_point_where_tries_begin_to_diff( } #[derive(Debug, Default)] -struct LatestNodeDiffState { +struct DepthNodeDiffState { longest_key_node_diff: Option, longest_key_hash_diff: Option, } -impl LatestNodeDiffState { - fn try_update_longest_divergence_key_hash(&mut self, state: &LatestDiffPerCallState) { +impl DepthNodeDiffState { + fn try_update_longest_divergence_key_hash(&mut self, state: &DepthDiffPerCallState) { Self::replace_longest_field_if_our_key_is_larger( &mut self.longest_key_hash_diff, &state.curr_key, @@ -170,7 +170,7 @@ impl LatestNodeDiffState { ); } - fn try_update_longest_divergence_key_node(&mut self, state: &LatestDiffPerCallState) { + fn try_update_longest_divergence_key_node(&mut self, state: &DepthDiffPerCallState) { Self::replace_longest_field_if_our_key_is_larger( &mut self.longest_key_node_diff, &state.curr_key, @@ -198,7 +198,7 @@ impl LatestNodeDiffState { // State that is copied per recursive call. #[derive(Clone, Debug)] -struct LatestDiffPerCallState<'a> { +struct DepthDiffPerCallState<'a> { a: &'a HashedPartialTrie, b: &'a HashedPartialTrie, curr_key: Nibbles, @@ -208,7 +208,7 @@ struct LatestDiffPerCallState<'a> { curr_path: NodePath, } -impl<'a> LatestDiffPerCallState<'a> { +impl<'a> DepthDiffPerCallState<'a> { /// Exists solely to prevent construction of this type from going over /// multiple lines. fn new( @@ -234,7 +234,7 @@ impl<'a> LatestDiffPerCallState<'a> { b: &'a HashedPartialTrie, key_piece: &Nibbles, ) -> Self { - let new_segment = get_segment_from_node_and_key_piece(a, key_piece); + let new_segment = get_segment_from_node_and_key_piece(self.a, key_piece); let new_path = self.curr_path.dup_and_append(new_segment); Self { @@ -247,9 +247,9 @@ impl<'a> LatestDiffPerCallState<'a> { } } -fn find_latest_diff_point_where_tries_begin_to_diff_rec( - state: LatestDiffPerCallState, - longest_state: &mut LatestNodeDiffState, +fn find_diff_point_where_tries_begin_to_diff_depth_rec( + state: DepthDiffPerCallState, + depth_state: &mut DepthNodeDiffState, ) -> DiffDetectionState { let a_hash = state.a.hash(); let b_hash = state.b.hash(); @@ -270,7 +270,7 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( // mismatch. match (a_type, a_key_piece) == (b_type, b_key_piece) { false => { - longest_state.try_update_longest_divergence_key_node(&state); + depth_state.try_update_longest_divergence_key_node(&state); DiffDetectionState::NodeTypesDiffer } true => { @@ -281,7 +281,7 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( a_hash, b_hash, &state.new_from_parent(state.a, state.b, &Nibbles::default()), - longest_state, + depth_state, ) } ( @@ -297,13 +297,13 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( let mut most_significant_diff_found = DiffDetectionState::NoDiffDetected; for i in 0..16 { - let res = find_latest_diff_point_where_tries_begin_to_diff_rec( + let res = find_diff_point_where_tries_begin_to_diff_depth_rec( state.new_from_parent( &a_children[i as usize], &b_children[i as usize], &Nibbles::from_nibble(i as u8), ), - longest_state, + depth_state, ); most_significant_diff_found = most_significant_diff_found.pick_most_significant_state(&res); @@ -318,7 +318,7 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( // Also run a hash check if we haven't picked anything up yet. create_diff_detection_state_based_from_hash_and_gen_hashes( &state, - longest_state, + depth_state, ) } } @@ -332,14 +332,14 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( nibbles: _b_nibs, child: b_child, }, - ) => find_latest_diff_point_where_tries_begin_to_diff_rec( + ) => find_diff_point_where_tries_begin_to_diff_depth_rec( state.new_from_parent(a_child, b_child, a_nibs), - longest_state, + depth_state, ), (Node::Leaf { .. }, Node::Leaf { .. }) => { create_diff_detection_state_based_from_hash_and_gen_hashes( - &state, - longest_state, + &state.new_from_parent(state.a, state.b, &a_key_piece), + depth_state, ) } _ => unreachable!(), @@ -349,24 +349,24 @@ fn find_latest_diff_point_where_tries_begin_to_diff_rec( } fn create_diff_detection_state_based_from_hash_and_gen_hashes( - state: &LatestDiffPerCallState, - longest_state: &mut LatestNodeDiffState, + state: &DepthDiffPerCallState, + depth_state: &mut DepthNodeDiffState, ) -> DiffDetectionState { let a_hash = state.a.hash(); let b_hash = state.b.hash(); - create_diff_detection_state_based_from_hashes(&a_hash, &b_hash, state, longest_state) + create_diff_detection_state_based_from_hashes(&a_hash, &b_hash, state, depth_state) } fn create_diff_detection_state_based_from_hashes( a_hash: &H256, b_hash: &H256, - state: &LatestDiffPerCallState, - longest_state: &mut LatestNodeDiffState, + state: &DepthDiffPerCallState, + depth_state: &mut DepthNodeDiffState, ) -> DiffDetectionState { match a_hash == b_hash { false => { - longest_state.try_update_longest_divergence_key_hash(state); + depth_state.try_update_longest_divergence_key_hash(state); DiffDetectionState::HashDiffDetected } true => DiffDetectionState::NoDiffDetected, @@ -403,7 +403,7 @@ mod tests { }; #[test] - fn latest_single_node_hash_diffs_work() { + fn depth_single_node_hash_diffs_work() { // TODO: Reduce duplication once we identify common structures across tests... let mut a = HashedPartialTrie::default(); a.insert(0x1234, vec![0]); @@ -442,43 +442,43 @@ mod tests { #[test] #[ignore] - fn latest_single_node_node_diffs_work() { + fn depth_single_node_node_diffs_work() { todo!() } #[test] #[ignore] - fn latest_multi_node_single_node_hash_diffs_work() { + fn depth_multi_node_single_node_hash_diffs_work() { todo!() } #[test] #[ignore] - fn latest_multi_node_single_node_node_diffs_work() { + fn depth_multi_node_single_node_node_diffs_work() { todo!() } #[test] #[ignore] - fn latest_massive_single_node_diff_tests() { + fn depth_massive_single_node_diff_tests() { todo!() } #[test] #[ignore] - fn latest_multi_node_multi_node_hash_diffs_work() { + fn depth_multi_node_multi_node_hash_diffs_work() { todo!() } #[test] #[ignore] - fn latest_multi_node_multi_node_node_diffs_work() { + fn depth_multi_node_multi_node_node_diffs_work() { todo!() } #[test] #[ignore] - fn latest_massive_multi_node_diff_tests() { + fn depth_massive_multi_node_diff_tests() { todo!() } } diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs index 8db40d7..ca9beb6 100644 --- a/src/debug_tools/query.rs +++ b/src/debug_tools/query.rs @@ -258,11 +258,12 @@ fn get_path_from_query_rec( get_path_from_query_rec(&children[nib as usize], curr_key, query_out) } Node::Extension { nibbles, child } => { - curr_key.pop_nibbles_front(nibbles.count); + pop_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); get_path_from_query_rec(child, curr_key, query_out); } Node::Leaf { nibbles, value: _ } => { - curr_key.pop_nibbles_front(nibbles.count); + pop_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); + curr_key.pop_nibbles_front(nibbles.count.min(curr_key.count)); } } @@ -270,3 +271,10 @@ fn get_path_from_query_rec( query_out.node_found = true; } } + +/// 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 pop_nibbles_from_node_key_and_avoid_underflow(curr_key: &mut Nibbles, node_key: &Nibbles) { + let num_nibs_to_pop = node_key.count.min(curr_key.count); + curr_key.pop_nibbles_front(num_nibs_to_pop); +} From 49e6594c0d1a51fa6bc3243f2f3fa78b4414d4f2 Mon Sep 17 00:00:00 2001 From: BGluth Date: Thu, 25 Jan 2024 08:04:51 -0700 Subject: [PATCH 14/20] More fixes --- src/debug_tools/common.rs | 19 +++++++++++++++++++ src/debug_tools/diff.rs | 22 +++++++--------------- src/debug_tools/query.rs | 35 +++++++++++++++++++++++++++-------- 3 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/debug_tools/common.rs b/src/debug_tools/common.rs index 7eeea73..5c018c6 100644 --- a/src/debug_tools/common.rs +++ b/src/debug_tools/common.rs @@ -61,6 +61,25 @@ pub(super) fn get_segment_from_node_and_key_piece( } } +pub(super) fn get_key_piece_from_node(n: &Node, curr_key: &Nibbles) -> Nibbles { + match n { + Node::Empty | Node::Hash(_) => Nibbles::default(), + Node::Branch { .. } => curr_key.get_next_nibbles(1), + Node::Extension { nibbles, child: _ } => *nibbles, + 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(n: &Node) -> Nibbles { + match n { + Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), + Node::Extension { nibbles, child: _ } => *nibbles, + Node::Leaf { nibbles, value: _ } => *nibbles, + } +} + #[derive(Clone, Debug, Default, Eq, Hash, PartialEq)] pub struct NodePath(pub(super) Vec); diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 6ecfb46..6ade1f0 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -3,7 +3,9 @@ use std::{fmt::Display, ops::Deref}; use ethereum_types::H256; -use super::common::{get_segment_from_node_and_key_piece, NodePath}; +use super::common::{ + get_key_piece_from_node_no_branch_key, get_segment_from_node_and_key_piece, NodePath, +}; use crate::{ nibbles::Nibbles, partial_trie::{HashedPartialTrie, Node, PartialTrie}, @@ -67,8 +69,8 @@ impl DiffPoint { parent_k: Nibbles, path: NodePath, ) -> Self { - 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)); + 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)); DiffPoint { depth: 0, @@ -263,8 +265,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(state.a); - let b_key_piece = get_key_piece_from_node(state.b); + 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); // Note that differences in a node's `value` will be picked up by a hash // mismatch. @@ -373,16 +375,6 @@ fn create_diff_detection_state_based_from_hashes( } } -// 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. -fn get_key_piece_from_node(n: &Node) -> Nibbles { - match n { - Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), - Node::Extension { nibbles, child: _ } => *nibbles, - Node::Leaf { nibbles, value: _ } => *nibbles, - } -} - /// If the node type contains a value (without looking at the children), then /// return it. fn get_value_from_node(n: &Node) -> Option<&Vec> { diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs index ca9beb6..e370d39 100644 --- a/src/debug_tools/query.rs +++ b/src/debug_tools/query.rs @@ -2,7 +2,9 @@ use std::fmt::{self, Display}; use ethereum_types::H256; -use super::common::{get_segment_from_node_and_key_piece, NodePath, PathSegment}; +use super::common::{ + get_key_piece_from_node, get_segment_from_node_and_key_piece, NodePath, PathSegment, +}; use crate::{ nibbles::Nibbles, partial_trie::{Node, PartialTrie, WrappedNode}, @@ -245,7 +247,12 @@ fn get_path_from_query_rec( curr_key: &mut Nibbles, query_out: &mut DebugQueryOutput, ) { - let seg = get_segment_from_node_and_key_piece(node, curr_key); + let key_piece = get_key_piece_from_node(node, curr_key); + + println!("key piece: {:x}", key_piece); + + let seg = get_segment_from_node_and_key_piece(node, &key_piece); + query_out.node_path.append(seg); query_out .extra_node_info @@ -255,15 +262,24 @@ fn get_path_from_query_rec( Node::Empty | Node::Hash(_) => (), Node::Branch { children, value: _ } => { let nib = curr_key.pop_next_nibble_front(); + + println!("NIB: {:x}", nib); + + // println!("CHILDREN: {:#?}", children); + get_path_from_query_rec(&children[nib as usize], curr_key, query_out) } Node::Extension { nibbles, child } => { - pop_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); + get_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); get_path_from_query_rec(child, curr_key, query_out); } Node::Leaf { nibbles, value: _ } => { - pop_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); - curr_key.pop_nibbles_front(nibbles.count.min(curr_key.count)); + let curr_key_next_nibs = + get_nibbles_from_node_key_and_avoid_underflow(curr_key, nibbles); + + if *nibbles == curr_key_next_nibs { + curr_key.pop_nibbles_front(curr_key_next_nibs.count); + } } } @@ -274,7 +290,10 @@ fn get_path_from_query_rec( /// 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 pop_nibbles_from_node_key_and_avoid_underflow(curr_key: &mut Nibbles, node_key: &Nibbles) { - let num_nibs_to_pop = node_key.count.min(curr_key.count); - curr_key.pop_nibbles_front(num_nibs_to_pop); +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) } From 81d8566a2b995d44d7af2a55e2a8fb574c973576 Mon Sep 17 00:00:00 2001 From: BGluth Date: Mon, 29 Jan 2024 12:41:45 -0700 Subject: [PATCH 15/20] Small fix in trie query --- src/debug_tools/diff.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 6ade1f0..570860b 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -86,7 +86,7 @@ impl Display for DiffPoint { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Point Diff {{depth: {}, ", self.depth)?; write!(f, "Path: ({}), ", self.path)?; - write!(f, "Key: 0x{:x} ", self.key)?; + write!(f, "Key: {:x} ", self.key)?; write!(f, "A info: {} ", self.a_info)?; write!(f, "B info: {}}}", self.b_info) } @@ -105,7 +105,7 @@ pub struct NodeInfo { impl Display for NodeInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "(key: 0x{:x} ", self.key)?; + write!(f, "(key: {:x} ", self.key)?; match &self.value { Some(v) => write!(f, "Value: 0x{}, ", hex::encode(v))?, @@ -339,10 +339,7 @@ fn find_diff_point_where_tries_begin_to_diff_depth_rec( depth_state, ), (Node::Leaf { .. }, Node::Leaf { .. }) => { - create_diff_detection_state_based_from_hash_and_gen_hashes( - &state.new_from_parent(state.a, state.b, &a_key_piece), - depth_state, - ) + create_diff_detection_state_based_from_hash_and_gen_hashes(&state, depth_state) } _ => unreachable!(), } From 41780f3a0eafba77098307587f6f385820a1dd64 Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 2 Feb 2024 10:11:07 -0700 Subject: [PATCH 16/20] Added documentation for the debug tooling --- src/debug_tools/diff.rs | 36 +++++++++++++++++++++++++++++++++++- src/debug_tools/mod.rs | 3 +++ src/debug_tools/query.rs | 15 ++++++++------- 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 570860b..04cc48c 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -1,3 +1,30 @@ +//! Diffing tools to compare two tries against each other. Useful when you want +//! to find where the tries diverge from one other. +//! +//! There are a few considerations when implementing the logic to create a trie +//! diff: +//! - What should be reported when the trie node structures diverge (eg. two +//! different node types proceeding a given common node)? +//! - If there are multiple structural differences, how do we discover the +//! smallest difference to report back? +//! +//! If the node types between the tries (structure) are identical but some +//! values are different, then these types of diffs are easy to detect and +//! report the lowest difference. Structural differences are more challenging +//! and a bit hard to report well. There are two approaches (only one currently +//! is implemented) in how to detect structural differences: +//! - Top-down search +//! - Bottom-up search +//! +//! These two searches are somewhat self-explanatory: +//! - Top-down will find the highest point of a structural divergence and report +//! it. If there are multiple divergences, then only the one that is the +//! highest in the trie will be reported. +//! - Bottom-up (not implemented) is a lot more complex to implement, but will +//! attempt to find the smallest structural trie difference between the trie. +//! If there are multiple differences, then this will likely be what you want +//! to use. + use std::fmt::{self, Debug}; use std::{fmt::Display, ops::Deref}; @@ -53,6 +80,7 @@ impl DiffDetectionState { } } +/// A point (node) between the two tries where the children differ. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct DiffPoint { pub depth: usize, @@ -82,6 +110,7 @@ impl DiffPoint { } } +// TODO: Redo display method so this is more readable... impl Display for DiffPoint { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Point Diff {{depth: {}, ", self.depth)?; @@ -92,6 +121,7 @@ impl Display for DiffPoint { } } +/// Meta information for a node in a trie. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct NodeInfo { key: Nibbles, @@ -103,6 +133,7 @@ pub struct NodeInfo { hash: H256, } +// TODO: Redo display method so this is more readable... impl Display for NodeInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "(key: {:x} ", self.key)?; @@ -128,6 +159,8 @@ impl NodeInfo { } } +/// Create a diff between two tries. Will perform both types of diff searches +/// (top-down & bottom-up). pub fn create_diff_between_tries(a: &HashedPartialTrie, b: &HashedPartialTrie) -> TrieDiff { TrieDiff { latest_diff_res: find_latest_diff_point_where_tries_begin_to_diff(a, b), @@ -198,7 +231,7 @@ impl DepthNodeDiffState { } } -// State that is copied per recursive call. +/// State that is copied per recursive call. #[derive(Clone, Debug)] struct DepthDiffPerCallState<'a> { a: &'a HashedPartialTrie, @@ -429,6 +462,7 @@ mod tests { assert_eq!(diff.latest_diff_res, Some(expected)); } + // TODO: Will finish these tests later (low-priority). #[test] #[ignore] fn depth_single_node_node_diffs_work() { diff --git a/src/debug_tools/mod.rs b/src/debug_tools/mod.rs index 7e49ab1..c05eb26 100644 --- a/src/debug_tools/mod.rs +++ b/src/debug_tools/mod.rs @@ -1,3 +1,6 @@ +//! Additional methods that may be useful when diagnosing tries from this +//! library. + pub mod common; pub mod diff; pub mod query; diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs index e370d39..63ede35 100644 --- a/src/debug_tools/query.rs +++ b/src/debug_tools/query.rs @@ -1,3 +1,6 @@ +//! Query tooling to report info on the path taken when searching down a trie +//! with a given key. + use std::fmt::{self, Display}; use ethereum_types::H256; @@ -10,6 +13,7 @@ use crate::{ partial_trie::{Node, PartialTrie, WrappedNode}, }; +/// Params controlling how much information is reported in the query output. #[derive(Clone, Debug)] pub struct DebugQueryParams { include_key_piece_per_node: bool, @@ -59,6 +63,7 @@ impl DebugQueryParamsBuilder { } } +/// The payload to give to the query function. Construct this from the builder. #[derive(Debug)] pub struct DebugQuery { k: Nibbles, @@ -185,6 +190,7 @@ impl DebugQueryOutput { } } + // TODO: Make the output easier to read... fn fmt_query_header(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "Query Result {{")?; @@ -194,6 +200,7 @@ impl DebugQueryOutput { writeln!(f, "}}") } + // TODO: Make the output easier to read... fn fmt_node_based_on_debug_params( f: &mut fmt::Formatter<'_>, seg: &PathSegment, @@ -230,6 +237,7 @@ impl DebugQueryOutput { } } +/// Get debug information on the path taken when querying a key in a given trie. pub fn get_path_from_query>( trie: &Node, q: Q, @@ -248,9 +256,6 @@ fn get_path_from_query_rec( query_out: &mut DebugQueryOutput, ) { let key_piece = get_key_piece_from_node(node, curr_key); - - println!("key piece: {:x}", key_piece); - let seg = get_segment_from_node_and_key_piece(node, &key_piece); query_out.node_path.append(seg); @@ -263,10 +268,6 @@ fn get_path_from_query_rec( Node::Branch { children, value: _ } => { let nib = curr_key.pop_next_nibble_front(); - println!("NIB: {:x}", nib); - - // println!("CHILDREN: {:#?}", children); - get_path_from_query_rec(&children[nib as usize], curr_key, query_out) } Node::Extension { nibbles, child } => { From 91f37944b49d4819f255ce69595ffcbd9295df31 Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 2 Feb 2024 15:43:45 -0700 Subject: [PATCH 17/20] Apply suggestions from code review Robin's suggested changes for #6 Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> --- src/debug_tools/common.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/debug_tools/common.rs b/src/debug_tools/common.rs index 5c018c6..4b496ac 100644 --- a/src/debug_tools/common.rs +++ b/src/debug_tools/common.rs @@ -42,8 +42,7 @@ impl PathSegment { match self { PathSegment::Empty | PathSegment::Hash => None, PathSegment::Branch(nib) => Some(Nibbles::from_nibble(*nib)), - PathSegment::Extension(nibs) => Some(*nibs), - PathSegment::Leaf(nibs) => Some(*nibs), + PathSegment::Extension(nibs) | PathSegment::Leaf(nibs) => Some(*nibs), } } } @@ -65,8 +64,7 @@ pub(super) fn get_key_piece_from_node(n: &Node, curr_key: &Ni match n { Node::Empty | Node::Hash(_) => Nibbles::default(), Node::Branch { .. } => curr_key.get_next_nibbles(1), - Node::Extension { nibbles, child: _ } => *nibbles, - Node::Leaf { nibbles, value: _ } => *nibbles, + Node::Extension { nibbles, child: _ } | Node::Leaf { nibbles, value: _ } => *nibbles, } } @@ -75,8 +73,7 @@ pub(super) fn get_key_piece_from_node(n: &Node, curr_key: &Ni pub(super) fn get_key_piece_from_node_no_branch_key(n: &Node) -> Nibbles { match n { Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), - Node::Extension { nibbles, child: _ } => *nibbles, - Node::Leaf { nibbles, value: _ } => *nibbles, + Node::Extension { nibbles, child: _ } | Node::Leaf { nibbles, value: _ } => *nibbles, } } From a830ab3442601e2d5316bfaf51bce9d3b2d352e8 Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 2 Feb 2024 16:17:27 -0700 Subject: [PATCH 18/20] Apply suggestions from code review Robin's suggestions #2 Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com> --- src/debug_tools/diff.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 04cc48c..17c7026 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -283,7 +283,7 @@ impl<'a> DepthDiffPerCallState<'a> { } fn find_diff_point_where_tries_begin_to_diff_depth_rec( - state: DepthDiffPerCallState, + state: &DepthDiffPerCallState, depth_state: &mut DepthNodeDiffState, ) -> DiffDetectionState { let a_hash = state.a.hash(); @@ -410,8 +410,7 @@ fn create_diff_detection_state_based_from_hashes( fn get_value_from_node(n: &Node) -> Option<&Vec> { match n { Node::Empty | Node::Hash(_) | Node::Extension { .. } => None, - Node::Branch { value, .. } => Some(value), - Node::Leaf { nibbles: _, value } => Some(value), + Node::Branch { value, .. } | Node::Leaf { nibbles: _, value } => Some(value), } } From e2d24b79e77625d1b87d17455312990f9ffb9d0c Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 2 Feb 2024 15:52:57 -0700 Subject: [PATCH 19/20] Requested changes for PR #6 --- src/debug_tools/common.rs | 18 ++++++++++++---- src/debug_tools/diff.rs | 12 +++++------ src/debug_tools/query.rs | 45 +++++++++++++++++++++++++-------------- src/nibbles.rs | 2 -- src/utils.rs | 16 ++++++++------ 5 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/debug_tools/common.rs b/src/debug_tools/common.rs index 4b496ac..b63bba9 100644 --- a/src/debug_tools/common.rs +++ b/src/debug_tools/common.rs @@ -60,7 +60,17 @@ pub(super) fn get_segment_from_node_and_key_piece( } } -pub(super) fn get_key_piece_from_node(n: &Node, 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( + n: &Node, + curr_key: &Nibbles, +) -> Nibbles { match n { Node::Empty | Node::Hash(_) => Nibbles::default(), Node::Branch { .. } => curr_key.get_next_nibbles(1), @@ -68,9 +78,9 @@ pub(super) fn get_key_piece_from_node(n: &Node, curr_key: &Ni } } -// 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(n: &Node) -> 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(n: &Node) -> Nibbles { match n { Node::Empty | Node::Hash(_) | Node::Branch { .. } => Nibbles::default(), Node::Extension { nibbles, child: _ } | Node::Leaf { nibbles, value: _ } => *nibbles, diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 17c7026..65983d9 100644 --- a/src/debug_tools/diff.rs +++ b/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. diff --git a/src/debug_tools/query.rs b/src/debug_tools/query.rs index 63ede35..e75c2ed 100644 --- a/src/debug_tools/query.rs +++ b/src/debug_tools/query.rs @@ -6,7 +6,8 @@ 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, @@ -14,10 +15,21 @@ use crate::{ }; /// 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 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(children: &[WrappedNode; 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( 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( 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( } } -/// 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 {} diff --git a/src/nibbles.rs b/src/nibbles.rs index 730d924..b463feb 100644 --- a/src/nibbles.rs +++ b/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)), diff --git a/src/utils.rs b/src/utils.rs index 887eca9..9b87d86 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -35,13 +35,15 @@ impl From<&Node> 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) } } From 753ab996ad9dbd6c906fc0378685ee96b52c1a4c Mon Sep 17 00:00:00 2001 From: BGluth Date: Fri, 2 Feb 2024 16:16:46 -0700 Subject: [PATCH 20/20] Requested changes for PR #6 (2) --- src/debug_tools/diff.rs | 155 +++++++++++++++++++--------------------- 1 file changed, 72 insertions(+), 83 deletions(-) diff --git a/src/debug_tools/diff.rs b/src/debug_tools/diff.rs index 65983d9..b043d72 100644 --- a/src/debug_tools/diff.rs +++ b/src/debug_tools/diff.rs @@ -1,5 +1,5 @@ //! Diffing tools to compare two tries against each other. Useful when you want -//! to find where the tries diverge from one other. +//! to find where the tries diverge from one each other. //! //! There are a few considerations when implementing the logic to create a trie //! diff: @@ -55,7 +55,7 @@ impl Display for TrieDiff { #[derive(Copy, Clone, Debug)] enum DiffDetectionState { - NodeTypesDiffer, // Also implies that hashes differ. + NodeTypesDiffer = 0, // Also implies that hashes differ. HashDiffDetected, NoDiffDetected, } @@ -69,12 +69,8 @@ impl DiffDetectionState { } /// The integer representation also indicates the more "significant" state. - fn get_int_repr(&self) -> usize { - match self { - DiffDetectionState::NodeTypesDiffer => 2, - DiffDetectionState::HashDiffDetected => 1, - DiffDetectionState::NoDiffDetected => 0, - } + fn get_int_repr(self) -> usize { + self as usize } } @@ -161,20 +157,20 @@ impl NodeInfo { /// (top-down & bottom-up). pub fn create_diff_between_tries(a: &HashedPartialTrie, b: &HashedPartialTrie) -> TrieDiff { TrieDiff { - latest_diff_res: find_latest_diff_point_where_tries_begin_to_diff(a, b), + latest_diff_res: find_latest_diff_point_between_tries(a, b), } } // Only support `HashedPartialTrie` due to it being significantly faster to -// detect differences due to hash caching. -fn find_latest_diff_point_where_tries_begin_to_diff( +// detect differences because of caching hashes. +fn find_latest_diff_point_between_tries( a: &HashedPartialTrie, b: &HashedPartialTrie, ) -> Option { let state = DepthDiffPerCallState::new(a, b, Nibbles::default(), 0); let mut longest_state = DepthNodeDiffState::default(); - find_diff_point_where_tries_begin_to_diff_depth_rec(state, &mut longest_state); + find_latest_diff_point_between_tries_rec(&state, &mut longest_state); // If there was a node diff, we always want to prioritize displaying this over a // hash diff. The reasoning behind this is hash diffs can become sort of @@ -224,7 +220,7 @@ impl DepthNodeDiffState { .as_ref() .map_or(true, |d_point| d_point.key.count < parent_k.count) { - *field = Some(DiffPoint::new(child_a, child_b, *parent_k, path)) + *field = Some(DiffPoint::new(child_a, child_b, *parent_k, path)); } } } @@ -280,7 +276,7 @@ impl<'a> DepthDiffPerCallState<'a> { } } -fn find_diff_point_where_tries_begin_to_diff_depth_rec( +fn find_latest_diff_point_between_tries_rec( state: &DepthDiffPerCallState, depth_state: &mut DepthNodeDiffState, ) -> DiffDetectionState { @@ -301,79 +297,72 @@ fn find_diff_point_where_tries_begin_to_diff_depth_rec( // Note that differences in a node's `value` will be picked up by a hash // mismatch. - match (a_type, a_key_piece) == (b_type, b_key_piece) { - false => { - depth_state.try_update_longest_divergence_key_node(&state); - DiffDetectionState::NodeTypesDiffer - } - true => { - match (&state.a.node, &state.b.node) { - (Node::Empty, Node::Empty) => DiffDetectionState::NoDiffDetected, - (Node::Hash(a_hash), Node::Hash(b_hash)) => { - create_diff_detection_state_based_from_hashes( - a_hash, - b_hash, - &state.new_from_parent(state.a, state.b, &Nibbles::default()), + if (a_type, a_key_piece) == (b_type, b_key_piece) { + depth_state.try_update_longest_divergence_key_node(state); + DiffDetectionState::NodeTypesDiffer + } else { + match (&state.a.node, &state.b.node) { + (Node::Empty, Node::Empty) => DiffDetectionState::NoDiffDetected, + (Node::Hash(a_hash), Node::Hash(b_hash)) => { + create_diff_detection_state_based_from_hashes( + a_hash, + b_hash, + &state.new_from_parent(state.a, state.b, &Nibbles::default()), + depth_state, + ) + } + ( + Node::Branch { + children: a_children, + value: _a_value, + }, + Node::Branch { + children: b_children, + value: _b_value, + }, + ) => { + let mut most_significant_diff_found = DiffDetectionState::NoDiffDetected; + + for i in 0..16_usize { + let res = find_latest_diff_point_between_tries_rec( + &state.new_from_parent( + &a_children[i], + &b_children[i], + &Nibbles::from_nibble(i as u8), + ), depth_state, - ) + ); + most_significant_diff_found = + most_significant_diff_found.pick_most_significant_state(&res); } - ( - Node::Branch { - children: a_children, - value: _a_value, - }, - Node::Branch { - children: b_children, - value: _b_value, - }, - ) => { - let mut most_significant_diff_found = DiffDetectionState::NoDiffDetected; - - for i in 0..16 { - let res = find_diff_point_where_tries_begin_to_diff_depth_rec( - state.new_from_parent( - &a_children[i as usize], - &b_children[i as usize], - &Nibbles::from_nibble(i as u8), - ), - depth_state, - ); - most_significant_diff_found = - most_significant_diff_found.pick_most_significant_state(&res); - } - - match matches!( - most_significant_diff_found, - DiffDetectionState::NoDiffDetected - ) { - false => most_significant_diff_found, - true => { - // Also run a hash check if we haven't picked anything up yet. - create_diff_detection_state_based_from_hash_and_gen_hashes( - &state, - depth_state, - ) - } - } - } - ( - Node::Extension { - nibbles: a_nibs, - child: a_child, - }, - Node::Extension { - nibbles: _b_nibs, - child: b_child, - }, - ) => find_diff_point_where_tries_begin_to_diff_depth_rec( - state.new_from_parent(a_child, b_child, a_nibs), - depth_state, - ), - (Node::Leaf { .. }, Node::Leaf { .. }) => { - create_diff_detection_state_based_from_hash_and_gen_hashes(&state, depth_state) + + if matches!( + most_significant_diff_found, + DiffDetectionState::NoDiffDetected + ) { + most_significant_diff_found + } else { + // Also run a hash check if we haven't picked anything up yet. + create_diff_detection_state_based_from_hash_and_gen_hashes(state, depth_state) } - _ => unreachable!(), } + ( + Node::Extension { + nibbles: a_nibs, + child: a_child, + }, + Node::Extension { + nibbles: _b_nibs, + child: b_child, + }, + ) => find_latest_diff_point_between_tries_rec( + &state.new_from_parent(a_child, b_child, a_nibs), + depth_state, + ), + (Node::Leaf { .. }, Node::Leaf { .. }) => { + create_diff_detection_state_based_from_hash_and_gen_hashes(state, depth_state) + } + _ => unreachable!(), } } }