chore(skeleton): add tree_height to UpdatedSkeleton

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree.rs

@@ -1,9 +1,9 @@
 use crate::patricia_merkle_tree::node_data::inner_node::EdgeData;
 use crate::patricia_merkle_tree::node_data::inner_node::PathToBottom;
 use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode;
-use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl;
 use crate::patricia_merkle_tree::original_skeleton_tree::utils::split_leaves;
 use crate::patricia_merkle_tree::types::NodeIndex;
+use crate::patricia_merkle_tree::types::TreeHeight;
 use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode;
 use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
 
@@ -25,36 +25,38 @@ impl TempSkeletonNode {
     }
 }
 
-impl OriginalSkeletonTreeImpl {
-    #[allow(dead_code)]
-    /// Returns the path from the given root_index to the LCA of the leaves. Assumes the leaves are:
-    /// * Sorted.
-    /// * Descendants of the given index.
-    /// * Non-empty list.
-    fn get_path_to_lca(&self, root_index: &NodeIndex, leaf_indices: &[NodeIndex]) -> PathToBottom {
-        if leaf_indices.is_empty() {
-            panic!("Unexpected empty array.");
-        }
-        let lca = if leaf_indices.len() == 1 {
-            leaf_indices[0]
-        } else {
-            leaf_indices[0].get_lca(leaf_indices.last().expect("Unexpected empty array"))
-        };
-        root_index.get_path_to_descendant(lca)
+#[allow(dead_code)]
+/// Returns the path from the given root_index to the LCA of the leaves. Assumes the leaves are:
+/// * Sorted.
+/// * Descendants of the given index.
+/// * Non-empty list.
+fn get_path_to_lca(root_index: &NodeIndex, leaf_indices: &[NodeIndex]) -> PathToBottom {
+    if leaf_indices.is_empty() {
+        panic!("Unexpected empty array.");
     }
+    let lca = if leaf_indices.len() == 1 {
+        leaf_indices[0]
+    } else {
+        leaf_indices[0].get_lca(leaf_indices.last().expect("Unexpected empty array"))
+    };
+    root_index.get_path_to_descendant(lca)
+}
 
-    /// Returns whether a root of a subtree has leaves on both sides. Assumes:
-    /// * The leaf indices array is sorted.
-    /// * All leaves are descendants of the root.
-    #[allow(dead_code)]
-    fn has_leaves_on_both_sides(&self, root_index: &NodeIndex, leaf_indices: &[NodeIndex]) -> bool {
-        if leaf_indices.is_empty() {
-            return false;
-        }
-        split_leaves(&self.tree_height, root_index, leaf_indices)
-            .iter()
-            .all(|leaves_in_side| !leaves_in_side.is_empty())
+/// Returns whether a root of a subtree has leaves on both sides. Assumes:
+/// * The leaf indices array is sorted.
+/// * All leaves are descendants of the root.
+#[allow(dead_code)]
+fn has_leaves_on_both_sides(
+    tree_height: &TreeHeight,
+    root_index: &NodeIndex,
+    leaf_indices: &[NodeIndex],
+) -> bool {
+    if leaf_indices.is_empty() {
+        return false;
     }
+    split_leaves(tree_height, root_index, leaf_indices)
+        .iter()
+        .all(|leaves_in_side| !leaves_in_side.is_empty())
 }
 
 impl UpdatedSkeletonTreeImpl {

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree_test.rs

@@ -1,29 +1,24 @@
 use ethnum::U256;
 use rstest::{fixture, rstest};
-use std::collections::HashMap;
 
 use crate::hash::hash_trait::HashOutput;
 use crate::patricia_merkle_tree::node_data::inner_node::{EdgeData, EdgePathLength, PathToBottom};
 use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf;
 use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode;
-use crate::patricia_merkle_tree::types::NodeIndex;
-use crate::patricia_merkle_tree::updated_skeleton_tree::compute_updated_skeleton_tree::TempSkeletonNode;
+use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::updated_skeleton_tree::compute_updated_skeleton_tree::{
+    get_path_to_lca, has_leaves_on_both_sides, TempSkeletonNode,
+};
 use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode;
 use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
-use crate::patricia_merkle_tree::{
-    original_skeleton_tree::tree::OriginalSkeletonTreeImpl, types::TreeHeight,
-};
-
-fn empty_skeleton(height: u8) -> OriginalSkeletonTreeImpl {
-    OriginalSkeletonTreeImpl {
-        nodes: HashMap::new(),
-        tree_height: TreeHeight::new(height),
-    }
-}
 
 #[fixture]
-fn updated_skeleton(#[default(&[])] leaf_modifications: &[(u128, u8)]) -> UpdatedSkeletonTreeImpl {
+fn updated_skeleton(
+    #[default(TreeHeight::MAX)] tree_height: TreeHeight,
+    #[default(&[])] leaf_modifications: &[(u128, u8)],
+) -> UpdatedSkeletonTreeImpl {
     UpdatedSkeletonTreeImpl {
+        tree_height,
         skeleton_tree: leaf_modifications
             .iter()
             .filter(|(_, leaf_val)| *leaf_val != 0)
@@ -57,15 +52,15 @@ fn updated_skeleton(#[default(&[])] leaf_modifications: &[(u128, u8)]) -> Update
     vec![NodeIndex::new(U256::from(3u8) << 250), NodeIndex::new((U256::from(3u8) << 250)+ U256::ONE)],
     false)]
 fn test_has_leaves_on_both_sides(
-    #[case] tree_height: u8,
+    #[case] _tree_height: u8,
+    #[with(TreeHeight::new(_tree_height), &[])] updated_skeleton: UpdatedSkeletonTreeImpl,
     #[case] root_index: u8,
     #[case] leaf_indices: Vec<NodeIndex>,
     #[case] expected: bool,
 ) {
-    let skeleton_tree = empty_skeleton(tree_height);
     let root_index = NodeIndex::new(root_index.into());
     assert_eq!(
-        skeleton_tree.has_leaves_on_both_sides(&root_index, &leaf_indices),
+        has_leaves_on_both_sides(&updated_skeleton.tree_height, &root_index, &leaf_indices),
         expected
     );
 }
@@ -75,41 +70,47 @@ fn test_has_leaves_on_both_sides(
 #[case::last_leaf_not_descendant(3, 2, vec![NodeIndex::from(8), NodeIndex::from(12)])]
 #[should_panic(expected = "is not a descendant of the root")]
 fn test_has_leaves_on_both_sides_assertions(
-    #[case] tree_height: u8,
+    #[case] _tree_height: u8,
+    #[with(TreeHeight::new(_tree_height), &[])] updated_skeleton: UpdatedSkeletonTreeImpl,
     #[case] root_index: u8,
     #[case] leaf_indices: Vec<NodeIndex>,
 ) {
-    let skeleton_tree = empty_skeleton(tree_height);
     let root_index = NodeIndex::new(root_index.into());
-    skeleton_tree.has_leaves_on_both_sides(&root_index, &leaf_indices);
+    has_leaves_on_both_sides(&updated_skeleton.tree_height, &root_index, &leaf_indices);
 }
 
 #[rstest]
-#[case::small_tree_single_leaf(3, 1, vec![U256::from(8_u8)], PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(3)})]
+#[case::small_tree_single_leaf(
+    1, vec![U256::from(8_u8)], PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(3)}
+)]
 #[case::small_tree_few_leaves(
-    3, 1, vec![U256::from(12_u8),U256::from(13_u8),U256::from(14_u8)], PathToBottom {path:U256::ONE.into(), length:EdgePathLength(1)})]
+    1,
+    vec![
+        U256::from(12_u8), U256::from(13_u8), U256::from(14_u8)
+    ],
+    PathToBottom {path:U256::ONE.into(), length:EdgePathLength(1)}
+)]
 #[case::small_tree_few_leaves2(
