starknet_committer,apollo_committer: remove CommitBlockTrait, promote commit_block to free function (#13758)

yoavGrs · claude · web-flow · commit d843434cb880 · 2026-04-26T07:08:37.000Z
Co-authored-by: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/crates/apollo_committer/src/committer.rs b/crates/apollo_committer/src/committer.rs
@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 use std::error::Error;
-use std::marker::PhantomData;
 use std::path::PathBuf;
 
 use apollo_committer_config::config::{ApolloStorage, CommitterConfig};
@@ -18,7 +17,7 @@ use starknet_api::block_hash::state_diff_hash::calculate_state_diff_hash;
 use starknet_api::core::{GlobalRoot, StateDiffCommitment};
 use starknet_api::hash::PoseidonHash;
 use starknet_api::state::ThinStateDiff;
-use starknet_committer::block_committer::commit::CommitBlockTrait;
+use starknet_committer::block_committer::commit::commit_block;
 use starknet_committer::block_committer::input::Input;
 use starknet_committer::block_committer::measurements_util::{
     Action,
@@ -70,10 +69,7 @@ mod committer_test;
 
 pub type ApolloCommitterDb = IndexDb<ApolloStorage>;
 
-pub struct BlockCommitter;
-impl CommitBlockTrait for BlockCommitter {}
-
-pub type ApolloCommitter = Committer<ApolloStorage, ApolloCommitterDb, BlockCommitter>;
+pub type ApolloCommitter = Committer<ApolloStorage, ApolloCommitterDb>;
 
 pub trait StorageConstructor: Storage {
     fn create_storage(db_path: PathBuf, storage_config: Self::Config) -> Self;
@@ -96,33 +92,29 @@ struct CommitStateDiffOutput {
 }
 
 /// Apollo committer. Maintains the Starknet state tries in persistent storage.
-pub struct Committer<S: Storage, ForestDB, BlockCommitter>
+pub struct Committer<S: Storage, ForestDB>
 where
     ForestDB: ForestStorageWithEmptyReadContext,
-    BlockCommitter: CommitBlockTrait,
 {
     /// Storage for forest operations.
     forest_storage: ForestDB,
     /// Committer config.
     config: CommitterConfig<S::Config>,
     /// The next block number to commit.
     offset: BlockNumber,
-    // Allow define the generic type CB and not use it.
-    phantom: PhantomData<BlockCommitter>,
 }
 
-impl<S, ForestDB, BlockCommitter> Committer<S, ForestDB, BlockCommitter>
+impl<S, ForestDB> Committer<S, ForestDB>
 where
     S: StorageConstructor,
     ForestDB: ForestStorageWithEmptyReadContext<Storage = S>,
-    BlockCommitter: CommitBlockTrait,
 {
     pub async fn new(config: CommitterConfig<S::Config>) -> Self {
         let storage = S::create_storage(config.db_path.clone(), config.storage_config.clone());
         let mut forest_storage = ForestDB::new(storage);
         let offset = Self::load_offset_or_panic(&mut forest_storage).await;
         info!("Initializing committer with offset: {offset}");
-        Self { forest_storage, config, offset, phantom: PhantomData }
+        Self { forest_storage, config, offset }
     }
 
     fn update_offset(&mut self, offset: BlockNumber) {
@@ -405,7 +397,7 @@ where
             config: self.config.reader_config.clone(),
         };
         let (filled_forest, deleted_nodes) =
-            BlockCommitter::commit_block(input, &mut self.forest_storage, measurements)
+            commit_block(input, &mut self.forest_storage, measurements)
                 .await
                 .map_err(|err| self.map_internal_error(err))?;
         let global_root = filled_forest.state_roots().global_root();
diff --git a/crates/apollo_committer/src/committer_test.rs b/crates/apollo_committer/src/committer_test.rs
@@ -17,12 +17,11 @@ use starknet_api::state::ThinStateDiff;
 use starknet_committer::db::index_db::db::IndexDb;
 use starknet_patricia_storage::map_storage::MapStorage;
 
-use super::{BlockCommitter, Committer};
+use super::Committer;
 use crate::committer::StorageConstructor;
 
 pub type ApolloTestStorage = MapStorage;
-pub type ApolloTestCommitter =
-    Committer<ApolloTestStorage, IndexDb<ApolloTestStorage>, BlockCommitter>;
+pub type ApolloTestCommitter = Committer<ApolloTestStorage, IndexDb<ApolloTestStorage>>;
 
 impl StorageConstructor for ApolloTestStorage {
     fn create_storage(_db_path: PathBuf, _storage_config: Self::Config) -> Self {
diff --git a/crates/apollo_committer/src/communication.rs b/crates/apollo_committer/src/communication.rs
@@ -2,7 +2,6 @@ use apollo_committer_types::communication::{CommitterRequest, CommitterResponse}
 use apollo_infra::component_definitions::ComponentRequestHandler;
 use apollo_infra::component_server::{LocalComponentServer, RemoteComponentServer};
 use async_trait::async_trait;
-use starknet_committer::block_committer::commit::CommitBlockTrait;
 use starknet_committer::db::forest_trait::ForestStorageWithEmptyReadContext;
 
 use crate::committer::{ApolloCommitter, Committer, StorageConstructor};
@@ -12,12 +11,8 @@ pub type LocalCommitterServer =
 pub type RemoteCommitterServer = RemoteComponentServer<CommitterRequest, CommitterResponse>;
 
 #[async_trait]
-impl<
-    S: StorageConstructor,
-    ForestDB: ForestStorageWithEmptyReadContext<Storage = S>,
-    BlockCommitter: CommitBlockTrait,
-> ComponentRequestHandler<CommitterRequest, CommitterResponse>
-    for Committer<S, ForestDB, BlockCommitter>
+impl<S: StorageConstructor, ForestDB: ForestStorageWithEmptyReadContext<Storage = S>>
+    ComponentRequestHandler<CommitterRequest, CommitterResponse> for Committer<S, ForestDB>
 {
     async fn handle_request(&mut self, request: CommitterRequest) -> CommitterResponse {
         match request {
diff --git a/crates/starknet_committer/src/block_committer/commit.rs b/crates/starknet_committer/src/block_committer/commit.rs
@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 
-use async_trait::async_trait;
 use starknet_api::core::{ClassHash, ContractAddress, Nonce};
 use starknet_patricia::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices};
 use starknet_types_core::felt::Felt;
@@ -30,101 +29,92 @@ use crate::patricia_merkle_tree::types::class_hash_into_node_index;
 
 pub type BlockCommitmentResult<T> = Result<T, BlockCommitmentError>;
 
-// TODO(Yoav): Remove this trait when the index layout is ready.
-#[async_trait]
-pub trait CommitBlockTrait: Send {
-    async fn commit_block<Reader: ForestReader + Send, M: MeasurementsTrait + Send>(
-        input: Input<Reader::InitialReadContext>,
-        trie_reader: &mut Reader,
-        measurements: &mut M,
-    ) -> BlockCommitmentResult<(FilledForest, DeletedNodes)> {
-        let (mut storage_tries_indices, mut contracts_trie_indices, mut classes_trie_indices) =
-            get_all_modified_indices(&input.state_diff);
-        let n_contracts_trie_modifications = contracts_trie_indices.len();
-        let forest_sorted_indices = ForestSortedIndices {
-            storage_tries_sorted_indices: storage_tries_indices
-                .iter_mut()
-                .map(|(address, indices)| (*address, SortedLeafIndices::new(indices)))
-                .collect(),
-            contracts_trie_sorted_indices: SortedLeafIndices::new(&mut contracts_trie_indices),
-            classes_trie_sorted_indices: SortedLeafIndices::new(&mut classes_trie_indices),
-        };
-        let actual_storage_updates = input.state_diff.actual_storage_updates();
-        let actual_classes_updates = input.state_diff.actual_classes_updates();
-        // Record the number of modifications.
-        measure_number_of_modifications(
-            measurements,
-            &actual_storage_updates,
-            n_contracts_trie_modifications,
-            actual_classes_updates.len(),
-        );
-        // Reads - fetch_nodes.
-        measurements.start_measurement(Action::Read);
-        let roots =
-            trie_reader.read_roots(input.initial_read_context).await.map_err(ForestError::from)?;
-        let (original_forest, original_contracts_trie_leaves) = trie_reader
-            .read(
-                roots,
-                &actual_storage_updates,
-                &actual_classes_updates,
-                &forest_sorted_indices,
-                input.config.clone(),
-            )
-            .await?;
-        let n_read_entries =
-            original_forest.storage_tries.values().map(|trie| trie.nodes.len()).sum();
-        measurements.attempt_to_stop_measurement(Action::Read, n_read_entries).ok();
-        debug!("Original skeleton forest created successfully.");
-
-        if input.config.warn_on_trivial_modifications() {
-            check_trivial_nonce_and_class_hash_updates(
-                &original_contracts_trie_leaves,
-                &input.state_diff.address_to_class_hash,
-                &input.state_diff.address_to_nonce,
-            );
-        }
-
-        // Compute the new topology.
-        measurements.start_measurement(Action::Compute);
-        let updated_forest = UpdatedSkeletonForest::create(
-            &original_forest,
-            &input.state_diff.skeleton_classes_updates(),
-            &input.state_diff.skeleton_storage_updates(),
-            &original_contracts_trie_leaves,
-            &input.state_diff.address_to_class_hash,
-            &input.state_diff.address_to_nonce,
-        )?;
-        debug!("Updated skeleton forest created successfully.");
-
-        // Find deleted nodes.
-        let deleted_nodes = find_deleted_nodes(
-            &original_forest,
-            &updated_forest,
+pub async fn commit_block<Reader: ForestReader + Send, M: MeasurementsTrait + Send>(
+    input: Input<Reader::InitialReadContext>,
+    trie_reader: &mut Reader,
+    measurements: &mut M,
+) -> BlockCommitmentResult<(FilledForest, DeletedNodes)> {
+    let (mut storage_tries_indices, mut contracts_trie_indices, mut classes_trie_indices) =
+        get_all_modified_indices(&input.state_diff);
+    let n_contracts_trie_modifications = contracts_trie_indices.len();
+    let forest_sorted_indices = ForestSortedIndices {
+        storage_tries_sorted_indices: storage_tries_indices
+            .iter_mut()
+            .map(|(address, indices)| (*address, SortedLeafIndices::new(indices)))
+            .collect(),
+        contracts_trie_sorted_indices: SortedLeafIndices::new(&mut contracts_trie_indices),
+        classes_trie_sorted_indices: SortedLeafIndices::new(&mut classes_trie_indices),
+    };
+    let actual_storage_updates = input.state_diff.actual_storage_updates();
+    let actual_classes_updates = input.state_diff.actual_classes_updates();
+    // Record the number of modifications.
+    measure_number_of_modifications(
+        measurements,
+        &actual_storage_updates,
+        n_contracts_trie_modifications,
+        actual_classes_updates.len(),
+    );
+    // Reads - fetch_nodes.
+    measurements.start_measurement(Action::Read);
+    let roots =
+        trie_reader.read_roots(input.initial_read_context).await.map_err(ForestError::from)?;
+    let (original_forest, original_contracts_trie_leaves) = trie_reader
+        .read(
+            roots,
             &actual_storage_updates,
             &actual_classes_updates,
-            &original_contracts_trie_leaves,
-        );
+            &forest_sorted_indices,
+            input.config.clone(),
+        )
+        .await?;
+    let n_read_entries = original_forest.storage_tries.values().map(|trie| trie.nodes.len()).sum();
+    measurements.attempt_to_stop_measurement(Action::Read, n_read_entries).ok();
+    debug!("Original skeleton forest created successfully.");
 
-        // Compute the new hashes.
-        let filled_forest = FilledForest::create::<TreeHashFunctionImpl>(
-            updated_forest,
-            actual_storage_updates,
-            actual_classes_updates,
+    if input.config.warn_on_trivial_modifications() {
+        check_trivial_nonce_and_class_hash_updates(
             &original_contracts_trie_leaves,
             &input.state_diff.address_to_class_hash,
             &input.state_diff.address_to_nonce,
-        )
-        .await?;
-        measurements.attempt_to_stop_measurement(Action::Compute, 0).ok();
-        debug!("Filled forest created successfully.");
-
-        Ok((filled_forest, deleted_nodes))
+        );
     }
-}
 
-pub struct CommitBlockImpl;
-
-impl CommitBlockTrait for CommitBlockImpl {}
+    // Compute the new topology.
+    measurements.start_measurement(Action::Compute);
+    let updated_forest = UpdatedSkeletonForest::create(
+        &original_forest,
+        &input.state_diff.skeleton_classes_updates(),
+        &input.state_diff.skeleton_storage_updates(),
+        &original_contracts_trie_leaves,
+        &input.state_diff.address_to_class_hash,
+        &input.state_diff.address_to_nonce,
+    )?;
+    debug!("Updated skeleton forest created successfully.");
+
+    // Find deleted nodes.
+    let deleted_nodes = find_deleted_nodes(
+        &original_forest,
+        &updated_forest,
+        &actual_storage_updates,
+        &actual_classes_updates,
+        &original_contracts_trie_leaves,
+    );
+
+    // Compute the new hashes.
+    let filled_forest = FilledForest::create::<TreeHashFunctionImpl>(
+        updated_forest,
+        actual_storage_updates,
+        actual_classes_updates,
+        &original_contracts_trie_leaves,
+        &input.state_diff.address_to_class_hash,
+        &input.state_diff.address_to_nonce,
+    )
+    .await?;
+    measurements.attempt_to_stop_measurement(Action::Compute, 0).ok();
+    debug!("Filled forest created successfully.");
+
+    Ok((filled_forest, deleted_nodes))
+}
 
 /// Compares the previous state's nonce and class hash with the given in the state diff.
 /// In case of trivial update, logs out a warning for trivial state diff update.
diff --git a/crates/starknet_committer/src/block_committer/commit_test.rs b/crates/starknet_committer/src/block_committer/commit_test.rs
@@ -11,7 +11,7 @@ use starknet_api::hash::StateRoots;
 use starknet_patricia_storage::map_storage::MapStorage;
 use starknet_types_core::felt::Felt;
 
-use crate::block_committer::commit::{CommitBlockImpl, CommitBlockTrait};
+use crate::block_committer::commit::commit_block;
 use crate::block_committer::input::{
     Input,
     ReaderConfig,
@@ -128,7 +128,7 @@ async fn test_commit_two_consecutive_blocks<Db: ForestReader + ForestWriter>(
         config: ReaderConfig::default(),
     };
     let (filled_forest, _deleted_nodes) =
-        CommitBlockImpl::commit_block(input, &mut db, &mut NoMeasurements).await.unwrap();
+        commit_block(input, &mut db, &mut NoMeasurements).await.unwrap();
     db.write(&filled_forest).await.unwrap();
 
     input = Input {
@@ -138,7 +138,7 @@ async fn test_commit_two_consecutive_blocks<Db: ForestReader + ForestWriter>(
     };
 
     let (filled_forest, _deleted_nodes) =
-        CommitBlockImpl::commit_block(input, &mut db, &mut NoMeasurements).await.unwrap();
+        commit_block(input, &mut db, &mut NoMeasurements).await.unwrap();
     db.write(&filled_forest).await.unwrap();
 
     expected_roots.assert_debug_eq(&filled_forest.state_roots());
diff --git a/crates/starknet_committer/src/forest/deleted_nodes_test.rs b/crates/starknet_committer/src/forest/deleted_nodes_test.rs
@@ -5,7 +5,7 @@ use starknet_patricia::patricia_merkle_tree::types::NodeIndex;
 use starknet_patricia_storage::map_storage::MapStorage;
 use starknet_types_core::felt::Felt;
 
-use crate::block_committer::commit::{CommitBlockImpl, CommitBlockTrait};
+use crate::block_committer::commit::commit_block;
 use crate::block_committer::input::{
     contract_address_into_node_index,
     Input,
@@ -23,7 +23,7 @@ const CONTRACT_ADDRESS: ContractAddress = ContractAddress(PatriciaKey::from_hex_
 
 /// Commits a block with the given leaves and value, and writes the forest to storage.
 /// Returns the deleted nodes.
-async fn commit_block(
+async fn commit_block_and_write(
     leaves: Vec<u128>,
     value: u128,
     index_db: &mut IndexDb<MapStorage>,
@@ -44,9 +44,7 @@ async fn commit_block(
     };
 
     let (filled_forest, deleted_nodes) =
-        CommitBlockImpl::commit_block(input, index_db, &mut NoMeasurements)
-            .await
-            .expect("Failed to commit block");
+        commit_block(input, index_db, &mut NoMeasurements).await.expect("Failed to commit block");
 
     // Write the forest to storage so the roots are persisted, including deleted nodes
     ForestWriterWithMetadata::write_with_metadata(
@@ -76,10 +74,10 @@ async fn verify_deleted_nodes(
     let mut index_db = IndexDb::new(MapStorage::default());
 
     // Commit block with original leaves (all non-zero)
-    commit_block(initial_leaves, 100, &mut index_db).await;
+    commit_block_and_write(initial_leaves, 100, &mut index_db).await;
 
     // Commit block with deletion leaves (all zero)
-    let deleted_nodes = commit_block(leaves_to_delete, 0, &mut index_db).await;
+    let deleted_nodes = commit_block_and_write(leaves_to_delete, 0, &mut index_db).await;
 
     // Verify the deleted nodes match the expected set.
     let storage_trie_deleted_nodes = deleted_nodes.storage_tries.get(&CONTRACT_ADDRESS);
diff --git a/crates/starknet_committer_and_os_cli/src/committer_cli/commands.rs b/crates/starknet_committer_and_os_cli/src/committer_cli/commands.rs
@@ -1,4 +1,4 @@
-use starknet_committer::block_committer::commit::{CommitBlockImpl, CommitBlockTrait};
+use starknet_committer::block_committer::commit::commit_block;
 use starknet_committer::block_committer::measurements_util::NoMeasurements;
 use starknet_committer::db::facts_db::FactsDb;
 use starknet_committer::db::forest_trait::StorageInitializer;
@@ -35,10 +35,9 @@ pub async fn parse_and_commit(
 
 pub async fn commit(input: FactsDbInputImpl, output_path: String, storage: MapStorage) {
     let mut facts_db = FactsDb::new(storage);
-    let (filled_forest, _deleted_nodes) =
-        CommitBlockImpl::commit_block(input, &mut facts_db, &mut NoMeasurements)
-            .await
-            .expect("Failed to commit the given block.");
+    let (filled_forest, _deleted_nodes) = commit_block(input, &mut facts_db, &mut NoMeasurements)
+        .await
+        .expect("Failed to commit the given block.");
     let output = SerializedForest(filled_forest)
         .forest_to_output()
         .await
diff --git a/crates/starknet_committer_and_os_cli/src/committer_cli/snap_sync.rs b/crates/starknet_committer_and_os_cli/src/committer_cli/snap_sync.rs
diff --git a/crates/starknet_committer_cli/src/commands.rs b/crates/starknet_committer_cli/src/commands.rs
diff --git a/crates/starknet_transaction_prover/src/running/committer_utils.rs b/crates/starknet_transaction_prover/src/running/committer_utils.rs
