From 24f214e98160e2efedf5c69b544af20bc6039d8a Mon Sep 17 00:00:00 2001 From: nimrod-starkware <143319383+nimrod-starkware@users.noreply.github.com> Date: Tue, 4 Jun 2024 10:43:47 +0300 Subject: [PATCH] refactor: rename TreeHeight => SubTreeHeight and TreeHeight::MAX => TreeHeight::ACTUAL_HEIGHT (#192) --- .../node_data/inner_node.rs | 6 ++-- .../original_skeleton_tree/create_tree.rs | 8 ++--- .../create_tree_test.rs | 31 ++++++++++--------- .../skeleton_forest_test.rs | 10 +++--- .../original_skeleton_tree/utils.rs | 6 ++-- .../original_skeleton_tree/utils_test.rs | 6 ++-- .../src/patricia_merkle_tree/test_utils.rs | 10 +++--- .../src/patricia_merkle_tree/types.rs | 15 +++++---- .../compute_updated_skeleton_tree_test.rs | 8 ++--- .../committer_cli/src/tests/python_tests.rs | 8 ++--- 10 files changed, 55 insertions(+), 53 deletions(-) diff --git a/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs b/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs index 06b69e5b..9df32831 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/inner_node.rs +++ b/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 { if length > Self::MAX.0 { diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs index c0ff6133..f91a72b3 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree.rs +++ b/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; diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs index 7f406731..9697aecf 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/create_tree_test.rs +++ b/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, #[case] root_hash: HashOutput, #[case] expected_skeleton: OriginalSkeletonTreeImpl, - #[case] tree_height: TreeHeight, + #[case] subtree_height: SubTreeHeight, ) { let mut sorted_leaf_indices: Vec = 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, diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs index 65e671e3..139bec40 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/skeleton_forest_test.rs +++ b/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), diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs index 3de23f4a..ed25c0cf 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils.rs +++ b/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 diff --git a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs b/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs index afb691ed..2b3d552c 100644 --- a/crates/committer/src/patricia_merkle_tree/original_skeleton_tree/utils_test.rs +++ b/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 diff --git a/crates/committer/src/patricia_merkle_tree/test_utils.rs b/crates/committer/src/patricia_merkle_tree/test_utils.rs index 9958d96c..4eec3c0d 100644 --- a/crates/committer/src/patricia_merkle_tree/test_utils.rs +++ b/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, ) -> Vec { 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() } diff --git a/crates/committer/src/patricia_merkle_tree/types.rs b/crates/committer/src/patricia_merkle_tree/types.rs index 459d9332..699d2801 100644 --- a/crates/committer/src/patricia_merkle_tree/types.rs +++ b/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 for u8 { - fn from(value: TreeHeight) -> Self { +impl From 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); diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree_test.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree_test.rs index 27dd761c..c8e994c6 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree_test.rs +++ b/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, #[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, ) { - 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, ) -> Vec { indices - .map(|index| NodeIndex::from_subtree_index(index, TreeHeight::new(subtree_height))) + .map(|index| NodeIndex::from_subtree_index(index, SubTreeHeight::new(subtree_height))) .collect() } diff --git a/crates/committer_cli/src/tests/python_tests.rs b/crates/committer_cli/src/tests/python_tests.rs index 38cc2577..ce158966 100644 --- a/crates/committer_cli/src/tests/python_tests.rs +++ b/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 {