refactor: removed forest traits (#285)

crates/committer/src/block_committer/commit.rs

@@ -6,32 +6,25 @@ use crate::block_committer::input::Config;
 use crate::block_committer::input::ConfigImpl;
 use crate::block_committer::input::ContractAddress;
 use crate::block_committer::input::Input;
-use crate::patricia_merkle_tree::filled_tree::forest::FilledForestImpl;
+use crate::patricia_merkle_tree::filled_tree::forest::FilledForest;
 use crate::patricia_merkle_tree::filled_tree::node::{ClassHash, Nonce};
 use crate::patricia_merkle_tree::node_data::leaf::ContractState;
-use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::{
-    OriginalSkeletonForest, OriginalSkeletonForestImpl,
-};
-use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl;
+use crate::patricia_merkle_tree::original_skeleton_tree::skeleton_forest::OriginalSkeletonForest;
 use crate::patricia_merkle_tree::types::NodeIndex;
 use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::TreeHashFunctionImpl;
-use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::{
-    UpdatedSkeletonForest, UpdatedSkeletonForestImpl,
-};
-use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
+use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::UpdatedSkeletonForest;
 use crate::storage::map_storage::MapStorage;
 
 type BlockCommitmentResult<T> = Result<T, BlockCommitmentError>;
 
-pub async fn commit_block(input: Input<ConfigImpl>) -> BlockCommitmentResult<FilledForestImpl> {
-    let (mut original_forest, original_contracts_trie_leaves) =
-        OriginalSkeletonForestImpl::<OriginalSkeletonTreeImpl>::create(
-            MapStorage::from(input.storage),
-            input.contracts_trie_root_hash,
-            input.classes_trie_root_hash,
-            &input.state_diff,
-            &input.config,
-        )?;
+pub async fn commit_block(input: Input<ConfigImpl>) -> BlockCommitmentResult<FilledForest> {
+    let (mut original_forest, original_contracts_trie_leaves) = OriginalSkeletonForest::create(
+        MapStorage::from(input.storage),
+        input.contracts_trie_root_hash,
+        input.classes_trie_root_hash,
+        &input.state_diff,
+        &input.config,
+    )?;
 
     if input.config.warn_on_trivial_modifications() {
         check_trivial_nonce_and_class_hash_updates(
@@ -41,7 +34,7 @@ pub async fn commit_block(input: Input<ConfigImpl>) -> BlockCommitmentResult<Fil
         );
     }
 
-    let updated_forest = UpdatedSkeletonForestImpl::<UpdatedSkeletonTreeImpl>::create(
+    let updated_forest = UpdatedSkeletonForest::create(
         &mut original_forest,
         &input.state_diff.skeleton_classes_updates(),
         &input.state_diff.skeleton_storage_updates(),
@@ -50,17 +43,15 @@ pub async fn commit_block(input: Input<ConfigImpl>) -> BlockCommitmentResult<Fil
         &input.state_diff.address_to_nonce,
     )?;
 
-    Ok(
-        FilledForestImpl::create::<UpdatedSkeletonTreeImpl, TreeHashFunctionImpl>(
-            updated_forest,
-            input.state_diff.actual_storage_updates(),
-            input.state_diff.actual_classes_updates(),
-            &original_contracts_trie_leaves,
-            &input.state_diff.address_to_class_hash,
-            &input.state_diff.address_to_nonce,
-        )
-        .await?,
+    Ok(FilledForest::create::<TreeHashFunctionImpl>(
+        updated_forest,
+        input.state_diff.actual_storage_updates(),
+        input.state_diff.actual_classes_updates(),
+        &original_contracts_trie_leaves,
+        &input.state_diff.address_to_class_hash,
+        &input.state_diff.address_to_nonce,
     )
+    .await?)
 }
 
 /// Compares the previous state's nonce and class hash with the given in the state diff.

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

@@ -10,31 +10,22 @@ use crate::patricia_merkle_tree::filled_tree::tree::{
 use crate::patricia_merkle_tree::node_data::leaf::{ContractState, LeafModifications};
 use crate::patricia_merkle_tree::types::NodeIndex;
 use crate::patricia_merkle_tree::updated_skeleton_tree::hash_function::ForestHashFunction;
-use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::UpdatedSkeletonForestImpl;
-use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTree;
+use crate::patricia_merkle_tree::updated_skeleton_tree::skeleton_forest::UpdatedSkeletonForest;
+use crate::patricia_merkle_tree::updated_skeleton_tree::tree::UpdatedSkeletonTreeImpl;
 use crate::storage::storage_trait::Storage;
 
 use std::collections::HashMap;
 use std::sync::Arc;
 use tokio::task::JoinSet;
 
-pub trait FilledForest {
-    /// Serialize each tree and store it.
-    fn write_to_storage(&self, storage: &mut impl Storage);
-
-    fn get_compiled_class_root_hash(&self) -> HashOutput;
-
-    fn get_contract_root_hash(&self) -> HashOutput;
-}
-
-pub struct FilledForestImpl {
+pub struct FilledForest {
     pub storage_tries: StorageTrieMap,
     pub contracts_trie: ContractsTrie,
     pub classes_trie: ClassesTrie,
 }
 
-impl FilledForest for FilledForestImpl {
-    fn write_to_storage(&self, storage: &mut impl Storage) {
+impl FilledForest {
+    pub fn write_to_storage(&self, storage: &mut impl Storage) {
         // Serialize all trees to one hash map.
         let new_db_objects = self
             .storage_tries
@@ -48,21 +39,16 @@ impl FilledForest for FilledForestImpl {
         storage.mset(new_db_objects);
     }
 
-    fn get_contract_root_hash(&self) -> HashOutput {
+    pub fn get_contract_root_hash(&self) -> HashOutput {
         self.contracts_trie.get_root_hash()
     }
 
-    fn get_compiled_class_root_hash(&self) -> HashOutput {
+    pub fn get_compiled_class_root_hash(&self) -> HashOutput {
         self.classes_trie.get_root_hash()
     }
-}
 
-impl FilledForestImpl {
-    pub(crate) async fn create<
-        T: UpdatedSkeletonTree + 'static,
-        TH: ForestHashFunction + 'static,
-    >(
-        mut updated_forest: UpdatedSkeletonForestImpl<T>,
+    pub(crate) async fn create<TH: ForestHashFunction + 'static>(
+        mut updated_forest: UpdatedSkeletonForest,
         storage_updates: HashMap<ContractAddress, LeafModifications<StarknetStorageValue>>,
         classes_updates: LeafModifications<CompiledClassHash>,
         original_contracts_trie_leaves: &HashMap<NodeIndex, ContractState>,
@@ -86,7 +72,7 @@ impl FilledForestImpl {
             let original_contract_state = original_contracts_trie_leaves
                 .get(&NodeIndex::from_contract_address(&address))
                 .ok_or(ForestError::MissingContractCurrentState(address))?;
-            tasks.spawn(Self::new_contract_state::<T, TH>(
+            tasks.spawn(Self::new_contract_state::<TH>(
                 address,
                 *(address_to_nonce
                     .get(&address)
@@ -121,14 +107,11 @@ impl FilledForestImpl {
         })
     }
 
-    async fn new_contract_state<
-        T: UpdatedSkeletonTree + 'static,
-        TH: ForestHashFunction + 'static,
-    >(
+    async fn new_contract_state<TH: ForestHashFunction + 'static>(
         contract_address: ContractAddress,
         new_nonce: Nonce,
         new_class_hash: ClassHash,
-        updated_storage_trie: T,
+        updated_storage_trie: UpdatedSkeletonTreeImpl,
         inner_updates: LeafModifications<StarknetStorageValue>,
     ) -> ForestResult<(ContractAddress, ContractState, StorageTrie)> {
         let filled_storage_trie =

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

@@ -12,6 +12,7 @@ use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeleto
 use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonContractsTrieConfig;
 use crate::patricia_merkle_tree::original_skeleton_tree::config::OriginalSkeletonStorageTrieConfig;
 use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTree;
+use crate::patricia_merkle_tree::original_skeleton_tree::tree::OriginalSkeletonTreeImpl;
 use crate::patricia_merkle_tree::types::NodeIndex;
 use crate::patricia_merkle_tree::types::SortedLeafIndices;
 use crate::storage::storage_trait::Storage;
@@ -22,30 +23,18 @@ use std::collections::HashSet;
 #[path = "skeleton_forest_test.rs"]
 pub mod skeleton_forest_test;
 
-pub(crate) trait OriginalSkeletonForest {
-    /// Creates an original skeleton forest that includes the storage tries of the modified contracts,
-    /// the classes trie and the contracts trie. Additionally, returns the original contract states that
-    /// are needed to compute the contract state tree.
-    fn create(
-        storage: impl Storage,
-        contracts_trie_root_hash: HashOutput,
-        classes_trie_root_hash: HashOutput,
-        state_diff: &StateDiff,
-        config: &impl Config,
-    ) -> ForestResult<(Self, HashMap<NodeIndex, ContractState>)>
-    where
-        Self: std::marker::Sized;
-}
-
 #[derive(Debug, Eq, PartialEq)]
-pub(crate) struct OriginalSkeletonForestImpl<T: OriginalSkeletonTree> {
-    pub(crate) classes_trie: T,
-    pub(crate) contracts_trie: T,
-    pub(crate) storage_tries: HashMap<ContractAddress, T>,
+pub(crate) struct OriginalSkeletonForest {
+    pub(crate) classes_trie: OriginalSkeletonTreeImpl,
+    pub(crate) contracts_trie: OriginalSkeletonTreeImpl,
+    pub(crate) storage_tries: HashMap<ContractAddress, OriginalSkeletonTreeImpl>,
 }
 
-impl<T: OriginalSkeletonTree> OriginalSkeletonForest for OriginalSkeletonForestImpl<T> {
-    fn create(
+impl OriginalSkeletonForest {
+    /// Creates an original skeleton forest that includes the storage tries of the modified contracts,
+    /// the classes trie and the contracts trie. Additionally, returns the original contract states that
+    /// are needed to compute the contract state tree.
+    pub(crate) fn create(
         storage: impl Storage,
         contracts_trie_root_hash: HashOutput,
         classes_trie_root_hash: HashOutput,
@@ -56,7 +45,7 @@ impl<T: OriginalSkeletonTree> OriginalSkeletonForest for OriginalSkeletonForestI
         Self: std::marker::Sized,
     {
         let accessed_addresses = state_diff.accessed_addresses();
-        let (contracts_state_trie, original_contracts_trie_leaves) =
+        let (contracts_trie, original_contracts_trie_leaves) =
             Self::create_contracts_trie(&accessed_addresses, contracts_trie_root_hash, &storage)?;
         let storage_tries = Self::create_storage_tries(
             &state_diff.actual_storage_updates(),
@@ -72,37 +61,27 @@ impl<T: OriginalSkeletonTree> OriginalSkeletonForest for OriginalSkeletonForestI
         )?;
 
         Ok((
-            Self::new(classes_trie, contracts_state_trie, storage_tries),
+            Self {
+                classes_trie,
+                contracts_trie,
+                storage_tries,
+            },
             original_contracts_trie_leaves,
         ))
     }
-}
-
-impl<T: OriginalSkeletonTree> OriginalSkeletonForestImpl<T> {
-    pub(crate) fn new(
-        classes_trie: T,
-        contracts_trie: T,
-        storage_tries: HashMap<ContractAddress, T>,
-    ) -> Self {
-        Self {
-            classes_trie,
-            contracts_trie,
-            storage_tries,
-        }
-    }
 
     /// Creates the contracts trie original skeleton.
     /// Also returns the previous contracts state of the modified contracts.
     fn create_contracts_trie(
         accessed_addresses: &HashSet<&ContractAddress>,
         contracts_trie_root_hash: HashOutput,
         storage: &impl Storage,
-    ) -> ForestResult<(T, HashMap<NodeIndex, ContractState>)> {
+    ) -> ForestResult<(OriginalSkeletonTreeImpl, HashMap<NodeIndex, ContractState>)> {
         let mut sorted_leaf_indices: Vec<NodeIndex> = accessed_addresses
             .iter()
             .map(|address| NodeIndex::from_contract_address(address))
             .collect();
-        Ok(T::create_and_get_previous_leaves(
+        Ok(OriginalSkeletonTreeImpl::create_and_get_previous_leaves(
             storage,
             contracts_trie_root_hash,
             SortedLeafIndices::new(&mut sorted_leaf_indices),
@@ -115,7 +94,7 @@ impl<T: OriginalSkeletonTree> OriginalSkeletonForestImpl<T> {
         original_contracts_trie_leaves: &HashMap<NodeIndex, ContractState>,
         storage: &impl Storage,
         config: &impl Config,
-    ) -> ForestResult<HashMap<ContractAddress, T>> {
+    ) -> ForestResult<HashMap<ContractAddress, OriginalSkeletonTreeImpl>> {
         let mut storage_tries = HashMap::new();
         for (address, updates) in actual_storage_updates {
             let mut sorted_leaf_indices: Vec<NodeIndex> = updates.keys().copied().collect();
@@ -127,7 +106,7 @@ impl<T: OriginalSkeletonTree> OriginalSkeletonForestImpl<T> {
                 updates,
                 config.warn_on_trivial_modifications(),
             );
-            let original_skeleton = T::create(
+            let original_skeleton = OriginalSkeletonTreeImpl::create(
                 storage,
                 contract_state.storage_root_hash,
                 SortedLeafIndices::new(&mut sorted_leaf_indices),
@@ -143,15 +122,15 @@ impl<T: OriginalSkeletonTree> OriginalSkeletonForestImpl<T> {
         classes_trie_root_hash: HashOutput,
         storage: &impl Storage,
         config: &impl Config,
-    ) -> ForestResult<T> {
+    ) -> ForestResult<OriginalSkeletonTreeImpl> {
         let config = OriginalSkeletonClassesTrieConfig::new(
             actual_classes_updates,
             config.warn_on_trivial_modifications(),
         );
         let mut sorted_leaf_indices: Vec<NodeIndex> =
             actual_classes_updates.keys().copied().collect();
 
-        Ok(T::create(
+        Ok(OriginalSkeletonTreeImpl::create(
             storage,
             classes_trie_root_hash,
             SortedLeafIndices::new(&mut sorted_leaf_indices),

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

@@ -2,7 +2,7 @@ use pretty_assertions::assert_eq;
 use rstest::rstest;
 use std::collections::HashMap;
 
-use super::OriginalSkeletonForestImpl;
+use super::OriginalSkeletonForest;
 use crate::block_committer::input::{
     ConfigImpl, ContractAddress, Input, StarknetStorageKey, StarknetStorageValue, StateDiff,
 };
@@ -18,8 +18,6 @@ use crate::patricia_merkle_tree::original_skeleton_tree::create_tree::create_tre
     create_compiled_class_leaf_entry, create_contract_state_leaf_entry, create_root_edge_entry,
     create_storage_leaf_entry,
 };
-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::NodeIndex;
 use crate::patricia_merkle_tree::types::SubTreeHeight;
 use crate::storage::map_storage::MapStorage;
@@ -152,7 +150,7 @@ use crate::storage::map_storage::MapStorage;
         contracts_trie_root_hash: HashOutput(Felt::from(861_u128 + 248_u128)),
         classes_trie_root_hash: HashOutput(Felt::from(155_u128 + 248_u128)),
         config: ConfigImpl::new(true),
-    }, OriginalSkeletonForestImpl{
+    }, OriginalSkeletonForest{
         classes_trie: create_expected_skeleton(
             vec![
                 create_edge_skeleton_node(1, 0, 1),
@@ -232,10 +230,10 @@ use crate::storage::map_storage::MapStorage;
 )]
 fn test_create_original_skeleton_forest(
     #[case] input: Input<ConfigImpl>,
-    #[case] expected_forest: OriginalSkeletonForestImpl<OriginalSkeletonTreeImpl>,
+    #[case] expected_forest: OriginalSkeletonForest,
     #[case] expected_original_contracts_trie_leaves: HashMap<ContractAddress, ContractState>,
 ) {
-    let (actual_forest, original_contracts_trie_leaves) = OriginalSkeletonForestImpl::create(
+    let (actual_forest, original_contracts_trie_leaves) = OriginalSkeletonForest::create(
         MapStorage::from(input.storage),
         input.contracts_trie_root_hash,
         input.classes_trie_root_hash,