Remove BlobsNotFound / EventsNotFound from the ViewError and make the read_blob / read_event return an option. (#4014)

## Motivation

The `ViewError` has the possibilities `BlobsNotFound` and
`EventsNotFound` which is a leakage of abstraction
as it should not be there.

## Proposal

There are several considerations for this work. The first is to
recognize that while the use of the `ViewError` to park
the error is inadequate, it is certainly practical.

**Possibility 1**: One possibility is to introduce a specific error
type:
```rust
enum StorageError {
  BlobsNotFound(Vec<BlobId>),
  EventsNotFound(Vec<EventId>),
  ViewError(ViewError),
}
```

while this addresses the concern with `ViewError` it complexifies the
code significantly. Worse it presents another problem.
Suppose that a crate is using the `read_blob` but not the `read_event`.
Then it has to handle the possibility of an `EventsNotFound`. Of course
this could never materialize, but the type system implies that either it
is marked as unreachable, or an entry is added to the corresponding
error type.

**Possibility 2**: Add a `ReadBlobError` and a `ReadEventError`. This is
satisfying from the type system, though unwieldly.

**Possibility 3**: Replace the `read_blob` by `maybe_read_blob` and
return an Option. The caller is then responsible for handling the None
case and decide what to do in that case. This is the chosen solution.
It simplifies away quite some code but introduce some complicacies
elsewhere. This is preferable: The handling of missing case is the
responsibility of the caller.

**Note 1**: The same transformation is probably possible for
`read_blob_state(s)`. For `read_certificate(s)` this looks more
complicated and a `ReadCertificateError` might be the right solution
here. This should be handled in subsequent PR.

**Note 2**: The plural `BlobsNotFound` / `EventsNotFound` is kept.
However, an examination of the code shows that only singular are built.
Maybe there is an underlying issue to address.

Other points:
* The `BlobsNotFound` has been replaced from `ChainError` as it is never
built.
* The `read_network_description` has been changed into
`maybe_read_network_description` as an Option is returned.

## Test Plan

The CI.

## Release Plan

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

## Links

None.

Author: MathieuDutSik

linera-chain/src/lib.rs

@@ -28,7 +28,7 @@ use linera_base::{
     bcs,
     crypto::{CryptoError, CryptoHash},
     data_types::{ArithmeticError, BlockHeight, Round, Timestamp},
-    identifiers::{ApplicationId, BlobId, ChainId},
+    identifiers::{ApplicationId, ChainId},
 };
 use linera_execution::ExecutionError;
 use linera_views::views::ViewError;
@@ -42,7 +42,7 @@ pub enum ChainError {
     #[error(transparent)]
     ArithmeticError(#[from] ArithmeticError),
     #[error(transparent)]
-    ViewError(ViewError),
+    ViewError(#[from] ViewError),
     #[error("Execution error: {0} during {1:?}")]
     ExecutionError(Box<ExecutionError>, ChainExecutionContext),
 
@@ -168,17 +168,6 @@ 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-client/src/chain_listener.rs

@@ -25,7 +25,6 @@ use linera_core::{
     Environment,
 };
 use linera_storage::{Clock as _, Storage as _};
-use linera_views::views::ViewError;
 use tokio_util::sync::CancellationToken;
 use tracing::{debug, info, instrument, warn, Instrument as _};
 
@@ -98,8 +97,8 @@ pub trait ClientContextExt: ClientContext {
         let blob_id = BlobId::new(chain_id.0, BlobType::ChainDescription);
 
         let blob = match self.storage().read_blob(blob_id).await {
-            Ok(blob) => blob,
-            Err(ViewError::BlobsNotFound(blob_ids)) if blob_ids == [blob_id] => {
+            Ok(Some(blob)) => blob,
+            Ok(None) => {
                 // we're missing the blob describing the chain we're assigning - try to
                 // get it
                 self.client().ensure_has_chain_description(chain_id).await?

linera-core/src/chain_worker/state/mod.rs

@@ -318,7 +318,8 @@ where
         if let Some(blob) = self.chain.manager.pending_blob(&blob_id).await? {
             return Ok(blob);
         }
-        Ok(self.storage.read_blob(blob_id).await?)
+        let blob = self.storage.read_blob(blob_id).await?;
+        blob.ok_or(WorkerError::BlobsNotFound(vec![blob_id]))
     }
 
     /// Adds the blob to pending blocks or validated block certificates that are missing it.

linera-core/src/client/mod.rs

@@ -473,15 +473,15 @@ impl<Env: Environment> Client<Env> {
         chain_id: ChainId,
     ) -> Result<Blob, ChainClientError> {
         let chain_desc_id = BlobId::new(chain_id.0, BlobType::ChainDescription);
-        if let Ok(blob) = self
+        let blob = self
             .local_node
             .storage_client()
             .read_blob(chain_desc_id)
-            .await
-        {
+            .await?;
+        if let Some(blob) = blob {
             // We have the blob - return it.
             return Ok(blob);
-        }
+        };
         // Recover history from the current validators, according to the admin chain.
         // TODO(#2351): make sure that the blob is legitimately created!
         let nodes = self.validator_nodes().await?;
@@ -3300,11 +3300,12 @@ impl<Env: Environment> ChainClient<Env> {
             stream_id: StreamId::system(stream_name),
             index,
         };
-        match self.client.storage_client().read_event(event_id).await {
-            Ok(_) => Ok(true),
-            Err(ViewError::EventsNotFound(_)) => Ok(false),
-            Err(error) => Err(error.into()),
-        }
+        Ok(self
+            .client
+            .storage_client()
+            .read_event(event_id)
+            .await?
+            .is_some())
     }
 
     /// Returns the indices and events from the storage

linera-core/src/local_node.rs

@@ -55,7 +55,7 @@ pub enum LocalNodeError {
     ArithmeticError(#[from] ArithmeticError),
 
     #[error(transparent)]
-    ViewError(ViewError),
+    ViewError(#[from] ViewError),
 
     #[error("Local node operation failed: {0}")]
     WorkerError(WorkerError),
@@ -82,15 +82,6 @@ impl From<WorkerError> for LocalNodeError {
     }
 }
 
-impl From<ViewError> for LocalNodeError {
-    fn from(error: ViewError) -> Self {
-        match error {
-            ViewError::BlobsNotFound(blob_ids) => LocalNodeError::BlobsNotFound(blob_ids),
-            error => LocalNodeError::ViewError(error),
-        }
-    }
-}
-
 impl<S> LocalNodeClient<S>
 where
     S: Storage + Clone + Send + Sync + 'static,

linera-core/src/node.rs

@@ -288,11 +288,8 @@ impl CrossChainMessageDelivery {
 
 impl From<ViewError> for NodeError {
     fn from(error: ViewError) -> Self {
-        match error {
-            ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
-            error => Self::ViewError {
-                error: error.to_string(),
-            },
+        Self::ViewError {
+            error: error.to_string(),
         }
     }
 }
@@ -326,7 +323,6 @@ impl From<ChainError> for NodeError {
                 height,
             },
             ChainError::InactiveChain(chain_id) => Self::InactiveChain(chain_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)

linera-core/src/unit_tests/test_utils.rs

@@ -513,6 +513,10 @@ where
             .read_blob(blob_id)
             .await
             .map_err(Into::into);
+        let blob = match blob {
+            Ok(blob) => blob.ok_or(NodeError::BlobsNotFound(vec![blob_id])),
+            Err(error) => Err(error),
+        };
         sender.send(blob.map(|blob| blob.into_content()))
     }
 

linera-core/src/worker.rs

@@ -150,7 +150,7 @@ pub enum WorkerError {
     ArithmeticError(#[from] ArithmeticError),
 
     #[error(transparent)]
-    ViewError(ViewError),
+    ViewError(#[from] ViewError),
 
     #[error(transparent)]
     ChainError(#[from] Box<ChainError>),
@@ -221,7 +221,6 @@ impl From<ChainError> for WorkerError {
     #[instrument(level = "trace", skip(chain_error))]
     fn from(chain_error: ChainError) -> Self {
         match chain_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)
@@ -237,15 +236,6 @@ impl From<ChainError> for WorkerError {
     }
 }
 
-impl From<ViewError> for WorkerError {
-    fn from(view_error: ViewError) -> Self {
-        match view_error {
-            ViewError::BlobsNotFound(blob_ids) => Self::BlobsNotFound(blob_ids),
-            error => Self::ViewError(error),
-        }
-    }
-}
-
 #[cfg(with_testing)]
 impl WorkerError {
     /// Returns the inner [`ExecutionError`] in this error.

linera-execution/src/execution_state_actor.rs

@@ -452,8 +452,9 @@ where
             }
 
             ReadEvent { event_id, callback } => {
-                let event_value = self.context().extra().get_event(event_id).await?;
-                callback.respond(event_value);
+                let event = self.context().extra().get_event(event_id.clone()).await?;
+                let event = event.ok_or(ExecutionError::EventsNotFound(vec![event_id]))?;
+                callback.respond(event);
             }
 
             SubscribeToEvents {

linera-execution/src/lib.rs

@@ -193,7 +193,7 @@ const _: () = {
 #[derive(Error, Debug)]
 pub enum ExecutionError {
     #[error(transparent)]
-    ViewError(ViewError),
+    ViewError(#[from] ViewError),
     #[error(transparent)]
     ArithmeticError(#[from] ArithmeticError),
     #[error("User application reported an error: {0}")]
@@ -283,6 +283,8 @@ pub enum ExecutionError {
     ServiceModuleSend(#[from] linera_base::task::SendError<UserServiceCode>),
     #[error("Blobs not found: {0:?}")]
     BlobsNotFound(Vec<BlobId>),
+    #[error("Events not found: {0:?}")]
+    EventsNotFound(Vec<EventId>),
 
     #[error("Invalid HTTP header name used for HTTP request")]
     InvalidHeaderName(#[from] reqwest::header::InvalidHeaderName),
@@ -342,15 +344,6 @@ pub enum ExecutionError {
     OutdatedUpdateStreams,
 }
 
-impl From<ViewError> for ExecutionError {
-    fn from(error: ViewError) -> Self {
-        match error {
-            ViewError::BlobsNotFound(blob_ids) => ExecutionError::BlobsNotFound(blob_ids),
-            error => ExecutionError::ViewError(error),
-        }
-    }
-}
-
 /// The public entry points provided by the contract part of an application.
 pub trait UserContract {
     /// Instantiate the application state on the chain that owns the application.
@@ -402,9 +395,9 @@ pub trait ExecutionRuntimeContext {
         description: &ApplicationDescription,
     ) -> Result<UserServiceCode, ExecutionError>;
 
-    async fn get_blob(&self, blob_id: BlobId) -> Result<Blob, ViewError>;
+    async fn get_blob(&self, blob_id: BlobId) -> Result<Option<Blob>, ViewError>;
 
-    async fn get_event(&self, event_id: EventId) -> Result<Vec<u8>, ViewError>;
+    async fn get_event(&self, event_id: EventId) -> Result<Option<Vec<u8>>, ViewError>;
 
     async fn get_network_description(&self) -> Result<Option<NetworkDescription>, ViewError>;
 
@@ -1070,20 +1063,18 @@ impl ExecutionRuntimeContext for TestExecutionRuntimeContext {
             .clone())
     }
 
-    async fn get_blob(&self, blob_id: BlobId) -> Result<Blob, ViewError> {
-        Ok(self
-            .blobs
-            .get(&blob_id)
-            .ok_or_else(|| ViewError::BlobsNotFound(vec![blob_id]))?
-            .clone())
+    async fn get_blob(&self, blob_id: BlobId) -> Result<Option<Blob>, ViewError> {
+        match self.blobs.get(&blob_id) {
+            None => Ok(None),
+            Some(blob) => Ok(Some(blob.clone())),
+        }
     }
 
-    async fn get_event(&self, event_id: EventId) -> Result<Vec<u8>, ViewError> {
-        Ok(self
-            .events
-            .get(&event_id)
-            .ok_or_else(|| ViewError::EventsNotFound(vec![event_id]))?
-            .clone())
+    async fn get_event(&self, event_id: EventId) -> Result<Option<Vec<u8>>, ViewError> {
+        match self.events.get(&event_id) {
+            None => Ok(None),
+            Some(event) => Ok(Some(event.clone())),
+        }
     }
 
     async fn get_network_description(&self) -> Result<Option<NetworkDescription>, ViewError> {