starknet_committer,starknet_patricia: remove generic config (#10683)

Author: ArielElp

crates/starknet_committer/src/db/external_test_utils.rs

@@ -25,7 +25,7 @@ pub async fn tree_computation_flow<L, TH>(
     leaf_modifications: LeafModifications<L>,
     storage: &mut MapStorage,
     root_hash: HashOutput,
-    config: impl OriginalSkeletonTreeConfig<L>,
+    config: impl OriginalSkeletonTreeConfig,
 ) -> FilledTreeImpl<L>
 where
     TH: TreeHashFunction<L> + 'static,
@@ -73,7 +73,7 @@ pub async fn single_tree_flow_test<
     leaf_modifications: LeafModifications<L>,
     storage: &mut MapStorage,
     root_hash: HashOutput,
-    config: impl OriginalSkeletonTreeConfig<L>,
+    config: impl OriginalSkeletonTreeConfig,
 ) -> String {
     // Move from leaf number to actual index.
     let leaf_modifications = leaf_modifications

crates/starknet_committer/src/db/facts_db/create_facts_tree.rs

@@ -10,10 +10,7 @@ use starknet_patricia::patricia_merkle_tree::node_data::inner_node::{
     NodeData,
 };
 use starknet_patricia::patricia_merkle_tree::node_data::leaf::{Leaf, LeafModifications};
-use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::config::{
-    NoCompareOriginalSkeletonTrieConfig,
-    OriginalSkeletonTreeConfig,
-};
+use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig;
 use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode;
 use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::tree::{
     OriginalSkeletonTreeImpl,
@@ -29,6 +26,7 @@ use crate::db::db_layout::NodeLayout;
 use crate::db::facts_db::db::FactsNodeLayout;
 use crate::db::facts_db::traversal::get_roots_from_storage;
 use crate::db::facts_db::types::FactsSubTree;
+use crate::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 
 #[cfg(test)]
 #[path = "create_facts_tree_test.rs"]
@@ -54,7 +52,7 @@ async fn fetch_nodes<'a, L, Layout>(
     subtrees: Vec<Layout::SubTree>,
     storage: &mut impl Storage,
     leaf_modifications: &LeafModifications<L>,
-    config: &impl OriginalSkeletonTreeConfig<L>,
+    config: &impl OriginalSkeletonTreeConfig,
     mut previous_leaves: Option<&mut HashMap<NodeIndex, L>>,
     key_context: &<L as HasStaticPrefix>::KeyContext,
 ) -> OriginalSkeletonTreeResult<()>
@@ -155,7 +153,7 @@ pub async fn create_original_skeleton_tree<'a, L: Leaf>(
     storage: &mut impl Storage,
     root_hash: HashOutput,
     sorted_leaf_indices: SortedLeafIndices<'a>,
-    config: &impl OriginalSkeletonTreeConfig<L>,
+    config: &impl OriginalSkeletonTreeConfig,
     leaf_modifications: &LeafModifications<L>,
     key_context: &<L as HasStaticPrefix>::KeyContext,
 ) -> OriginalSkeletonTreeResult<OriginalSkeletonTreeImpl<'a>> {
@@ -190,7 +188,7 @@ pub async fn create_original_skeleton_tree_and_get_previous_leaves<'a, L: Leaf>(
     root_hash: HashOutput,
     sorted_leaf_indices: SortedLeafIndices<'a>,
     leaf_modifications: &LeafModifications<L>,
