chore(skeleton): hide private field in EdgePathLength

crates/committer/src/patricia_merkle_tree/filled_tree/node_serde.rs

@@ -66,7 +66,7 @@ impl<L: LeafData> DBObject for FilledNode<L> {
                 let bottom: [u8; SERIALIZE_HASH_BYTES] = bottom_hash.0.to_bytes_be();
                 let path: [u8; SERIALIZE_HASH_BYTES] =
                     U256::from(&path_to_bottom.path).to_be_bytes();
-                let length: [u8; 1] = path_to_bottom.length.0.to_be_bytes();
+                let length: [u8; 1] = u8::from(path_to_bottom.length).to_be_bytes();
 
                 // Concatenate bottom hash, path, and path length.
                 let serialized = [bottom.to_vec(), path.to_vec(), length.to_vec()].concat();
@@ -116,7 +116,7 @@ impl Deserializable for OriginalSkeletonInputNode {
                             .expect("Slice with incorrect length."),
                     )
                     .into(),
-                    length: EdgePathLength(value.0[EDGE_BYTES - 1]),
+                    length: EdgePathLength::new(value.0[EDGE_BYTES - 1])?,
                 },
             }));
         } else {

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

@@ -257,7 +257,7 @@ fn create_path_to_bottom_edge_updated_skeleton_node_for_testing(
         NodeIndex::from(index),
         UpdatedSkeletonNode::Edge(PathToBottom {
             path: path.into(),
-            length: EdgePathLength(length),
+            length: EdgePathLength::new(length).unwrap(),
         }),
     )
 }
@@ -309,7 +309,7 @@ fn create_edge_entry_for_testing(
                 bottom_hash: HashOutput(Felt::from_hex(bottom_hash).unwrap()),
                 path_to_bottom: PathToBottom {
                     path: path.into(),
-                    length: EdgePathLength(length),
+                    length: EdgePathLength::new(length).unwrap(),
                 },
             }),
         },

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

@@ -13,6 +13,12 @@ pub enum PathToBottomError {
     },
 }
 
+#[derive(Debug, Error)]
+pub enum EdgePathError {
+    #[error("Length {length:?} is not in range [0, EdgePathLength::MAX]")]
+    IllegalLength { length: u8 },
+}
+
 #[derive(Debug, Error)]
 pub enum LeafError {
     #[error("Missing modification data at index {0:?}.")]

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

@@ -1,6 +1,6 @@
 use crate::felt::Felt;
 use crate::hash::hash_trait::HashOutput;
-use crate::patricia_merkle_tree::node_data::errors::PathToBottomError;
+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};
 
@@ -65,13 +65,33 @@ impl From<&EdgePath> for U256 {
 #[derive(
     Clone, Copy, Debug, Default, PartialOrd, derive_more::Add, derive_more::Sub, PartialEq, Eq, Hash,
 )]
-pub struct EdgePathLength(pub u8);
+pub struct EdgePathLength(u8);
 
 impl EdgePathLength {
     /// [EdgePathLength] constant that represents the longest path (from some node) in a tree.
-    #[allow(clippy::as_conversions)]
+    pub const ONE: Self = Self(1);
     pub const MAX: Self = Self(TreeHeight::MAX.0);
+
+    pub fn new(length: u8) -> Result<Self, EdgePathError> {
+        if length > Self::MAX.0 {
+            return Err(EdgePathError::IllegalLength { length });
+        }
+        Ok(Self(length))
+    }
 }
+
+impl From<EdgePathLength> for u8 {
+    fn from(value: EdgePathLength) -> Self {
+        value.0
+    }
+}
+
+impl From<EdgePathLength> for Felt {
+    fn from(value: EdgePathLength) -> Self {
+        value.0.into()
+    }
+}
+
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)]
 pub struct PathToBottom {
     pub path: EdgePath,
@@ -85,13 +105,11 @@ pub struct EdgeData {
 }
 
 impl PathToBottom {
-    #[allow(dead_code)]
     pub(crate) const LEFT_CHILD: Self = Self {
         path: EdgePath(U256::ZERO),
         length: EdgePathLength(1),
     };
 
-    #[allow(dead_code)]
     pub(crate) const RIGHT_CHILD: Self = Self {
         path: EdgePath(U256::ONE),
         length: EdgePathLength(1),

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

@@ -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.0);
+        let bottom_height = self.get_height() - TreeHeight::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

@@ -269,7 +269,7 @@ pub(crate) fn create_expected_skeleton(
                 NodeIndex::ROOT,
                 OriginalSkeletonNode::Edge(PathToBottom {
                     path: 0.into(),
-                    length: EdgePathLength(251 - height),
+                    length: EdgePathLength::new(251 - height).unwrap(),
                 }),
             )])
             .collect(),
