apollo_batcher: commitment manager uses apollo committer types

yoavGrs · yoavGrs · commit 2e05557c1e25 · 2026-01-06T14:19:25.000+02:00
diff --git a/crates/apollo_batcher/src/batcher.rs b/crates/apollo_batcher/src/batcher.rs
@@ -26,7 +26,9 @@ use apollo_batcher_types::batcher_types::{
 use apollo_batcher_types::errors::BatcherError;
 use apollo_class_manager_types::transaction_converter::TransactionConverter;
 use apollo_class_manager_types::SharedClassManagerClient;
-use apollo_committer_types::communication::SharedCommitterClient;
+use apollo_committer_types::committer_types::{CommitBlockResponse, RevertBlockResponse};
+use apollo_committer_types::communication::{CommitterClientResponse, SharedCommitterClient};
+use apollo_committer_types::errors::CommitterClientResult;
 use apollo_infra::component_definitions::{default_component_start_fn, ComponentStarter};
 use apollo_l1_provider_types::errors::{L1ProviderClientError, L1ProviderError};
 use apollo_l1_provider_types::{SessionState, SharedL1ProviderClient};
@@ -705,7 +707,7 @@ impl Batcher {
             .expect("The commitment offset unexpectedly doesn't match the given block height.");
 
         // Write ready commitments to storage.
-        self.write_commitment_results_to_storage().await?;
+        self.handle_committer_results().await?;
 
         LAST_SYNCED_BLOCK_HEIGHT.set_lossy(block_number.0);
         SYNCED_TRANSACTIONS.increment(
@@ -770,7 +772,7 @@ impl Batcher {
             .expect("The commitment offset unexpectedly doesn't match the given block height.");
 
         // Write ready commitments to storage.
-        self.write_commitment_results_to_storage().await?;
+        self.handle_committer_results().await?;
 
         let execution_infos = block_execution_artifacts
             .execution_data
@@ -1152,66 +1154,95 @@ impl Batcher {
     }
 
     /// Writes the ready commitment results to storage.
-    async fn write_commitment_results_to_storage(&mut self) -> BatcherResult<()> {
+    async fn handle_committer_results(&mut self) -> BatcherResult<()> {
         let commitment_results = self.commitment_manager.get_commitment_results().await;
         for commitment_task_output in commitment_results.into_iter() {
-            let height = commitment_task_output.height;
-
-            // Decide whether to finalize the block hash based on the config.
-            let should_finalize_block_hash =
-                match self.config.first_block_with_partial_block_hash.as_ref() {
-                    Some(FirstBlockWithPartialBlockHash { block_number, .. }) => {
-                        height >= *block_number
-                    }
-                    None => true,
-                };
-
-            // Get the final commitment.
-            let FinalBlockCommitment { height, block_hash, global_root } =
-                ApolloCommitmentManager::final_commitment_output(
-                    self.storage_reader.clone(),
-                    commitment_task_output,
-                    should_finalize_block_hash,
-                )
-                .map_err(|err| {
-                    error!("Failed to get the final commitment output for height {height}: {err}");
-                    BatcherError::InternalError
-                })?;
-
-            // Verify the first new block hash matches the configured block hash.
-            if let Some(FirstBlockWithPartialBlockHash {
-                block_number,
-                block_hash: expected_block_hash,
-                ..
-            }) = self.config.first_block_with_partial_block_hash.as_ref()
-            {
-                if height == *block_number {
-                    assert_eq!(
-                        *expected_block_hash,
-                        block_hash.expect(
-                            "The block hash of the first new block should be finalized and \
-                             therefore set."
-                        ),
-                        "The calculated block hash of the first new block ({block_hash:?}) does \
-                         not match the configured block hash ({expected_block_hash:?})"
-                    );
+            match commitment_task_output.committer_response {
+                CommitterClientResponse::CommitBlock(commit_response) => {
+                    self.write_commitment_result_to_storage(
+                        commitment_task_output.height,
+                        commit_response,
+                    )
+                    .await?;
+                }
+                CommitterClientResponse::RevertBlock(revert_response) => {
+                    self.handle_revert_result(commitment_task_output.height, revert_response)
+                        .await?;
                 }
             }
+        }
+        Ok(())
+    }
 
-            // Write the block hash and global root to storage.
-            self.storage_writer
-                .set_global_root_and_block_hash(height, global_root, block_hash)
-                .map_err(|err| {
-                    error!(
-                        "Failed to set global root and block hash in storage for {height}: {err}"
-                    );
-                    BatcherError::InternalError
-                })?;
+    async fn write_commitment_result_to_storage(
+        &mut self,
+        height: BlockNumber,
+        commit_response_result: CommitterClientResult<CommitBlockResponse>,
+    ) -> BatcherResult<()> {
+        // TODO: Handle commit block error.
+        let commit_response = commit_response_result.expect("Commit block error.");
+
+        // Decide whether to finalize the block hash based on the config.
+        let should_finalize_block_hash =
+            match self.config.first_block_with_partial_block_hash.as_ref() {
+                Some(FirstBlockWithPartialBlockHash { block_number, .. }) => {
+                    height >= *block_number
+                }
+                None => true,
+            };
+
+        // Get the final commitment.
+        let FinalBlockCommitment { height, block_hash, global_root } =
+            ApolloCommitmentManager::final_commitment_output(
+                self.storage_reader.clone(),
+                commit_response,
+                height,
+                should_finalize_block_hash,
+            )
+            .map_err(|err| {
+                error!("Failed to get the final commitment output for height {height}: {err}");
+                BatcherError::InternalError
+            })?;
+
+        // Verify the first new block hash matches the configured block hash.
+        if let Some(FirstBlockWithPartialBlockHash {
+            block_number,
+            block_hash: expected_block_hash,
+            ..
+        }) = self.config.first_block_with_partial_block_hash.as_ref()
+        {
+            if height == *block_number {
+                assert_eq!(
+                    *expected_block_hash,
+                    block_hash.expect(
+                        "The block hash of the first new block should be finalized and therefore \
+                         set."
+                    ),
+                    "The calculated block hash of the first new block ({block_hash:?}) does not \
+                     match the configured block hash ({expected_block_hash:?})"
+                );
+            }
         }
 
+        // Write the block hash and global root to storage.
+        self.storage_writer
+            .set_global_root_and_block_hash(height, global_root, block_hash)
+            .map_err(|err| {
+                error!("Failed to set global root and block hash in storage for {height}: {err}");
+                BatcherError::InternalError
+            })?;
+
         Ok(())
     }
 
+    async fn handle_revert_result(
+        &mut self,
+        _height: BlockNumber,
+        _revert_response: CommitterClientResult<RevertBlockResponse>,
+    ) -> BatcherResult<()> {
+        unimplemented!()
+    }
+
     pub fn get_block_hash(&self, block_number: BlockNumber) -> BatcherResult<BlockHash> {
         self.storage_reader
             .get_block_hash(block_number)
diff --git a/crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs b/crates/apollo_batcher/src/commitment_manager/commitment_manager_impl.rs
@@ -3,7 +3,8 @@
 use std::sync::Arc;
 
 use apollo_batcher_config::config::BatcherConfig;
-use apollo_committer_types::communication::SharedCommitterClient;
+use apollo_committer_types::committer_types::{CommitBlockRequest, CommitBlockResponse};
+use apollo_committer_types::communication::{CommitterRequest, SharedCommitterClient};
 use starknet_api::block::BlockNumber;
 use starknet_api::block_hash::block_hash_calculator::{
     calculate_block_hash,
@@ -110,7 +111,11 @@ impl<S: StateCommitterTrait> CommitmentManager<S> {
             });
         }
         let commitment_task_input =
-            CommitmentTaskInput { height, state_diff, state_diff_commitment };
+            CommitmentTaskInput(CommitterRequest::CommitBlock(CommitBlockRequest {
+                height,
+                state_diff,
+                state_diff_commitment,
+            }));
         let error_message = format!(
             "Failed to send commitment task to state committer. Block: {height}, state diff \
              commitment: {state_diff_commitment:?}",
@@ -262,13 +267,14 @@ impl<S: StateCommitterTrait> CommitmentManager<S> {
     // TODO(Rotem): Test this function.
     pub(crate) fn final_commitment_output<R: BatcherStorageReader + ?Sized>(
         storage_reader: Arc<R>,
-        CommitmentTaskOutput { height, global_root }: CommitmentTaskOutput,
+        CommitBlockResponse { state_root }: CommitBlockResponse,
+        height: BlockNumber,
         should_finalize_block_hash: bool,
     ) -> CommitmentManagerResult<FinalBlockCommitment> {
         match should_finalize_block_hash {
             false => {
                 info!("Finalized commitment for block {height} without calculating block hash.");
-                Ok(FinalBlockCommitment { height, block_hash: None, global_root })
+                Ok(FinalBlockCommitment { height, block_hash: None, global_root: state_root })
             }
             true => {
                 info!("Finalizing commitment for block {height} with calculating block hash.");
@@ -283,8 +289,12 @@ impl<S: StateCommitterTrait> CommitmentManager<S> {
                 let partial_block_hash_components = partial_block_hash_components
                     .ok_or(CommitmentManagerError::MissingPartialBlockHashComponents(height))?;
                 let block_hash =
-                    calculate_block_hash(&partial_block_hash_components, global_root, parent_hash)?;
-                Ok(FinalBlockCommitment { height, block_hash: Some(block_hash), global_root })
+                    calculate_block_hash(&partial_block_hash_components, state_root, parent_hash)?;
+                Ok(FinalBlockCommitment {
+                    height,
+                    block_hash: Some(block_hash),
+                    global_root: state_root,
+                })
             }
         }
     }
diff --git a/crates/apollo_batcher/src/commitment_manager/types.rs b/crates/apollo_batcher/src/commitment_manager/types.rs
@@ -1,20 +1,14 @@
 #![allow(dead_code)]
+use apollo_committer_types::communication::{CommitterClientResponse, CommitterRequest};
 use starknet_api::block::{BlockHash, BlockNumber};
-use starknet_api::core::{GlobalRoot, StateDiffCommitment};
-use starknet_api::state::ThinStateDiff;
+use starknet_api::core::GlobalRoot;
 
 /// Input for commitment tasks.
-pub(crate) struct CommitmentTaskInput {
-    pub(crate) state_diff: ThinStateDiff,
-    pub(crate) height: BlockNumber,
-    // Field is optional because for old blocks, the state diff commitment might not be available.
-    pub(crate) state_diff_commitment: Option<StateDiffCommitment>,
-}
+pub(crate) struct CommitmentTaskInput(pub(crate) CommitterRequest);
 
 /// Output of commitment tasks.
-#[cfg_attr(test, derive(Default))]
 pub(crate) struct CommitmentTaskOutput {
-    pub(crate) global_root: GlobalRoot,
+    pub(crate) committer_response: CommitterClientResponse,
     pub(crate) height: BlockNumber,
 }
 
diff --git a/crates/apollo_batcher/src/test_utils.rs b/crates/apollo_batcher/src/test_utils.rs
@@ -5,7 +5,12 @@ use std::sync::Arc;
 use apollo_batcher_config::config::{BatcherConfig, FirstBlockWithPartialBlockHash};
 use apollo_batcher_types::batcher_types::{ProposalId, ProposeBlockInput};
 use apollo_class_manager_types::{EmptyClassManagerClient, SharedClassManagerClient};
-use apollo_committer_types::communication::{MockCommitterClient, SharedCommitterClient};
+use apollo_committer_types::committer_types::CommitBlockResponse;
+use apollo_committer_types::communication::{
+    CommitterClientResponse,
+    MockCommitterClient,
+    SharedCommitterClient,
+};
 use apollo_l1_provider_types::MockL1ProviderClient;
 use apollo_mempool_types::communication::MockMempoolClient;
 use apollo_mempool_types::mempool_types::CommitBlockArgs;
@@ -340,8 +345,12 @@ impl MockStateCommitter {
     ) {
         while mock_task_receiver.recv().await.is_some() {
             let task = tasks_receiver.try_recv().unwrap();
-            let result =
-                CommitmentTaskOutput { global_root: GlobalRoot::default(), height: task.height };
+            let result = CommitmentTaskOutput {
+                committer_response: CommitterClientResponse::CommitBlock(Ok(CommitBlockResponse {
+                    state_root: GlobalRoot::default(),
+                })),
+                height: task.0.height(),
+            };
             results_sender.try_send(result).unwrap();
         }
     }
diff --git a/crates/apollo_committer_types/src/communication.rs b/crates/apollo_committer_types/src/communication.rs
@@ -10,6 +10,7 @@ use async_trait::async_trait;
 #[cfg(any(feature = "testing", test))]
 use mockall::automock;
 use serde::{Deserialize, Serialize};
+use starknet_api::block::BlockNumber;
 use strum::{EnumVariantNames, VariantNames};
 use strum_macros::{AsRefStr, EnumDiscriminants, EnumIter, IntoStaticStr};
 
@@ -58,6 +59,15 @@ impl_debug_for_infra_requests_and_responses!(CommitterRequest);
 impl_labeled_request!(CommitterRequest, CommitterRequestLabelValue);
 impl PrioritizedRequest for CommitterRequest {}
 
+impl CommitterRequest {
+    pub fn height(&self) -> BlockNumber {
+        match self {
+            CommitterRequest::CommitBlock(request) => request.height,
+            CommitterRequest::RevertBlock(request) => request.height,
+        }
+    }
+}
+
 #[derive(Clone, Serialize, Deserialize, AsRefStr)]
 pub enum CommitterResponse {
     CommitBlock(CommitterResult<CommitBlockResponse>),
@@ -71,6 +81,11 @@ generate_permutation_labels! {
     (LABEL_NAME_REQUEST_VARIANT, CommitterRequestLabelValue),
 }
 
+pub enum CommitterClientResponse {
+    CommitBlock(CommitterClientResult<CommitBlockResponse>),
+    RevertBlock(CommitterClientResult<RevertBlockResponse>),
+}
+
 #[async_trait]
 impl<ComponentClientType> CommitterClient for ComponentClientType
 where