-    config: &impl OriginalSkeletonTreeConfig<L>,
+    config: &impl OriginalSkeletonTreeConfig,
     key_context: &<L as HasStaticPrefix>::KeyContext,
 ) -> OriginalSkeletonTreeResult<(OriginalSkeletonTreeImpl<'a>, HashMap<NodeIndex, L>)> {
     if sorted_leaf_indices.is_empty() {
@@ -228,7 +226,7 @@ pub async fn get_leaves<'a, L: Leaf>(
     sorted_leaf_indices: SortedLeafIndices<'a>,
     key_context: &<L as HasStaticPrefix>::KeyContext,
 ) -> OriginalSkeletonTreeResult<HashMap<NodeIndex, L>> {
-    let config = NoCompareOriginalSkeletonTrieConfig::default();
+    let config = OriginalSkeletonTrieConfig::default();
     let leaf_modifications = LeafModifications::new();
     let (_, previous_leaves) = create_original_skeleton_tree_and_get_previous_leaves(
         storage,
@@ -331,7 +329,7 @@ fn handle_child_subtree<'a, SubTree: SubTreeTrait<'a>>(
 fn log_warning_for_empty_leaves<L: Leaf, T: Borrow<NodeIndex> + Debug>(
     leaf_indices: &[T],
     leaf_modifications: &LeafModifications<L>,
-    config: &impl OriginalSkeletonTreeConfig<L>,
+    config: &impl OriginalSkeletonTreeConfig,
 ) -> OriginalSkeletonTreeResult<()> {
     if !config.compare_modified_leaves() {
         return Ok(());

crates/starknet_committer/src/db/facts_db/create_facts_tree_test.rs

@@ -13,7 +13,6 @@ use starknet_patricia::patricia_merkle_tree::external_test_utils::{
     create_unmodified_subtree_skeleton_node,
     AdditionHash,
     MockLeaf,
-    OriginalSkeletonMockTrieConfig,
 };
 use starknet_patricia::patricia_merkle_tree::node_data::leaf::LeafModifications;
 use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::node::OriginalSkeletonNode;
@@ -24,6 +23,7 @@ use starknet_patricia_storage::storage_trait::{DbHashMap, DbKey, DbValue};
 use starknet_types_core::felt::Felt;
 
 use crate::db::facts_db::create_facts_tree::create_original_skeleton_tree;
+use crate::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 
 #[tokio::test]
 #[rstest]
@@ -208,7 +208,7 @@ async fn test_create_tree(
         .into_iter()
         .map(|(idx, leaf)| (NodeIndex::from_subtree_index(idx, subtree_height), leaf))
         .collect();
-    let config = OriginalSkeletonMockTrieConfig::new(compare_modified_leaves);
+    let config = OriginalSkeletonTrieConfig::new_for_tests(compare_modified_leaves);
     let mut sorted_leaf_indices: Vec<NodeIndex> = leaf_modifications.keys().copied().collect();
     let sorted_leaf_indices = SortedLeafIndices::new(&mut sorted_leaf_indices);
     let skeleton_tree = create_original_skeleton_tree(

crates/starknet_committer/src/db/facts_db/db.rs

@@ -29,11 +29,7 @@ use crate::forest::filled_forest::FilledForest;
 use crate::forest::forest_errors::{ForestError, ForestResult};
 use crate::forest::original_skeleton_forest::{ForestSortedIndices, OriginalSkeletonForest};
 use crate::patricia_merkle_tree::leaf::leaf_impl::ContractState;
-use crate::patricia_merkle_tree::tree::{
-    OriginalSkeletonClassesTrieConfig,
-    OriginalSkeletonContractsTrieConfig,
-    OriginalSkeletonStorageTrieConfig,
-};
+use crate::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 use crate::patricia_merkle_tree::types::CompiledClassHash;
 
 /// Facts DB node layout.
@@ -74,7 +70,7 @@ impl<S: Storage> FactsDb<S> {
             contracts_trie_root_hash,
             contracts_trie_sorted_indices,
             &HashMap::new(),
-            &OriginalSkeletonContractsTrieConfig::new(),
+            &OriginalSkeletonTrieConfig::new_for_contracts_trie(),
             &EmptyKeyContext,
         )
         .await?)
@@ -95,8 +91,9 @@ impl<S: Storage> FactsDb<S> {
             let contract_state = original_contracts_trie_leaves
                 .get(&contract_address_into_node_index(address))
                 .ok_or(ForestError::MissingContractCurrentState(*address))?;
-            let config =
-                OriginalSkeletonStorageTrieConfig::new(config.warn_on_trivial_modifications());
+            let config = OriginalSkeletonTrieConfig::new_for_classes_or_storage_trie(
+                config.warn_on_trivial_modifications(),
+            );
 
             let original_skeleton = create_original_skeleton_tree(
                 &mut self.storage,
@@ -119,7 +116,9 @@ impl<S: Storage> FactsDb<S> {
         config: &ReaderConfig,
         contracts_trie_sorted_indices: SortedLeafIndices<'a>,
     ) -> ForestResult<OriginalSkeletonTreeImpl<'a>> {
-        let config = OriginalSkeletonClassesTrieConfig::new(config.warn_on_trivial_modifications());
+        let config = OriginalSkeletonTrieConfig::new_for_classes_or_storage_trie(
+            config.warn_on_trivial_modifications(),
+        );
 
         Ok(create_original_skeleton_tree(
             &mut self.storage,

crates/starknet_committer/src/patricia_merkle_tree/tree.rs

@@ -2,7 +2,6 @@ use std::collections::HashMap;
 
 use starknet_api::core::{ClassHash, ContractAddress};
 use starknet_api::hash::HashOutput;
-use starknet_patricia::generate_trie_config;
 use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig;
 use starknet_patricia::patricia_merkle_tree::traversal::TraversalResult;
 use starknet_patricia::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices};
@@ -24,21 +23,30 @@ use crate::patricia_merkle_tree::types::{
     RootHashes,
     StarknetForestProofs,
 };
-generate_trie_config!(OriginalSkeletonStorageTrieConfig, StarknetStorageValue);
 
-generate_trie_config!(OriginalSkeletonClassesTrieConfig, CompiledClassHash);
+#[derive(Default)]
+pub struct OriginalSkeletonTrieConfig {
+    compare_modified_leaves: bool,
+}
 
-pub(crate) struct OriginalSkeletonContractsTrieConfig;
+impl OriginalSkeletonTrieConfig {
+    pub fn new_for_contracts_trie() -> Self {
+        Self { compare_modified_leaves: false }
+    }
 
-impl OriginalSkeletonTreeConfig<ContractState> for OriginalSkeletonContractsTrieConfig {
-    fn compare_modified_leaves(&self) -> bool {
-        false
+    pub fn new_for_classes_or_storage_trie(warn_on_trivial_modifications: bool) -> Self {
+        Self { compare_modified_leaves: warn_on_trivial_modifications }
+    }
+
+    #[cfg(test)]
+    pub(crate) fn new_for_tests(should_compare_modified_leaves: bool) -> Self {
+        Self { compare_modified_leaves: should_compare_modified_leaves }
     }
 }
 
-impl OriginalSkeletonContractsTrieConfig {
-    pub(crate) fn new() -> Self {
-        Self
+impl OriginalSkeletonTreeConfig for OriginalSkeletonTrieConfig {
+    fn compare_modified_leaves(&self) -> bool {
+        self.compare_modified_leaves
     }
 }
 

crates/starknet_committer_and_os_cli/benches/main.rs

@@ -13,7 +13,7 @@ use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
 use starknet_committer::block_committer::input::StarknetStorageValue;
 use starknet_committer::db::external_test_utils::tree_computation_flow;
 use starknet_committer::hash_function::hash::TreeHashFunctionImpl;
-use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonStorageTrieConfig;
+use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 use starknet_committer_and_os_cli::committer_cli::commands::commit;
 use starknet_committer_and_os_cli::committer_cli::parse_input::cast::CommitterFactsDbInputImpl;
 use starknet_committer_and_os_cli::committer_cli::parse_input::read::parse_input;
@@ -48,7 +48,7 @@ pub fn single_tree_flow_benchmark(criterion: &mut Criterion) {
                         leaf_modifications_input,
                         &mut storage,
                         root_hash,
-                        OriginalSkeletonStorageTrieConfig::new(false),
+                        OriginalSkeletonTrieConfig::new_for_classes_or_storage_trie(false),
                     ),
                 );
             },

crates/starknet_committer_and_os_cli/src/committer_cli/tests/python_tests.rs

@@ -19,7 +19,7 @@ use starknet_committer::hash_function::hash::{
     CONTRACT_STATE_HASH_VERSION,
 };
 use starknet_committer::patricia_merkle_tree::leaf::leaf_impl::ContractState;
-use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonStorageTrieConfig;
+use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 use starknet_committer::patricia_merkle_tree::types::CompiledClassHash;
 use starknet_patricia::patricia_merkle_tree::filled_tree::node::FactDbFilledNode;
 use starknet_patricia::patricia_merkle_tree::node_data::inner_node::{
@@ -167,7 +167,7 @@ impl PythonTestRunner for CommitterPythonTestRunner {
                     leaf_modifications,
                     &mut storage,
                     root_hash,
-                    OriginalSkeletonStorageTrieConfig::new(false),
+                    OriginalSkeletonTrieConfig::new_for_classes_or_storage_trie(false),
                 )
                 .await;
                 // 3. Serialize and return output.

crates/starknet_committer_and_os_cli/src/committer_cli/tests/regression_tests.rs

@@ -7,7 +7,7 @@ use serde_json::{Map, Value};
 use starknet_committer::block_committer::input::StarknetStorageValue;
 use starknet_committer::db::external_test_utils::single_tree_flow_test;
 use starknet_committer::hash_function::hash::TreeHashFunctionImpl;
-use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonStorageTrieConfig;
+use starknet_committer::patricia_merkle_tree::tree::OriginalSkeletonTrieConfig;
 use tempfile::NamedTempFile;
 
 use crate::committer_cli::commands::commit;
@@ -110,7 +110,7 @@ pub async fn test_regression_single_tree() {
         leaf_modifications,
         &mut storage,
         root_hash,
-        OriginalSkeletonStorageTrieConfig::new(false),
+        OriginalSkeletonTrieConfig::new_for_classes_or_storage_trie(false),
     )
     .await;
     let execution_time = std::time::Instant::now() - start;

crates/starknet_patricia/src/patricia_merkle_tree/external_test_utils.rs

@@ -21,10 +21,8 @@ use super::node_data::leaf::Leaf;
 use super::original_skeleton_tree::node::OriginalSkeletonNode;
 use super::types::{NodeIndex, SubTreeHeight};
 use crate::felt::u256_from_felt;
-use crate::generate_trie_config;
 use crate::patricia_merkle_tree::errors::TypesError;
 use crate::patricia_merkle_tree::node_data::errors::{LeafError, LeafResult};
-use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonTreeConfig;
 
 pub(crate) const TEST_PREFIX: &[u8] = &[0];
 
@@ -70,8 +68,6 @@ impl Leaf for MockLeaf {
     }
 }
 
-generate_trie_config!(OriginalSkeletonMockTrieConfig, MockLeaf);
-
 pub fn u256_try_into_felt(value: &U256) -> Result<Felt, TypesError<U256>> {
     if *value > u256_from_felt(&Felt::MAX) {
         return Err(TypesError::ConversionError {

crates/starknet_patricia/src/patricia_merkle_tree/original_skeleton_tree/config.rs

@@ -1,41 +1,6 @@
-use crate::patricia_merkle_tree::node_data::leaf::Leaf;
-
 /// Configures the creation of an original skeleton tree.
-pub trait OriginalSkeletonTreeConfig<L: Leaf> {
+pub trait OriginalSkeletonTreeConfig {
     /// Configures whether modified leaves should be compared to the previous leaves and log out a
     /// warning when encountering a trivial modification.
     fn compare_modified_leaves(&self) -> bool;
 }
-
-// TODO(Aviv 05/08/2024): Move this macro to starknet_committer crate
-#[macro_export]
-macro_rules! generate_trie_config {
-    ($struct_name:ident, $leaf_type:ty) => {
-        pub struct $struct_name {
-            compare_modified_leaves: bool,
-        }
-
-        impl $struct_name {
-            #[allow(dead_code)]
-            pub fn new(compare_modified_leaves: bool) -> Self {
-                Self { compare_modified_leaves }
-            }
-        }
-
-        impl OriginalSkeletonTreeConfig<$leaf_type> for $struct_name {
-            fn compare_modified_leaves(&self) -> bool {
-                self.compare_modified_leaves
-            }
-        }
-    };
-}
-
-#[derive(Default)]
-/// Generic config that doesn't compare the modified leaves.
-pub struct NoCompareOriginalSkeletonTrieConfig<L: Leaf>(std::marker::PhantomData<L>);
-
-impl<L: Leaf> OriginalSkeletonTreeConfig<L> for NoCompareOriginalSkeletonTrieConfig<L> {
-    fn compare_modified_leaves(&self) -> bool {
-        false
-    }
-}