-    3, 1, vec![U256::from(12_u8),U256::from(13_u8)], PathToBottom {path:2_u128.into(), length:EdgePathLength(2)})]
+    1,
+    vec![U256::from(12_u8),U256::from(13_u8)],
+    PathToBottom {path:2_u128.into(), length:EdgePathLength(2)}
+)]
 #[case::large_tree_positive_consecutive_indices_of_different_sides(
-    251,
     1,
     vec![(U256::from(3u8) << 250) - U256::ONE, U256::from(3u8) << 250],
     PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(0)})]
 #[case::large_tree_positive_consecutive_indices(
-    251,
     3<<126,
     vec![U256::from(3u8) << 250, (U256::from(3u8) << 250)+ U256::ONE],
     PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(123)})]
 fn test_get_path_to_lca(
-    #[case] tree_height: u8,
     #[case] root_index: u128,
     #[case] leaf_indices: Vec<U256>,
     #[case] expected: PathToBottom,
 ) {
-    let skeleton_tree = empty_skeleton(tree_height);
     let root_index = NodeIndex::new(root_index.into());
     assert_eq!(
-        skeleton_tree.get_path_to_lca(
+        get_path_to_lca(
             &root_index,
             &leaf_indices
                 .iter()
@@ -186,7 +187,7 @@ fn test_node_from_binary_data(
     #[case] _leaf_modifications: &[(u128, u8)],
     #[case] expected_node: TempSkeletonNode,
     #[case] expected_skeleton_additions: &[(NodeIndex, UpdatedSkeletonNode)],
-    #[with(_leaf_modifications)] mut updated_skeleton: UpdatedSkeletonTreeImpl,
+    #[with(TreeHeight::MAX, _leaf_modifications)] mut updated_skeleton: UpdatedSkeletonTreeImpl,
 ) {
     let mut expected_skeleton_tree = updated_skeleton.skeleton_tree.clone();
     expected_skeleton_tree.extend(expected_skeleton_additions.iter().cloned());
@@ -255,7 +256,7 @@ fn test_node_from_edge_data(
     #[case] _leaf_modifications: &[(u128, u8)],
     #[case] expected_node: TempSkeletonNode,
     #[case] expected_skeleton_additions: &[(NodeIndex, UpdatedSkeletonNode)],
-    #[with(_leaf_modifications)] mut updated_skeleton: UpdatedSkeletonTreeImpl,
+    #[with(TreeHeight::MAX, _leaf_modifications)] mut updated_skeleton: UpdatedSkeletonTreeImpl,
 ) {
     let mut expected_skeleton_tree = updated_skeleton.skeleton_tree.clone();
     expected_skeleton_tree.extend(expected_skeleton_additions.iter().cloned());

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 
 use crate::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf};
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTree;
-use crate::patricia_merkle_tree::types::NodeIndex;
+use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
 use crate::patricia_merkle_tree::updated_skeleton_tree::errors::UpdatedSkeletonTreeError;
 use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode;
 
@@ -38,6 +38,8 @@ pub(crate) trait UpdatedSkeletonTree: Sized + Send + Sync {
 }
 
 pub(crate) struct UpdatedSkeletonTreeImpl {
+    #[allow(dead_code)]
+    pub(crate) tree_height: TreeHeight,
     pub(crate) skeleton_tree: HashMap<NodeIndex, UpdatedSkeletonNode>,
 }
 

crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree_test.rs

@@ -9,7 +9,7 @@ use crate::patricia_merkle_tree::node_data::inner_node::{
     BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom,
 };
 use crate::patricia_merkle_tree::node_data::leaf::{LeafDataImpl, SkeletonLeaf};
-use crate::patricia_merkle_tree::types::NodeIndex;
+use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
 use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl;
 use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode;
 use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
@@ -23,7 +23,10 @@ async fn test_filled_tree_sanity() {
     let new_leaf_index = NodeIndex::ROOT;
     skeleton_tree.insert(new_leaf_index, UpdatedSkeletonNode::Leaf(new_skeleton_leaf));
     let modifications = HashMap::from([(new_leaf_index, new_filled_leaf)]);
-    let updated_skeleton_tree = UpdatedSkeletonTreeImpl { skeleton_tree };
+    let updated_skeleton_tree = UpdatedSkeletonTreeImpl {
+        tree_height: TreeHeight(1),
+        skeleton_tree,
+    };
     let root_hash = FilledTreeImpl::create::<
         PedersenHashFunction,
         TreeHashFunctionImpl<PedersenHashFunction>,
@@ -40,7 +43,7 @@ async fn test_filled_tree_sanity() {
 
 #[tokio::test(flavor = "multi_thread")]
 /// This test is a small test for testing the root hash computation of the patricia merkle tree.
-/// The tree structure & results were computed seperately and tested for regression.
+/// The tree structure & results were computed separately and tested for regression.
 ///                                i=1: binary
 ///                                /        \
 ///                        i=2: edge      i=3: edge
@@ -75,7 +78,10 @@ async fn test_small_filled_tree() {
     let skeleton_tree: HashMap<NodeIndex, UpdatedSkeletonNode> =
         nodes_in_skeleton_tree.into_iter().collect();
 
-    let updated_skeleton_tree = UpdatedSkeletonTreeImpl { skeleton_tree };
+    let updated_skeleton_tree = UpdatedSkeletonTreeImpl {
+        tree_height: TreeHeight(7),
+        skeleton_tree,
+    };
     let modifications = new_leaves
         .iter()
         .map(|(index, value)| {
@@ -186,7 +192,10 @@ async fn test_small_tree_with_sibling_nodes() {
     let skeleton_tree: HashMap<NodeIndex, UpdatedSkeletonNode> =
         nodes_in_skeleton_tree.into_iter().collect();
 
-    let updated_skeleton_tree = UpdatedSkeletonTreeImpl { skeleton_tree };
+    let updated_skeleton_tree = UpdatedSkeletonTreeImpl {
+        tree_height: TreeHeight(7),
+        skeleton_tree,
+    };
     let modifications = HashMap::from([(
         NodeIndex::from(new_leaf_index),
         LeafDataImpl::StorageValue(Felt::from_hex(new_leaf).unwrap()),