Get rid of get_blobs_not_found and unify blob errors (#2649)

## Motivation

We've had these two different error variants for blobs for a while: `BlobNotFoundOnRead` and `BlobsNotFound`. The motivation was to differentiate between errors when doing a storage read, and everything else. In the current state of the code, I don't think this distinction is really giving us much, and it complicates the code more.

We also have this `get_blobs_not_found` function that abstracts away the different nested error types.

## Proposal

Remove `BlobNotFoundOnRead`, and simplify things a bit. Wrote custom/manual `From` implementations for these different error types, so we can have a `BlobsNotFound` error everywhere and can get rid of `get_blobs_not_found`.
Fixes #2628

## Test Plan

CI

## Release Plan

- Nothing to do / These changes follow the usual release cycle.

linera-chain/src/lib.rs

@@ -25,7 +25,7 @@ use data_types::{MessageBundle, Origin, PostedMessage};
 use linera_base::{
     crypto::{CryptoError, CryptoHash},
     data_types::{ArithmeticError, BlockHeight, Round, Timestamp},
-    identifiers::{ApplicationId, ChainId},
+    identifiers::{ApplicationId, BlobId, ChainId},
 };
 use linera_execution::ExecutionError;
 use linera_views::views::ViewError;
@@ -39,7 +39,7 @@ pub enum ChainError {
     #[error("Arithmetic error: {0}")]
     ArithmeticError(#[from] ArithmeticError),
     #[error("Error in view operation: {0}")]
-    ViewError(#[from] ViewError),
+    ViewError(ViewError),
     #[error("Execution error: {0} during {1:?}")]
     ExecutionError(Box<ExecutionError>, ChainExecutionContext),
 
@@ -157,6 +157,17 @@ pub enum ChainError {
         expected: CryptoHash,
         actual: CryptoHash,
     },
+    #[error("Blobs not found: {0:?}")]
+    BlobsNotFound(Vec<BlobId>),
+}
+
+impl From<ViewError> for ChainError {
+    fn from(error: ViewError) -> Self {
+        match error {
+            ViewError::BlobsNotFound(blob_ids) => ChainError::BlobsNotFound(blob_ids),
+            error => ChainError::ViewError(error),
+        }
+    }
 }
 
 #[derive(Copy, Clone, Debug)]

linera-core/src/client/mod.rs

@@ -383,12 +383,8 @@ where
                 .handle_certificate(certificate.clone().into(), vec![])
                 .await;
 
-            if let Some(blob_ids) = result
-                .as_ref()
-                .err()
-                .and_then(LocalNodeError::get_blobs_not_found)
-            {
-                if let Some(blobs) = remote_node.try_download_blobs(&blob_ids).await {
+            if let Err(LocalNodeError::BlobsNotFound(blob_ids)) = &result {
+                if let Some(blobs) = remote_node.try_download_blobs(blob_ids).await {
                     result = self.handle_certificate(certificate.into(), blobs).await;
                 }
             }
@@ -530,7 +526,7 @@ where
 #[derive(Debug, Error)]
 pub enum ChainClientError {
     #[error("Local node operation failed: {0}")]
-    LocalNodeError(#[from] LocalNodeError),
+    LocalNodeError(LocalNodeError),
 
     #[error("Remote node operation failed: {0}")]
     RemoteNodeError(#[from] NodeError),
@@ -542,7 +538,7 @@ pub enum ChainClientError {
     JsonError(#[from] serde_json::Error),
 
     #[error("Chain operation failed: {0}")]
-    ChainError(#[from] ChainError),
+    ChainError(ChainError),
 
     #[error(transparent)]
     CommunicationError(#[from] CommunicationError<NodeError>),
@@ -578,7 +574,7 @@ pub enum ChainClientError {
     FoundMultipleKeysForChain(ChainId),
 
     #[error(transparent)]
-    ViewError(#[from] ViewError),
+    ViewError(ViewError),
 
     #[error("Blobs not found: {0:?}")]
     BlobsNotFound(Vec<BlobId>),
@@ -593,6 +589,40 @@ pub enum ChainClientError {
     },
 }
 
+impl From<ViewError> for ChainClientError {
+    fn from(error: ViewError) -> Self {
+        match error {
+            ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
+            error => Self::ViewError(error),
+        }
+    }
+}
+
+impl From<LocalNodeError> for ChainClientError {
+    fn from(error: LocalNodeError) -> Self {
+        match error {
+            LocalNodeError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
+            error => Self::LocalNodeError(error),
+        }
+    }
+}
+
+impl From<ChainError> for ChainClientError {
+    fn from(error: ChainError) -> Self {
+        match error {
+            ChainError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
+            ChainError::ExecutionError(execution_error, context) => {
+                if let ExecutionError::BlobsNotFound(blob_ids) = *execution_error {
+                    Self::BlobsNotFound(blob_ids)
+                } else {
+                    Self::ChainError(ChainError::ExecutionError(execution_error, context))
+                }
+            }
+            error => Self::ChainError(error),
+        }
+    }
+}
+
 impl From<Infallible> for ChainClientError {
     fn from(infallible: Infallible) -> Self {
         match infallible {}
@@ -1229,7 +1259,7 @@ where
             .await
         {
             match &err {
-                LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) => {
+                LocalNodeError::BlobsNotFound(blob_ids) => {
                     let blobs = RemoteNode::download_blobs(blob_ids, &nodes)
                         .await
                         .ok_or(err)?;
@@ -1727,8 +1757,8 @@ where
                 .handle_block_proposal(*proposal.clone())
                 .await
             {
-                if let Some(blob_ids) = original_err.get_blobs_not_found() {
-                    self.update_local_node_with_blobs_from(blob_ids, remote_node)
+                if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
+                    self.update_local_node_with_blobs_from(blob_ids.clone(), remote_node)
                         .await?;
                     continue; // We found the missing blobs: retry.
                 }
@@ -1748,9 +1778,7 @@ where
                 .handle_certificate(Certificate::from(*cert.clone()), blobs)
                 .await
             {
-                if let LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) =
-                    &original_err
-                {
+                if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
                     if let Some(new_blobs) = remote_node
                         .find_missing_blobs(blob_ids.clone(), chain_id)
                         .await?
@@ -1887,11 +1915,10 @@ where
                 .local_node
                 .stage_block_execution(block.clone())
                 .await;
-            if let Err(err) = &result {
-                if let Some(blob_ids) = err.get_blobs_not_found() {
-                    self.receive_certificates_for_blobs(blob_ids).await?;
-                    continue; // We found the missing blob: retry.
-                }
+            if let Err(LocalNodeError::BlobsNotFound(blob_ids)) = &result {
+                self.receive_certificates_for_blobs(blob_ids.clone())
+                    .await?;
+                continue; // We found the missing blob: retry.
             }
             return Ok(result?);
         }

linera-core/src/local_node.rs

@@ -14,9 +14,9 @@ use linera_base::{
 use linera_chain::{
     data_types::{Block, BlockProposal, ExecutedBlock},
     types::{Certificate, CertificateValue, ConfirmedBlockCertificate, LiteCertificate},
-    ChainError, ChainStateView,
+    ChainStateView,
 };
-use linera_execution::{ExecutionError, Query, Response, SystemExecutionError};
+use linera_execution::{Query, Response};
 use linera_storage::Storage;
 use linera_views::views::ViewError;
 use thiserror::Error;
@@ -54,10 +54,10 @@ pub enum LocalNodeError {
     ArithmeticError(#[from] ArithmeticError),
 
     #[error(transparent)]
-    ViewError(#[from] linera_views::views::ViewError),
+    ViewError(ViewError),
 
     #[error("Local node operation failed: {0}")]
-    WorkerError(#[from] WorkerError),
+    WorkerError(WorkerError),
 
     #[error("Failed to read blob {blob_id:?} of chain {chain_id:?}")]
     CannotReadLocalBlob { chain_id: ChainId, blob_id: BlobId },
@@ -67,29 +67,25 @@ pub enum LocalNodeError {
 
     #[error("The chain info response received from the local node is invalid")]
     InvalidChainInfoResponse,
+
+    #[error("Blobs not found: {0:?}")]
+    BlobsNotFound(Vec<BlobId>),
 }
 
-impl LocalNodeError {
-    pub fn get_blobs_not_found(&self) -> Option<Vec<BlobId>> {
-        match self {
-            LocalNodeError::WorkerError(WorkerError::ChainError(chain_error)) => {
-                match &**chain_error {
-                    ChainError::ExecutionError(execution_error, _) => match **execution_error {
-                        ExecutionError::SystemError(SystemExecutionError::BlobNotFoundOnRead(
-                            blob_id,
-                        ))
-                        | ExecutionError::ViewError(ViewError::BlobNotFoundOnRead(blob_id)) => {
-                            Some(vec![blob_id])
-                        }
-                        _ => None,
-                    },
-                    _ => None,
-                }
-            }
-            LocalNodeError::WorkerError(WorkerError::BlobsNotFound(blob_ids)) => {
-                Some(blob_ids.clone())
-            }
-            _ => None,
+impl From<WorkerError> for LocalNodeError {
+    fn from(error: WorkerError) -> Self {
+        match error {
+            WorkerError::BlobsNotFound(blob_ids) => LocalNodeError::BlobsNotFound(blob_ids),
+            error => LocalNodeError::WorkerError(error),
+        }
+    }
+}
+
+impl From<ViewError> for LocalNodeError {
+    fn from(error: ViewError) -> Self {
+        match error {
+            ViewError::BlobsNotFound(blob_ids) => LocalNodeError::BlobsNotFound(blob_ids),
+            error => LocalNodeError::ViewError(error),
         }
     }
 }

linera-core/src/node.rs

@@ -19,7 +19,7 @@ use linera_chain::{
 };
 use linera_execution::{
     committee::{Committee, ValidatorName},
-    ExecutionError, SystemExecutionError,
+    ExecutionError,
 };
 use linera_version::VersionInfo;
 use linera_views::views::ViewError;
@@ -182,7 +182,7 @@ pub enum NodeError {
         height: BlockHeight,
     },
 
-    #[error("The following blobs are missing: {0:?}.")]
+    #[error("Blobs not found: {0:?}")]
     BlobsNotFound(Vec<BlobId>),
 
     // This error must be normalized during conversions.
@@ -220,8 +220,6 @@ pub enum NodeError {
     #[error("Failed to make a chain info query on the local node: {error}")]
     LocalNodeQuery { error: String },
 
-    #[error("Blob not found on storage read: {0}")]
-    BlobNotFoundOnRead(BlobId),
     #[error("Node failed to provide a 'last used by' certificate for the blob")]
     InvalidCertificateForBlob(BlobId),
     #[error("Local error handling validator response")]
@@ -255,8 +253,11 @@ impl CrossChainMessageDelivery {
 
 impl From<ViewError> for NodeError {
     fn from(error: ViewError) -> Self {
-        Self::ViewError {
-            error: error.to_string(),
+        match error {
+            ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
+            error => Self::ViewError {
+                error: error.to_string(),
+            },
         }
     }
 }
@@ -290,16 +291,16 @@ impl From<ChainError> for NodeError {
                 height,
             },
             ChainError::InactiveChain(chain_id) => Self::InactiveChain(chain_id),
-            ChainError::ExecutionError(execution_error, context) => match *execution_error {
-                ExecutionError::SystemError(SystemExecutionError::BlobNotFoundOnRead(blob_id))
-                | ExecutionError::ViewError(ViewError::BlobNotFoundOnRead(blob_id)) => {
-                    Self::BlobNotFoundOnRead(blob_id)
+            ChainError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
+            ChainError::ExecutionError(execution_error, context) => {
+                if let ExecutionError::BlobsNotFound(blob_ids) = *execution_error {
+                    Self::BlobsNotFound(blob_ids)
+                } else {
+                    Self::ChainError {
+                        error: ChainError::ExecutionError(execution_error, context).to_string(),
+                    }
                 }
-                execution_error => Self::ChainError {
-                    error: ChainError::ExecutionError(Box::new(execution_error), context)
-                        .to_string(),
-                },
-            },
+            }
             error => Self::ChainError {
                 error: error.to_string(),
             },
@@ -312,7 +313,7 @@ impl From<WorkerError> for NodeError {
         match error {
             WorkerError::ChainError(error) => (*error).into(),
             WorkerError::MissingCertificateValue => Self::MissingCertificateValue,
-            WorkerError::BlobsNotFound(blob_ids) => NodeError::BlobsNotFound(blob_ids),
+            WorkerError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
             error => Self::WorkerError {
                 error: error.to_string(),
             },

linera-core/src/unit_tests/test_utils.rs

@@ -275,7 +275,7 @@ where
         let handle_block_proposal_result =
             Self::handle_block_proposal(proposal, &mut validator).await;
         let result = match handle_block_proposal_result {
-            Some(Err(NodeError::BlobsNotFound(_) | NodeError::BlobNotFoundOnRead(_))) => {
+            Some(Err(NodeError::BlobsNotFound(_))) => {
                 handle_block_proposal_result.expect("handle_block_proposal_result should be Some")
             }
             _ => match validator.fault_type {
@@ -367,7 +367,7 @@ where
         let handle_certificate_result =
             Self::handle_certificate(certificate, validator, blobs).await;
         match handle_certificate_result {
-            Some(Err(NodeError::BlobsNotFound(_) | NodeError::BlobNotFoundOnRead(_))) => {
+            Some(Err(NodeError::BlobsNotFound(_))) => {
                 handle_certificate_result.expect("handle_certificate_result should be Some")
             }
             _ => match validator.fault_type {

linera-core/src/updater.rs

@@ -278,8 +278,8 @@ where
                     // synchronize them now and retry.
                     self.send_chain_information_for_senders(chain_id).await?;
                 }
-                Err(NodeError::BlobNotFoundOnRead(_)) if !blob_ids.is_empty() => {
-                    // For `BlobNotFoundOnRead`, we assume that the local node should already be
+                Err(NodeError::BlobsNotFound(_)) if !blob_ids.is_empty() => {
+                    // For `BlobsNotFound`, we assume that the local node should already be
                     // updated with the needed blobs, so sending the chain information about the
                     // certificates that last used the blobs to the validator node should be enough.
                     let missing_blob_ids = stream::iter(mem::take(&mut blob_ids))