refactor: rename TreeHeight => SubTreeHeight and TreeHeight::MAX => T…

…reeHeight::ACTUAL_HEIGHT (#192)

crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs

@@ -2,7 +2,7 @@ use crate::felt::Felt;
 use crate::hash::hash_trait::HashOutput;
 use crate::patricia_merkle_tree::node_data::errors::{EdgePathError, PathToBottomError};
 use crate::patricia_merkle_tree::node_data::leaf::LeafData;
-use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight};
 
 use ethnum::U256;
 use strum_macros::{EnumDiscriminants, EnumIter};
@@ -28,7 +28,7 @@ pub struct BinaryData {
 pub struct EdgePath(pub U256);
 
 impl EdgePath {
-    pub const BITS: u8 = TreeHeight::MAX.0;
+    pub const BITS: u8 = SubTreeHeight::ACTUAL_HEIGHT.0;
 
     /// [EdgePath] constant that represents the longest path (from some node) in a tree.
     #[allow(clippy::as_conversions)]
@@ -70,7 +70,7 @@ pub struct EdgePathLength(u8);
 impl EdgePathLength {
     /// [EdgePathLength] constant that represents the longest path (from some node) in a tree.
     pub const ONE: Self = Self(1);
-    pub const MAX: Self = Self(TreeHeight::MAX.0);
+    pub const MAX: Self = Self(SubTreeHeight::ACTUAL_HEIGHT.0);
 
     pub fn new(length: u8) -> Result<Self, EdgePathError> {
         if length > Self::MAX.0 {

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs

@@ -6,7 +6,7 @@ use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonI
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl;
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeResult;
 use crate::patricia_merkle_tree::original_skeleton_tree::utils::split_leaves;
-use crate::patricia_merkle_tree::types::TreeHeight;
+use crate::patricia_merkle_tree::types::SubTreeHeight;
 use crate::patricia_merkle_tree::{
     original_skeleton_tree::node::OriginalSkeletonNode, types::NodeIndex,
 };
@@ -29,8 +29,8 @@ struct SubTree<'a> {
 }
 
 impl<'a> SubTree<'a> {
-    pub(crate) fn get_height(&self) -> TreeHeight {
-        TreeHeight::new(TreeHeight::MAX.0 - (self.root_index.bit_length() - 1))
+    pub(crate) fn get_height(&self) -> SubTreeHeight {
+        SubTreeHeight::new(SubTreeHeight::ACTUAL_HEIGHT.0 - (self.root_index.bit_length() - 1))
     }
 
     pub(crate) fn split_leaves(&self) -> [&'a [NodeIndex]; 2] {
@@ -43,7 +43,7 @@ impl<'a> SubTree<'a> {
 
     fn get_bottom_subtree(&self, path_to_bottom: &PathToBottom, bottom_hash: HashOutput) -> Self {
         let bottom_index = path_to_bottom.bottom_index(self.root_index);
-        let bottom_height = self.get_height() - TreeHeight::new(path_to_bottom.length.into());
+        let bottom_height = self.get_height() - SubTreeHeight::new(path_to_bottom.length.into());
         let leftmost_in_subtree = bottom_index << bottom_height.into();
         let rightmost_in_subtree =
             leftmost_in_subtree + (NodeIndex::ROOT << bottom_height.into()) - NodeIndex::ROOT;

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs

@@ -11,7 +11,7 @@ use pretty_assertions::assert_eq;
 use rstest::rstest;
 use std::collections::HashMap;
 
-use crate::patricia_merkle_tree::types::TreeHeight;
+use crate::patricia_merkle_tree::types::SubTreeHeight;
 use crate::storage::storage_trait::{create_db_key, StorageKey, StoragePrefix, StorageValue};
 
 use super::OriginalSkeletonTreeImpl;
@@ -45,8 +45,8 @@ use super::OriginalSkeletonTreeImpl;
 
 #[case::simple_tree_of_height_3(
     HashMap::from([
-    create_root_edge_entry(50, TreeHeight::new(3)),
-    create_root_edge_entry(50, TreeHeight::new(3)),
+    create_root_edge_entry(50, SubTreeHeight::new(3)),
+    create_root_edge_entry(50, SubTreeHeight::new(3)),
     create_binary_entry(8, 9),
     create_edge_entry(11, 1, 1),
     create_binary_entry(17, 13),
@@ -68,7 +68,7 @@ use super::OriginalSkeletonTreeImpl;
         ],
         3
     ),
-    TreeHeight::new(3)
+    SubTreeHeight::new(3)
 )]
 ///                 Old tree structure:
 ///
@@ -95,7 +95,7 @@ use super::OriginalSkeletonTreeImpl;
 
 #[case::another_simple_tree_of_height_3(
     HashMap::from([
-    create_root_edge_entry(29, TreeHeight::new(3)),
+    create_root_edge_entry(29, SubTreeHeight::new(3)),
     create_binary_entry(10, 2),
     create_edge_entry(3, 1, 1),
     create_binary_entry(4, 7),
@@ -117,7 +117,7 @@ use super::OriginalSkeletonTreeImpl;
         ],
         3
     ),
-    TreeHeight::new(3)
+    SubTreeHeight::new(3)
 )]
 ///                  Old tree structure:
 ///
@@ -147,7 +147,7 @@ use super::OriginalSkeletonTreeImpl;
 ///
 #[case::tree_of_height_4_with_long_edge(
     HashMap::from([
-    create_root_edge_entry(116, TreeHeight::new(4)),
+    create_root_edge_entry(116, SubTreeHeight::new(4)),
     create_binary_entry(11, 13),
     create_edge_entry(5, 0, 1),
     create_binary_entry(19, 40),
@@ -175,18 +175,18 @@ use super::OriginalSkeletonTreeImpl;
         ],
         4
     ),
-    TreeHeight::new(4)
+    SubTreeHeight::new(4)
 )]
 fn test_fetch_nodes(
     #[case] storage: MapStorage,
     #[case] leaf_modifications: LeafModifications<LeafDataImpl>,
     #[case] root_hash: HashOutput,
     #[case] expected_skeleton: OriginalSkeletonTreeImpl,
-    #[case] tree_height: TreeHeight,
+    #[case] subtree_height: SubTreeHeight,
 ) {
     let mut sorted_leaf_indices: Vec<NodeIndex> = leaf_modifications
         .keys()
-        .map(|idx| NodeIndex::from_subtree_index(*idx, tree_height))
+        .map(|idx| NodeIndex::from_subtree_index(*idx, subtree_height))
         .collect();
 
     sorted_leaf_indices.sort();
@@ -258,12 +258,15 @@ pub(crate) fn create_expected_skeleton(
     nodes: Vec<(NodeIndex, OriginalSkeletonNode)>,
     height: u8,
 ) -> OriginalSkeletonTreeImpl {
-    let tree_height = TreeHeight::new(height);
+    let subtree_height = SubTreeHeight::new(height);
     OriginalSkeletonTreeImpl {
         nodes: nodes
             .into_iter()
             .map(|(node_index, node)| {
-                (NodeIndex::from_subtree_index(node_index, tree_height), node)
+                (
+                    NodeIndex::from_subtree_index(node_index, subtree_height),
+                    node,
+                )
             })
             .chain([(
                 NodeIndex::ROOT,
@@ -316,10 +319,10 @@ pub(crate) fn create_unmodified_bottom_skeleton_node(
 
 pub(crate) fn create_root_edge_entry(
     old_root: u8,
-    subtree_height: TreeHeight,
+    subtree_height: SubTreeHeight,
 ) -> (StorageKey, StorageValue) {
     // Assumes path is 0.
-    let length = TreeHeight::MAX.0 - subtree_height.0;
+    let length = SubTreeHeight::ACTUAL_HEIGHT.0 - subtree_height.0;
     let new_root = u128::from(old_root) + u128::from(length);
     let key = create_db_key(
         StoragePrefix::InnerNode,

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs

@@ -18,7 +18,7 @@ use crate::patricia_merkle_tree::original_skeleton_tree::create_tree::create_tre
 };
 use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::OriginalSkeletonForest;
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl;
-use crate::patricia_merkle_tree::types::TreeHeight;
+use crate::patricia_merkle_tree::types::SubTreeHeight;
 use crate::storage::map_storage::MapStorage;
 
 // This test assumes for simplicity that hash is addition (i.e hash(a,b) = a + b).
@@ -77,10 +77,10 @@ use crate::storage::map_storage::MapStorage;
 #[case(
     Input {
         storage: HashMap::from([
-            create_root_edge_entry(29, TreeHeight::new(3)),
-            create_root_edge_entry(55, TreeHeight::new(3)),
-            create_root_edge_entry(157, TreeHeight::new(3)),
-            create_root_edge_entry(254, TreeHeight::new(3)),
+            create_root_edge_entry(29, SubTreeHeight::new(3)),
+            create_root_edge_entry(55, SubTreeHeight::new(3)),
+            create_root_edge_entry(157, SubTreeHeight::new(3)),
+            create_root_edge_entry(254, SubTreeHeight::new(3)),
             create_binary_entry(8, 9),
             create_edge_entry(16, 1, 1),
             create_binary_entry(17, 18),

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs

@@ -1,14 +1,14 @@
 use bisection::bisect_left;
 
-use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight};
 
 #[cfg(test)]
 #[path = "utils_test.rs"]
 pub mod utils_test;
 
 /// Returns the height of the node with the given index.
-pub(crate) fn get_node_height(index: &NodeIndex) -> TreeHeight {
-    TreeHeight::new(TreeHeight::MAX.0 + 1 - index.bit_length())
+pub(crate) fn get_node_height(index: &NodeIndex) -> SubTreeHeight {
+    SubTreeHeight::new(SubTreeHeight::ACTUAL_HEIGHT.0 + 1 - index.bit_length())
 }
 
 /// Splits leaf_indices into two arrays according to the given root: the left child leaves and

crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs

@@ -3,7 +3,7 @@ use crate::patricia_merkle_tree::test_utils::as_fully_indexed;
 use crate::patricia_merkle_tree::test_utils::get_random_u256;
 use crate::patricia_merkle_tree::test_utils::random;
 use crate::patricia_merkle_tree::test_utils::small_tree_index_to_full;
-use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight};
 use ethnum::{uint, U256};
 use rand::rngs::ThreadRng;
 use rand::Rng;
@@ -52,7 +52,7 @@ fn test_split_leaves(
     #[case] expected_left: &[U256],
     #[case] expected_right: &[U256],
 ) {
-    let height = TreeHeight(subtree_height);
+    let height = SubTreeHeight(subtree_height);
     let root_index = small_tree_index_to_full(root_index, height);
     let expected = [
         as_fully_indexed(subtree_height, expected_left.iter().copied()),
@@ -76,7 +76,7 @@ fn test_split_leaves_big_tree(mut random: ThreadRng) {
         U256::ONE << 100,
     );
     test_split_leaves(
-        TreeHeight::MAX.into(),
+        SubTreeHeight::ACTUAL_HEIGHT.into(),
         NodeIndex::ROOT.into(),
         [
             left_leaf_indices

crates/committer/src/patricia_merkle_tree/test_utils.rs

@@ -35,22 +35,22 @@ pub(crate) fn random() -> ThreadRng {
 }
 
 #[cfg(test)]
-use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight};
 
 #[cfg(test)]
 impl NodeIndex {
     /// Assumes self represents an index in a smaller tree height. Returns a node index represents
     /// the same index in the starknet state tree as if the smaller tree was 'planted' at the lowest
     /// leftmost node from the root.
-    pub(crate) fn from_subtree_index(subtree_index: Self, subtree_height: TreeHeight) -> Self {
-        let height_diff = TreeHeight::MAX.0 - subtree_height.0;
+    pub(crate) fn from_subtree_index(subtree_index: Self, subtree_height: SubTreeHeight) -> Self {
+        let height_diff = SubTreeHeight::ACTUAL_HEIGHT.0 - subtree_height.0;
         let offset = (NodeIndex::ROOT << height_diff) - 1.into();
         subtree_index + (offset << (subtree_index.bit_length() - 1))
     }
 }
 
 #[cfg(test)]
-pub(crate) fn small_tree_index_to_full(index: U256, height: TreeHeight) -> NodeIndex {
+pub(crate) fn small_tree_index_to_full(index: U256, height: SubTreeHeight) -> NodeIndex {
     NodeIndex::from_subtree_index(NodeIndex::new(index), height)
 }
 /// Generates a random U256 number between low and high (exclusive).
@@ -106,6 +106,6 @@ pub(crate) fn as_fully_indexed(
     indices: impl Iterator<Item = U256>,
 ) -> Vec<NodeIndex> {
     indices
-        .map(|index| small_tree_index_to_full(index, TreeHeight::new(subtree_height)))
+        .map(|index| small_tree_index_to_full(index, SubTreeHeight::new(subtree_height)))
         .collect()
 }

crates/committer/src/patricia_merkle_tree/types.rs

@@ -11,22 +11,21 @@ use ethnum::U256;
 pub mod types_test;
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq, derive_more::Sub, derive_more::Display)]
-pub struct TreeHeight(pub(crate) u8);
+pub struct SubTreeHeight(pub(crate) u8);
 
-impl TreeHeight {
-    // TODO(Tzahi, 15/6/2024): Make height configurable for easy testing.
-    pub const MAX: TreeHeight = TreeHeight(251);
+impl SubTreeHeight {
+    pub const ACTUAL_HEIGHT: SubTreeHeight = SubTreeHeight(251);
 
     pub fn new(height: u8) -> Self {
-        if height > Self::MAX.0 {
+        if height > Self::ACTUAL_HEIGHT.0 {
             panic!("Height {height} is too large.");
         }
         Self(height)
     }
 }
 
-impl From<TreeHeight> for u8 {
-    fn from(value: TreeHeight) -> Self {
+impl From<SubTreeHeight> for u8 {
+    fn from(value: SubTreeHeight) -> Self {
         value.0
     }
 }
@@ -38,7 +37,7 @@ pub struct NodeIndex(U256);
 
 // Wraps a U256. Maximal possible value is the largest index in a tree of height 251 (2 ^ 252 - 1).
 impl NodeIndex {
-    pub const BITS: u8 = TreeHeight::MAX.0 + 1;
+    pub const BITS: u8 = SubTreeHeight::ACTUAL_HEIGHT.0 + 1;
 
     /// [NodeIndex] constant that represents the root index.
     pub const ROOT: Self = Self(U256::ONE);

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

@@ -8,7 +8,7 @@ use crate::patricia_merkle_tree::node_data::inner_node::{EdgePathLength, PathToB
 use crate::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode;
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonNodeMap;
 use crate::patricia_merkle_tree::test_utils::small_tree_index_to_full;
-use crate::patricia_merkle_tree::types::{NodeIndex, TreeHeight};
+use crate::patricia_merkle_tree::types::{NodeIndex, SubTreeHeight};
 use crate::patricia_merkle_tree::updated_skeleton_tree::compute_updated_skeleton_tree::{
     get_path_to_lca, has_leaves_on_both_sides, TempSkeletonNode,
 };
@@ -78,7 +78,7 @@ fn test_has_leaves_on_both_sides(
     #[case] leaf_indices: Vec<NodeIndex>,
     #[case] expected: bool,
 ) {
-    let height = TreeHeight(subtree_height);
+    let height = SubTreeHeight(subtree_height);
     let root_index = small_tree_index_to_full(root_index.into(), height);
     assert_eq!(
         has_leaves_on_both_sides(&root_index, &leaf_indices),
@@ -95,7 +95,7 @@ fn test_has_leaves_on_both_sides_assertions(
     #[case] root_index: u8,
     #[case] leaf_indices: Vec<NodeIndex>,
 ) {
-    let height = TreeHeight(subtree_height);
+    let height = SubTreeHeight(subtree_height);
     let root_index = small_tree_index_to_full(root_index.into(), height);
     has_leaves_on_both_sides(&root_index, &leaf_indices);
 }
@@ -490,6 +490,6 @@ pub(crate) fn as_fully_indexed(
     indices: impl Iterator<Item = NodeIndex>,
 ) -> Vec<NodeIndex> {
     indices
-        .map(|index| NodeIndex::from_subtree_index(index, TreeHeight::new(subtree_height)))
+        .map(|index| NodeIndex::from_subtree_index(index, SubTreeHeight::new(subtree_height)))
         .collect()
 }

crates/committer_cli/src/tests/python_tests.rs

@@ -15,7 +15,7 @@ use committer::patricia_merkle_tree::node_data::inner_node::{
     BinaryData, EdgeData, EdgePathLength, NodeData, PathToBottom,
 };
 use committer::patricia_merkle_tree::node_data::leaf::{ContractState, LeafDataImpl};
-use committer::patricia_merkle_tree::types::TreeHeight;
+use committer::patricia_merkle_tree::types::SubTreeHeight;
 use committer::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl;
 use committer::storage::db_object::DBObject;
 use committer::storage::errors::{DeserializationError, SerializationError};
@@ -133,7 +133,7 @@ impl PythonTest {
                 let block_info: BlockInfo = serde_json::from_str(Self::non_optional_input(input)?)?;
                 Ok(parse_block_info_test(block_info))
             }
-            Self::TreeHeightComparison => Ok(get_tree_height()),
+            Self::TreeHeightComparison => Ok(get_actual_tree_height()),
         }
     }
 }
@@ -150,8 +150,8 @@ fn get_or_key_not_found<'a, T: Debug>(
     })
 }
 
-fn get_tree_height() -> String {
-    TreeHeight::MAX.to_string()
+fn get_actual_tree_height() -> String {
+    SubTreeHeight::ACTUAL_HEIGHT.to_string()
 }
 
 pub(crate) fn example_test(test_args: HashMap<String, String>) -> String {