@@ -289,7 +289,7 @@ pub(crate) fn create_edge_skeleton_node(
         NodeIndex::from(idx),
         OriginalSkeletonNode::Edge(PathToBottom {
             path: path.into(),
-            length: EdgePathLength(length),
+            length: EdgePathLength::new(length).unwrap(),
         }),
     )
 }

crates/committer/src/patricia_merkle_tree/test_utils.rs

@@ -19,11 +19,12 @@ impl From<&str> for PathToBottom {
             path: U256::from_str_radix(value, 2)
                 .expect("Invalid binary string")
                 .into(),
-            length: EdgePathLength(
+            length: EdgePathLength::new(
                 (value.len() - if value.starts_with('+') { 1 } else { 0 })
                     .try_into()
                     .expect("String is too large"),
-            ),
+            )
+            .expect("Invalid length"),
         }
     }
 }

crates/committer/src/patricia_merkle_tree/types.rs

@@ -70,7 +70,7 @@ impl NodeIndex {
         path_to_bottom: &PathToBottom,
     ) -> NodeIndex {
         let PathToBottom { path, length } = path_to_bottom;
-        (index << length.0) + Self::new(path.into())
+        (index << u8::from(*length)) + Self::new(path.into())
     }
 
     pub(crate) fn get_children_indices(&self) -> [Self; 2] {
@@ -111,7 +111,7 @@ impl NodeIndex {
         NodeIndex::new(lca)
     }
 
-    /// Returns the path from the node to its given descendant.
+    /// Returns the path from the node to its given descendant (0 length if node == descendant).
     /// Panics if the supposed descendant is not really a descendant.
     pub(crate) fn get_path_to_descendant(&self, descendant: Self) -> PathToBottom {
         let descendant_bit_length = descendant.bit_length();
@@ -128,7 +128,7 @@ impl NodeIndex {
 
         PathToBottom {
             path: delta.0.into(),
-            length: EdgePathLength(distance),
+            length: EdgePathLength::new(distance).expect("Illegal length"),
         }
     }
 

crates/committer/src/patricia_merkle_tree/types_test.rs

@@ -25,7 +25,7 @@ fn test_compute_bottom_index(
         NodeIndex::from(node_index),
         &PathToBottom {
             path: path.into(),
-            length: EdgePathLength(length),
+            length: EdgePathLength::new(length).unwrap(),
         },
     );
     let expected = NodeIndex::from(expected);
@@ -114,7 +114,10 @@ fn test_get_path_to_descendant(
     let descendant = NodeIndex::new(descendant.into());
     let path_to_bottom = root_index.get_path_to_descendant(descendant);
     assert_eq!(path_to_bottom.path, U256::from(expected_path).into());
-    assert_eq!(path_to_bottom.length, EdgePathLength(expected_length));
+    assert_eq!(
+        path_to_bottom.length,
+        EdgePathLength::new(expected_length).unwrap()
+    );
 }
 
 #[rstest]
@@ -129,7 +132,7 @@ fn test_get_path_to_descendant_big() {
     assert_eq!(path_to_bottom.path, extension.into());
     assert_eq!(
         path_to_bottom.length,
-        EdgePathLength(extension_index.bit_length())
+        EdgePathLength::new(extension_index.bit_length()).unwrap()
     );
 }
 

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

@@ -37,6 +37,7 @@ impl TempSkeletonNode {
 /// * Sorted.
 /// * Descendants of the given index.
 /// * A non-empty array.
+/// Note that the if the LCA is the root, the path will be empty (0 length).
 fn get_path_to_lca(root_index: &NodeIndex, subtree_indices: &[NodeIndex]) -> PathToBottom {
     if subtree_indices.is_empty() {
         panic!("Unexpected empty array.");
@@ -352,13 +353,13 @@ impl UpdatedSkeletonTreeImpl {
 
             // 1. Handle the originally non-empty subtree, replacing the root with the child in the
             //    direction of the edge.
-            if path_to_bottom.length.0 > 1 {
+            if u8::from(path_to_bottom.length) > 1 {
                 // Bottom is not a child of the root, removing the first edge returns a valid new
                 // edge node. Inject the new node to the original skeleton as if it was in it
                 // originally (fake original).
                 let fake_original_child_node = OriginalSkeletonNode::Edge(
                     path_to_bottom
-                        .remove_first_edges(EdgePathLength(1))
+                        .remove_first_edges(EdgePathLength::ONE)
                         .expect("Original Edge node is unexpectedly trivial"),
                 );
                 original_skeleton.insert(nonempty_subtree_child_index, fake_original_child_node);

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

@@ -96,28 +96,30 @@ fn test_has_leaves_on_both_sides_assertions(
 
 #[rstest]
 #[case::small_tree_single_leaf(
-    1, vec![U256::from(8_u8)], PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(3)}
+    1,
+    vec![U256::from(8_u8)],
+    PathToBottom {path: U256::ZERO.into(), length: EdgePathLength::new(3).unwrap()}
 )]
 #[case::small_tree_few_leaves(
     1,
     vec![
         U256::from(12_u8), U256::from(13_u8), U256::from(14_u8)
     ],
-    PathToBottom {path:U256::ONE.into(), length:EdgePathLength(1)}
+    PathToBottom {path:U256::ONE.into(), length:EdgePathLength::ONE}
 )]
 #[case::small_tree_few_leaves2(
     1,
     vec![U256::from(12_u8),U256::from(13_u8)],
-    PathToBottom {path:2_u128.into(), length:EdgePathLength(2)}
+    PathToBottom {path:2_u128.into(), length: EdgePathLength::new(2).unwrap()}
 )]
 #[case::large_tree_positive_consecutive_indices_of_different_sides(
     1,
     vec![(U256::from(3u8) << 250) - U256::ONE, U256::from(3u8) << 250],
-    PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(0)})]
+    PathToBottom {path:U256::ZERO.into(), length: EdgePathLength::new(0).unwrap()})]
 #[case::large_tree_positive_consecutive_indices(
     3<<126,
     vec![U256::from(3u8) << 250, (U256::from(3u8) << 250)+ U256::ONE],
