diff --git a/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs b/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs index d0df347e..1fd8256d 100644 --- a/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs +++ b/crates/committer/src/patricia_merkle_tree/filled_tree/tree.rs @@ -177,7 +177,7 @@ impl FilledTreeImpl { if skeleton_leaf.is_zero() != leaf_data.is_empty() { return Err(FilledTreeError::::InconsistentModification { index, - skeleton_leaf: skeleton_leaf.clone(), + skeleton_leaf: *skeleton_leaf, }); } let node_data = NodeData::Leaf(leaf_data); 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 dc428e83..0600237b 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 @@ -20,7 +20,7 @@ pub struct BinaryData { #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] pub struct EdgePath(pub Felt); -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, Debug, Default, derive_more::Add, PartialEq, Eq, Hash)] pub struct EdgePathLength(pub u8); #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] @@ -36,7 +36,26 @@ pub struct EdgeData { } impl PathToBottom { + #[allow(dead_code)] + pub(crate) const LEFT_CHILD: Self = Self { + path: EdgePath(Felt::ZERO), + length: EdgePathLength(1), + }; + + #[allow(dead_code)] + pub(crate) const RIGHT_CHILD: Self = Self { + path: EdgePath(Felt::ONE), + length: EdgePathLength(1), + }; + pub(crate) fn bottom_index(&self, root_index: NodeIndex) -> NodeIndex { NodeIndex::compute_bottom_index(root_index, self) } + + pub(crate) fn concat_paths(&self, other: Self) -> Self { + Self { + path: EdgePath((self.path.0 * Felt::TWO.pow(other.length.0)) + other.path.0), + length: self.length + other.length, + } + } } diff --git a/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs b/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs index c93205dd..6064e0ba 100644 --- a/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs +++ b/crates/committer/src/patricia_merkle_tree/node_data/leaf.rs @@ -42,7 +42,7 @@ impl LeafData for LeafDataImpl { } #[allow(dead_code)] -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum SkeletonLeaf { Zero, NonZero, diff --git a/crates/committer/src/patricia_merkle_tree/test_utils.rs b/crates/committer/src/patricia_merkle_tree/test_utils.rs index b3e1d55f..c9968b7d 100644 --- a/crates/committer/src/patricia_merkle_tree/test_utils.rs +++ b/crates/committer/src/patricia_merkle_tree/test_utils.rs @@ -2,6 +2,23 @@ use ethnum::U256; use rand::Rng; use rstest::rstest; +use crate::felt::Felt; +use crate::patricia_merkle_tree::node_data::inner_node::{EdgePath, EdgePathLength, PathToBottom}; + +impl From<&str> for PathToBottom { + fn from(value: &str) -> Self { + Self { + path: EdgePath(Felt::from( + u128::from_str_radix(value, 2).expect("Invalid binary string"), + )), + length: EdgePathLength( + (value.len() - if value.starts_with('+') { 1 } else { 0 }) + .try_into() + .expect("String is too large"), + ), + } + } +} /// Generates a random U256 number between low and high (exclusive). /// Panics if low > high. pub(crate) fn get_random_u256(low: U256, high: U256) -> U256 { diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree.rs index 4f67de70..6e59484f 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/compute_updated_skeleton_tree.rs @@ -1,12 +1,26 @@ +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::node_data::leaf::LeafModifications; +use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf; +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::updated_skeleton_tree::node::UpdatedSkeletonNode; +use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl; #[cfg(test)] #[path = "compute_updated_skeleton_tree_test.rs"] pub mod compute_updated_skeleton_tree_test; +#[derive(Debug, PartialEq, Eq)] +/// A temporary skeleton node used to during the computation of the updated skeleton tree. +enum TempSkeletonNode { + Empty, + #[allow(dead_code)] + Original(OriginalSkeletonNode), +} + impl OriginalSkeletonTreeImpl { #[allow(dead_code)] /// Returns the path from the given root_index to the LCA of the leaves. Assumes the leaves are: @@ -38,3 +52,65 @@ impl OriginalSkeletonTreeImpl { .all(|leaves_in_side| !leaves_in_side.is_empty()) } } + +impl UpdatedSkeletonTreeImpl { + #[allow(dead_code)] + /// Builds a (probably edge) node from its given descendant. Returns the TempSkeletonNode + /// matching the given root (the source for the path to bottom) for the subtree it is the root + /// of. If bottom is empty, returns an empty node. + fn node_from_edge_data( + &mut self, + path: &PathToBottom, + bottom_index: &NodeIndex, + bottom: &TempSkeletonNode, + leaf_modifications: &LeafModifications, + ) -> TempSkeletonNode { + TempSkeletonNode::Original(match bottom { + TempSkeletonNode::Empty => return TempSkeletonNode::Empty, + TempSkeletonNode::Original(OriginalSkeletonNode::Leaf(_)) => { + let leaf = leaf_modifications + .get(bottom_index) + .unwrap_or_else(|| panic!("Leaf modification {bottom_index:?} not found")); + match leaf { + SkeletonLeaf::Zero => { + return TempSkeletonNode::Empty; + } + SkeletonLeaf::NonZero => { + // TODO(Tzahi, 1/6/2024): Consider inserting all modification + //leaves at one go and remove this. + // Finalize bottom leaf node (may happen multiple times) + self.skeleton_tree + .insert(*bottom_index, UpdatedSkeletonNode::Leaf(*leaf)); + OriginalSkeletonNode::Edge { + path_to_bottom: *path, + } + } + } + } + TempSkeletonNode::Original(OriginalSkeletonNode::Edge { path_to_bottom }) => { + OriginalSkeletonNode::Edge { + path_to_bottom: path.concat_paths(*path_to_bottom), + } + } + TempSkeletonNode::Original(OriginalSkeletonNode::EdgeSibling(edge_data)) => { + OriginalSkeletonNode::EdgeSibling(EdgeData { + bottom_hash: edge_data.bottom_hash, + path_to_bottom: path.concat_paths(edge_data.path_to_bottom), + }) + } + TempSkeletonNode::Original(OriginalSkeletonNode::Binary) => { + // Finalize bottom - a binary descendant cannot change form. + self.skeleton_tree + .insert(*bottom_index, UpdatedSkeletonNode::Binary); + OriginalSkeletonNode::Edge { + path_to_bottom: *path, + } + } + TempSkeletonNode::Original(OriginalSkeletonNode::LeafOrBinarySibling(_)) => { + OriginalSkeletonNode::Edge { + path_to_bottom: *path, + } + } + }) + } +} 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 82854788..ba7fdd1a 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 @@ -1,11 +1,18 @@ -use crate::felt::Felt; -use crate::patricia_merkle_tree::node_data::inner_node::{EdgePath, EdgePathLength, PathToBottom}; -use crate::patricia_merkle_tree::types::NodeIndex; -use std::collections::HashMap; - use ethnum::U256; -use rstest::rstest; +use rstest::{fixture, rstest}; +use std::collections::HashMap; +use crate::felt::Felt; +use crate::hash::hash_trait::HashOutput; +use crate::patricia_merkle_tree::node_data::inner_node::{ + EdgeData, EdgePath, 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::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, }; @@ -17,6 +24,13 @@ fn empty_skeleton(height: u8) -> OriginalSkeletonTreeImpl { } } +#[fixture] +fn updated_skeleton() -> UpdatedSkeletonTreeImpl { + UpdatedSkeletonTreeImpl { + skeleton_tree: HashMap::new(), + } +} + #[rstest] #[case::small_tree_positive( 3, 2, vec![NodeIndex::from(8),NodeIndex::from(10),NodeIndex::from(11)], true)] @@ -99,3 +113,83 @@ fn test_get_path_to_lca( expected ); } + +#[rstest] +#[case::to_empty( + &PathToBottom::LEFT_CHILD, + &NodeIndex::ROOT, + &TempSkeletonNode::Empty, + &[], + TempSkeletonNode::Empty, + &[], +)] +#[case::to_edge( + &PathToBottom::from("00"), + &NodeIndex::from(4), + &TempSkeletonNode::Original( + OriginalSkeletonNode::Edge {path_to_bottom: PathToBottom::from("11")} + ), + &[], + TempSkeletonNode::Original( + OriginalSkeletonNode::Edge { path_to_bottom: (PathToBottom::from("0011")) } + ), + &[], +)] +#[case::to_edge_sibling( + &PathToBottom::RIGHT_CHILD, + &NodeIndex::from(5), + &TempSkeletonNode::Original(OriginalSkeletonNode::EdgeSibling( + EdgeData {bottom_hash: HashOutput::ZERO, path_to_bottom: PathToBottom::from("01")} + )), + &[], + TempSkeletonNode::Original(OriginalSkeletonNode::EdgeSibling( + EdgeData {bottom_hash: HashOutput::ZERO, path_to_bottom: (PathToBottom::from("101"))} + )), + &[], +)] +#[case::to_binary( + &PathToBottom::RIGHT_CHILD, + &NodeIndex::from(7), + &TempSkeletonNode::Original(OriginalSkeletonNode::Binary), + &[], + TempSkeletonNode::Original( + OriginalSkeletonNode::Edge {path_to_bottom: PathToBottom::RIGHT_CHILD} + ), + &[(NodeIndex::from(7), UpdatedSkeletonNode::Binary)] +)] +#[case::to_empty_leaf( + &PathToBottom::RIGHT_CHILD, + &NodeIndex::from(7), + &TempSkeletonNode::Original(OriginalSkeletonNode::Leaf(SkeletonLeaf::NonZero)), + &[(NodeIndex::from(7),SkeletonLeaf::Zero)], + TempSkeletonNode::Empty, + &[], +)] +#[case::to_non_empty_leaf( + &PathToBottom::RIGHT_CHILD, + &NodeIndex::from(7), + &TempSkeletonNode::Original(OriginalSkeletonNode::Leaf(SkeletonLeaf::NonZero)), + &[(NodeIndex::from(7), SkeletonLeaf::NonZero)], + TempSkeletonNode::Original( + OriginalSkeletonNode::Edge {path_to_bottom: PathToBottom::RIGHT_CHILD} + ), + &[(NodeIndex::from(7), UpdatedSkeletonNode::Leaf(SkeletonLeaf::NonZero))] +)] +fn test_node_from_edge_data( + mut updated_skeleton: UpdatedSkeletonTreeImpl, + #[case] path: &PathToBottom, + #[case] bottom_index: &NodeIndex, + #[case] bottom: &TempSkeletonNode, + #[case] leaf_modifications: &[(NodeIndex, SkeletonLeaf)], + #[case] expected_node: TempSkeletonNode, + #[case] expected_skeleton_additions: &[(NodeIndex, UpdatedSkeletonNode)], +) { + let mut expected_skeleton_tree = updated_skeleton.skeleton_tree.clone(); + expected_skeleton_tree.extend(expected_skeleton_additions.iter().cloned()); + let leaf_modifications: HashMap = + leaf_modifications.iter().cloned().collect(); + let temp_node = + updated_skeleton.node_from_edge_data(path, bottom_index, bottom, &leaf_modifications); + assert_eq!(temp_node, expected_node); + assert_eq!(updated_skeleton.skeleton_tree, expected_skeleton_tree); +} diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs index 00ab3d4f..448727ad 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/node.rs @@ -4,7 +4,7 @@ use crate::patricia_merkle_tree::node_data::leaf::SkeletonLeaf; #[allow(dead_code)] /// A node in the structure of a Patricia-Merkle tree, after the update. -#[derive(Clone)] +#[derive(Debug, Clone, PartialEq)] pub(crate) enum UpdatedSkeletonNode { Binary, Edge { path_to_bottom: PathToBottom }, diff --git a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs index d5d1ea94..26ae2741 100644 --- a/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs +++ b/crates/committer/src/patricia_merkle_tree/updated_skeleton_tree/tree.rs @@ -19,7 +19,7 @@ pub(crate) trait UpdatedSkeletonTree: Sized + Send + Sync { #[allow(dead_code)] fn create( original_skeleton: impl OriginalSkeletonTree, - index_to_updated_leaf: &LeafModifications, + leaf_modifications: &LeafModifications, ) -> Result; /// Does the skeleton represents an empty-tree (i.e. all leaves are empty). @@ -42,7 +42,7 @@ pub(crate) struct UpdatedSkeletonTreeImpl { impl UpdatedSkeletonTree for UpdatedSkeletonTreeImpl { fn create( _original_skeleton: impl OriginalSkeletonTree, - _index_to_updated_leaf: &LeafModifications, + _leaf_modifications: &LeafModifications, ) -> Result { todo!() }