test(skeleton): tests for Updated_skeletonTree create

Cargo.toml

@@ -20,6 +20,7 @@ derive_more = "0.99.17"
 ethnum = "1.5.0"
 hex = "0.4"
 indexmap = "2.2.6"
+log = "0.4"
 pretty_assertions = "1.2.1"
 rand = "0.8.5"
 rand_distr = "0.4.3"

crates/committer/Cargo.toml

@@ -23,6 +23,7 @@ bisection.workspace = true
 derive_more.workspace = true
 ethnum.workspace = true
 hex.workspace = true
+log.workspace = true
 rand.workspace = true
 serde.workspace = true
 serde_json.workspace = true

crates/committer/src/patricia_merkle_tree.rs

@@ -8,5 +8,5 @@ pub mod updated_skeleton_tree;
 #[cfg(test)]
 pub mod internal_test_utils;
 
-#[cfg(feature = "testing")]
+#[cfg(any(feature = "testing", test))]
 pub mod external_test_utils;

crates/committer/src/patricia_merkle_tree/internal_test_utils.rs

@@ -3,8 +3,10 @@ use crate::patricia_merkle_tree::external_test_utils::get_random_u256;
 
 use crate::patricia_merkle_tree::node_data::inner_node::{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, SubTreeHeight};
+use crate::patricia_merkle_tree::updated_skeleton_tree::node::UpdatedSkeletonNode;
+use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
 
 use ethnum::U256;
 use rand::rngs::ThreadRng;
@@ -65,6 +67,32 @@ fn test_get_random_u256(mut random: ThreadRng, #[case] low: U256, #[case] high:
     assert!(low <= r && r < high);
 }
 
+/// Returns an UpdatedSkeleton instance initialized with the UpdatedSkeletonNodes immediately
+/// derived from the leaf_modifications (as done in UpdatedSkeletonTreeImpl::finalize_bottom_layer).
+pub(crate) fn get_initial_updated_skeleton(
+    original_skeleton: &[(NodeIndex, OriginalSkeletonNode)],
+    leaf_modifications: &[(NodeIndex, u8)],
+) -> UpdatedSkeletonTreeImpl {
+    UpdatedSkeletonTreeImpl {
+        skeleton_tree: leaf_modifications
+            .iter()
+            .filter(|(_, leaf_val)| *leaf_val != 0)
+            .map(|(index, _)| (*index, UpdatedSkeletonNode::Leaf))
+            .chain(
+                original_skeleton
+                    .iter()
+                    .filter_map(|(index, node)| match node {
+                        OriginalSkeletonNode::UnmodifiedSubTree(hash) => {
+                            Some((*index, UpdatedSkeletonNode::UnmodifiedSubTree(*hash)))
+                        }
+                        OriginalSkeletonNode::Binary | OriginalSkeletonNode::Edge(_) => None,
+                    }),
+            )
+            .collect(),
+    }
+}
+
+#[cfg(test)]
 pub(crate) fn as_fully_indexed(
     subtree_height: u8,
     indices: impl Iterator<Item = U256>,

crates/committer/src/patricia_merkle_tree/types.rs

@@ -198,7 +198,7 @@ impl std::ops::Shr<u8> for NodeIndex {
 
 impl From<u128> for NodeIndex {
     fn from(value: u128) -> Self {
-        Self(U256::from(value))
+        Self::new(U256::from(value))
     }
 }
 

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

@@ -1,3 +1,4 @@
+use log::warn;
 use std::collections::HashMap;
 
 use crate::patricia_merkle_tree::node_data::inner_node::EdgePathLength;
@@ -119,13 +120,19 @@ impl UpdatedSkeletonTreeImpl {
         leaf_indices: &[NodeIndex],
     ) -> TempSkeletonNode {
         if root_index.is_leaf() {
-            // Leaf. As this is an empty tree, the leaf must be new.
+            // Leaf. As this is an empty tree, the leaf *should* be new.
             assert!(
-                leaf_indices.len() == 1
-                    && leaf_indices[0] == *root_index
-                    && self.skeleton_tree.contains_key(root_index),
+                leaf_indices.len() == 1 && leaf_indices[0] == *root_index,
                 "Unexpected leaf index (root_index={root_index:?}, leaf_indices={leaf_indices:?})."
             );
+            if !self.skeleton_tree.contains_key(root_index) {
+                // "Deletion" of an already empty leaf. Supported but not expected.
+                warn!(
+                    "Leaf {root_index:?} was not finalized (i.e., a deleted leaf) but is in an
+                empty subtree."
+                );
+                return TempSkeletonNode::Empty;
+            }
             return TempSkeletonNode::Leaf;
         }
 
@@ -283,7 +290,8 @@ impl UpdatedSkeletonTreeImpl {
                     // Leaf is finalized in the initial phase of updated skeleton creation.
                     assert!(
                         self.skeleton_tree.contains_key(bottom_index),
-                        "bottom is a non-empty leaf but doesn't appear in the skeleton."
+                        "bottom {bottom_index:?} is a non-empty leaf but doesn't appear in the \
+                        skeleton."
                     );
                     return TempSkeletonNode::Original(OriginalSkeletonNode::Edge(*path));
                 }