-    PathToBottom {path:U256::ZERO.into(), length:EdgePathLength(123)})]
+    PathToBottom {path:U256::ZERO.into(), length: EdgePathLength::new(123).unwrap()})]
 fn test_get_path_to_lca(
     #[case] root_index: u128,
     #[case] leaf_indices: Vec<U256>,

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

@@ -43,7 +43,7 @@ impl TreeHashFunction<LeafDataImpl> for TreeHashFunctionImpl {
                 Felt::from(Pedersen::hash(
                     &hash_output.0.into(),
                     &Felt::from(path).into(),
-                )) + length.0.into()
+                )) + (*length).into()
             }
             NodeData::Leaf(LeafDataImpl::StorageValue(storage_value)) => *storage_value,
             NodeData::Leaf(LeafDataImpl::CompiledClassHash(compiled_class_hash)) => {

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

@@ -47,7 +47,7 @@ fn test_tree_hash_function_impl_edge_node(
         bottom_hash: HashOutput(bottom_hash),
         path_to_bottom: PathToBottom {
             path: edge_path.into(),
-            length: EdgePathLength(length),
+            length: EdgePathLength::new(length).unwrap(),
         },
     }));
     let direct_hash_computation = HashOutput(

crates/committer/src/storage/errors.rs

@@ -1,4 +1,6 @@
+use crate::patricia_merkle_tree::node_data::errors::EdgePathError;
 use crate::storage::storage_trait::StorageKey;
+
 use serde_json;
 use thiserror::Error;
 
@@ -20,4 +22,6 @@ pub enum DeserializationError {
     KeyDuplicate(String),
     #[error("Couldn't read and parse the given input JSON: {0}")]
     ParsingError(#[from] serde_json::Error),
+    #[error("Illegal EdgePath: {0}")]
+    EdgePathError(#[from] EdgePathError),
 }

crates/committer_cli/src/tests/python_tests.rs

@@ -538,7 +538,10 @@ fn test_storage_node(data: HashMap<String, String>) -> Result<String, PythonTest
             bottom_hash: HashOutput(Felt::from(*get_or_key_not_found(&edge_data, "bottom")?)),
             path_to_bottom: PathToBottom {
                 path: U256::from(*get_or_key_not_found(&edge_data, "path")?).into(),
-                length: EdgePathLength((*get_or_key_not_found(&edge_data, "length")?).try_into()?),
+                length: EdgePathLength::new(
+                    (*get_or_key_not_found(&edge_data, "length")?).try_into()?,
+                )
+                .expect("Invalid length"),
             },
         }),
         hash: HashOutput(Felt::from(*get_or_key_not_found(&edge_data, "hash")?)),

crates/committer_cli/src/tests/utils/random_structs.rs

@@ -98,7 +98,7 @@ impl RandomValue for PathToBottom {
 
         Self {
             path,
-            length: EdgePathLength(length),
+            length: EdgePathLength::new(length).expect("Invalid length"),
         }
     }
 }