Consolidate store errors

crates/block-producer/src/block.rs

@@ -13,8 +13,6 @@ use miden_objects::{
     BlockHeader, Digest,
 };
 
-use crate::store::BlockInputsError;
-
 // BLOCK INPUTS
 // ================================================================================================
 
@@ -44,12 +42,12 @@ pub struct AccountWitness {
 }
 
 impl TryFrom<GetBlockInputsResponse> for BlockInputs {
-    type Error = BlockInputsError;
+    type Error = ConversionError;
 
     fn try_from(response: GetBlockInputsResponse) -> Result<Self, Self::Error> {
         let block_header: BlockHeader = response
             .block_header
-            .ok_or(GetBlockInputsResponse::missing_field(stringify!(block_header)))?
+            .ok_or(miden_node_proto::generated::block::BlockHeader::missing_field("block_header"))?
             .try_into()?;
 
         let chain_peaks = {

crates/block-producer/src/block_builder/mod.rs

@@ -135,7 +135,8 @@ impl BlockBuilder {
                 produced_nullifiers.iter(),
                 dangling_notes.iter(),
             )
-            .await?;
+            .await
+            .map_err(BuildBlockError::GetBlockInputsFailed)?;
 
         let missing_notes: Vec<_> = dangling_notes
             .difference(&block_inputs.found_unauthenticated_notes.note_ids())
@@ -160,7 +161,10 @@ impl BlockBuilder {
         info!(target: COMPONENT, block_num, %block_hash, "block built");
         debug!(target: COMPONENT, ?block);
 
-        self.store.apply_block(&block).await?;
+        self.store
+            .apply_block(&block)
+            .await
+            .map_err(BuildBlockError::ApplyBlockFailed)?;
 
         info!(target: COMPONENT, block_num, %block_hash, "block committed");
 

crates/block-producer/src/errors.rs

@@ -2,7 +2,7 @@ use miden_node_proto::errors::ConversionError;
 use miden_node_utils::formatting::format_opt;
 use miden_objects::{
     accounts::AccountId,
-    crypto::merkle::{MerkleError, MmrError},
+    crypto::merkle::MerkleError,
     notes::{NoteId, Nullifier},
     transaction::TransactionId,
     AccountDeltaError, Digest, TransactionInputError,
@@ -65,7 +65,7 @@ pub enum VerifyTxError {
     /// TODO: Make this an "internal error". Q: Should we have a single `InternalError` enum for
     /// all internal errors that can occur across the system?
     #[error("Failed to retrieve transaction inputs from the store: {0}")]
-    StoreConnectionFailed(#[from] TxInputsError),
+    StoreConnectionFailed(#[from] StoreError),
 
     #[error("Transaction input error: {0}")]
     TransactionInputError(#[from] TransactionInputError),
@@ -157,29 +157,6 @@ pub enum BlockProverError {
     InvalidRootOutput(&'static str),
 }
 
-// Block inputs errors
-// =================================================================================================
-
-#[allow(clippy::enum_variant_names)]
-#[derive(Debug, Error)]
-pub enum BlockInputsError {
-    #[error("failed to parse protobuf message: {0}")]
-    ConversionError(#[from] ConversionError),
-    #[error("MmrPeaks error: {0}")]
-    MmrPeaksError(#[from] MmrError),
-    #[error("gRPC client failed with error: {0}")]
-    GrpcClientError(String),
-}
-
-// Block applying errors
-// =================================================================================================
-
-#[derive(Debug, Error)]
-pub enum ApplyBlockError {
-    #[error("gRPC client failed with error: {0}")]
-    GrpcClientError(String),
-}
-
 // Block building errors
 // =================================================================================================
 
@@ -188,9 +165,9 @@ pub enum BuildBlockError {
     #[error("failed to compute new block: {0}")]
     BlockProverFailed(#[from] BlockProverError),
     #[error("failed to apply block: {0}")]
-    ApplyBlockFailed(#[from] ApplyBlockError),
+    ApplyBlockFailed(#[source] StoreError),
     #[error("failed to get block inputs from store: {0}")]
-    GetBlockInputsFailed(#[from] BlockInputsError),
+    GetBlockInputsFailed(#[source] StoreError),
     #[error("store did not produce data for account: {0}")]
     MissingAccountInput(AccountId),
     #[error("store produced extra account data. Offending accounts: {0:?}")]
@@ -210,17 +187,16 @@ pub enum BuildBlockError {
     InjectedFailure,
 }
 
-// Transaction inputs errors
+// Store errors
 // =================================================================================================
 
+/// Errors returned by the [StoreClient](crate::store::StoreClient).
 #[derive(Debug, Error)]
-pub enum TxInputsError {
-    #[error("gRPC client failed with error: {0}")]
-    GrpcClientError(String),
+pub enum StoreError {
+    #[error("gRPC client error")]
+    GrpcClientError(#[from] tonic::Status),
     #[error("malformed response from store: {0}")]
     MalformedResponse(String),
-    #[error("failed to parse protobuf message: {0}")]
-    ConversionError(#[from] ConversionError),
-    #[error("dummy")]
-    Dummy,
+    #[error("failed to parse response")]
+    DeserializationError(#[from] ConversionError),
 }

crates/block-producer/src/store/mod.rs

@@ -28,8 +28,7 @@ use miden_processor::crypto::RpoDigest;
 use tonic::transport::Channel;
 use tracing::{debug, info, instrument};
 
-pub use crate::errors::{ApplyBlockError, BlockInputsError, TxInputsError};
-use crate::{block::BlockInputs, COMPONENT};
+use crate::{block::BlockInputs, errors::StoreError, COMPONENT};
 
 // TRANSACTION INPUTS
 // ================================================================================================
@@ -132,24 +131,26 @@ impl StoreClient {
 
     /// Returns the latest block's header from the store.
     #[instrument(target = "miden-block-producer", skip_all, err)]
-    pub async fn latest_header(&self) -> Result<BlockHeader, String> {
-        // TODO: Consolidate the error types returned by the store (and its trait).
+    pub async fn latest_header(&self) -> Result<BlockHeader, StoreError> {
         let response = self
             .inner
             .clone()
             .get_block_header_by_number(tonic::Request::new(Default::default()))
-            .await
-            .map_err(|err| err.to_string())?
-            .into_inner();
-
-        BlockHeader::try_from(response.block_header.unwrap()).map_err(|err| err.to_string())
+            .await?
+            .into_inner()
+            .block_header
+            .ok_or(miden_node_proto::generated::block::BlockHeader::missing_field(
+                "block_header",
+            ))?;
+
+        BlockHeader::try_from(response).map_err(Into::into)
     }
 
     #[instrument(target = "miden-block-producer", skip_all, err)]
     pub async fn get_tx_inputs(
         &self,
         proven_tx: &ProvenTransaction,
-    ) -> Result<TransactionInputs, TxInputsError> {
+    ) -> Result<TransactionInputs, StoreError> {
         let message = GetTransactionInputsRequest {
             account_id: Some(proven_tx.account_id().into()),
             nullifiers: proven_tx.get_nullifiers().map(Into::into).collect(),
@@ -163,20 +164,14 @@ impl StoreClient {
         debug!(target: COMPONENT, ?message);
 
         let request = tonic::Request::new(message);
-        let response = self
-            .inner
-            .clone()
-            .get_transaction_inputs(request)
-            .await
-            .map_err(|status| TxInputsError::GrpcClientError(status.message().to_string()))?
-            .into_inner();
+        let response = self.inner.clone().get_transaction_inputs(request).await?.into_inner();
 
         debug!(target: COMPONENT, ?response);
 
         let tx_inputs: TransactionInputs = response.try_into()?;
 
         if tx_inputs.account_id != proven_tx.account_id() {
-            return Err(TxInputsError::MalformedResponse(format!(
+            return Err(StoreError::MalformedResponse(format!(
                 "incorrect account id returned from store. Got: {}, expected: {}",
                 tx_inputs.account_id,
                 proven_tx.account_id()
@@ -194,35 +189,22 @@ impl StoreClient {
         updated_accounts: impl Iterator<Item = AccountId> + Send,
         produced_nullifiers: impl Iterator<Item = &Nullifier> + Send,
         notes: impl Iterator<Item = &NoteId> + Send,
-    ) -> Result<BlockInputs, BlockInputsError> {
+    ) -> Result<BlockInputs, StoreError> {
         let request = tonic::Request::new(GetBlockInputsRequest {
             account_ids: updated_accounts.map(Into::into).collect(),
             nullifiers: produced_nullifiers.map(digest::Digest::from).collect(),
             unauthenticated_notes: notes.map(digest::Digest::from).collect(),
         });
 
-        let store_response = self
-            .inner
-            .clone()
-            .get_block_inputs(request)
-            .await
-            .map_err(|err| BlockInputsError::GrpcClientError(err.message().to_string()))?
-            .into_inner();
+        let store_response = self.inner.clone().get_block_inputs(request).await?.into_inner();
 
-        store_response.try_into()
+        store_response.try_into().map_err(Into::into)
     }
 
     #[instrument(target = "miden-block-producer", skip_all, err)]
-    pub async fn apply_block(&self, block: &Block) -> Result<(), ApplyBlockError> {
+    pub async fn apply_block(&self, block: &Block) -> Result<(), StoreError> {
         let request = tonic::Request::new(ApplyBlockRequest { block: block.to_bytes() });
 
-        let _ = self
-            .inner
-            .clone()
-            .apply_block(request)
-            .await
-            .map_err(|status| ApplyBlockError::GrpcClientError(status.message().to_string()))?;
-
-        Ok(())
+        self.inner.clone().apply_block(request).await.map(|_| ()).map_err(Into::into)
     }
 }

crates/block-producer/src/test_utils/store.rs

@@ -18,7 +18,8 @@ use super::*;
 use crate::{
     batch_builder::TransactionBatch,
     block::{AccountWitness, BlockInputs},
-    store::{ApplyBlockError, BlockInputsError, TransactionInputs, TxInputsError},
+    errors::StoreError,
+    store::TransactionInputs,
     test_utils::block::{
         block_output_notes, flatten_output_notes, note_created_smt_from_note_batches,
     },
@@ -183,7 +184,7 @@ impl MockStoreSuccess {
         locked_accounts.root()
     }
 
-    pub async fn apply_block(&self, block: &Block) -> Result<(), ApplyBlockError> {
+    pub async fn apply_block(&self, block: &Block) -> Result<(), StoreError> {
         // Intentionally, we take and hold both locks, to prevent calls to `get_tx_inputs()` from
         // going through while we're updating the store's data structure
         let mut locked_accounts = self.accounts.write().await;
@@ -238,7 +239,7 @@ impl MockStoreSuccess {
     pub async fn get_tx_inputs(
         &self,
         proven_tx: &ProvenTransaction,
-    ) -> Result<TransactionInputs, TxInputsError> {
+    ) -> Result<TransactionInputs, StoreError> {
         let locked_accounts = self.accounts.read().await;
         let locked_produced_nullifiers = self.produced_nullifiers.read().await;
 
@@ -286,7 +287,7 @@ impl MockStoreSuccess {
         updated_accounts: impl Iterator<Item = AccountId> + Send,
         produced_nullifiers: impl Iterator<Item = &Nullifier> + Send,
         notes: impl Iterator<Item = &NoteId> + Send,
-    ) -> Result<BlockInputs, BlockInputsError> {
+    ) -> Result<BlockInputs, StoreError> {
         let locked_accounts = self.accounts.read().await;
         let locked_produced_nullifiers = self.produced_nullifiers.read().await;
 

crates/proto/src/errors.rs

@@ -26,6 +26,8 @@ pub enum ConversionError {
         entity: &'static str,
         field_name: &'static str,
     },
+    #[error("MMR error: {0}")]
+    MmrError(#[from] miden_objects::crypto::merkle::MmrError),
 }
 
 pub trait MissingFieldHelper {