remove one chain state view + nits (#4623)

## Motivation

Shared state views should only be used for GraphQL / service queries.

## Proposal

Introduce a proper enum and return value (we were halfway there)

## Test Plan

CI

## Release Plan

- These changes should be backported to the latest `testnet` branch,
then
    - be released in a new SDK,

Author: ma2bd

linera-core/src/chain_worker/actor.rs

@@ -33,6 +33,7 @@ use tracing::{debug, instrument, trace, Instrument as _};
 
 use super::{config::ChainWorkerConfig, state::ChainWorkerState, DeliveryNotifier};
 use crate::{
+    chain_worker::BlockOutcome,
     data_types::{ChainInfoQuery, ChainInfoResponse},
     value_cache::ValueCache,
     worker::{NetworkActions, WorkerError},
@@ -100,7 +101,8 @@ where
     ProcessValidatedBlock {
         certificate: ValidatedBlockCertificate,
         #[debug(skip)]
-        callback: oneshot::Sender<Result<(ChainInfoResponse, NetworkActions, bool), WorkerError>>,
+        callback:
+            oneshot::Sender<Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError>>,
     },
 
     /// Process a confirmed block (a commit).
@@ -109,7 +111,8 @@ where
         #[debug(with = "elide_option")]
         notify_when_messages_are_delivered: Option<oneshot::Sender<()>>,
         #[debug(skip)]
-        callback: oneshot::Sender<Result<(ChainInfoResponse, NetworkActions), WorkerError>>,
+        callback:
+            oneshot::Sender<Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError>>,
     },
 
     /// Process a cross-chain update.

linera-core/src/chain_worker/mod.rs

@@ -14,4 +14,5 @@ pub(crate) use self::state::CrossChainUpdateHelper;
 pub(crate) use self::{
     actor::{ChainWorkerActor, ChainWorkerRequest},
     config::ChainWorkerConfig,
+    state::BlockOutcome,
 };

linera-core/src/chain_worker/state.rs

@@ -65,6 +65,13 @@ where
     knows_chain_is_active: bool,
 }
 
+/// Whether the block was processed or skipped. Used for metrics.
+pub enum BlockOutcome {
+    Processed,
+    Preprocessed,
+    Skipped,
+}
+
 impl<StorageClient> ChainWorkerState<StorageClient>
 where
     StorageClient: Storage + Clone + Send + Sync + 'static,
@@ -617,7 +624,7 @@ where
     pub(super) async fn process_validated_block(
         &mut self,
         certificate: ValidatedBlockCertificate,
-    ) -> Result<(ChainInfoResponse, NetworkActions, bool), WorkerError> {
+    ) -> Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError> {
         let block = certificate.block();
 
         let header = &block.header;
@@ -637,7 +644,11 @@ where
         };
         if already_committed_block || should_skip_validated_block()? {
             // If we just processed the same pending block, return the chain info unchanged.
-            return Ok((self.chain_info_response(), NetworkActions::default(), true));
+            return Ok((
+                self.chain_info_response(),
+                NetworkActions::default(),
+                BlockOutcome::Skipped,
+            ));
         }
 
         self.block_values
@@ -667,7 +678,7 @@ where
         )?;
         self.save().await?;
         let actions = self.create_network_actions(Some(old_round)).await?;
-        Ok((self.chain_info_response(), actions, false))
+        Ok((self.chain_info_response(), actions, BlockOutcome::Processed))
     }
 
     /// Processes a confirmed block (aka a commit).
@@ -680,7 +691,7 @@ where
         &mut self,
         certificate: ConfirmedBlockCertificate,
         notify_when_messages_are_delivered: Option<oneshot::Sender<()>>,
-    ) -> Result<(ChainInfoResponse, NetworkActions), WorkerError> {
+    ) -> Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError> {
         let block = certificate.block();
         let height = block.header.height;
         let chain_id = block.header.chain_id;
@@ -692,7 +703,7 @@ where
             let actions = self.create_network_actions(None).await?;
             self.register_delivery_notifier(height, &actions, notify_when_messages_are_delivered)
                 .await;
-            return Ok((self.chain_info_response(), actions));
+            return Ok((self.chain_info_response(), actions, BlockOutcome::Skipped));
         }
 
         // We haven't processed the block - verify the certificate first
@@ -771,7 +782,11 @@ where
             trace!("Preprocessed confirmed block {height} on chain {chain_id:.8}");
             self.register_delivery_notifier(height, &actions, notify_when_messages_are_delivered)
                 .await;
-            return Ok((self.chain_info_response(), actions));
+            return Ok((
+                self.chain_info_response(),
+                actions,
+                BlockOutcome::Preprocessed,
+            ));
         }
 
         // This should always be true for valid certificates.
@@ -878,7 +893,7 @@ where
         self.register_delivery_notifier(height, &actions, notify_when_messages_are_delivered)
             .await;
 
-        Ok((self.chain_info_response(), actions))
+        Ok((self.chain_info_response(), actions, BlockOutcome::Processed))
     }
 
     /// Schedules a notification for when cross-chain messages are delivered up to the given

linera-core/src/worker.rs

@@ -36,7 +36,9 @@ use tokio::sync::{mpsc, oneshot, OwnedRwLockReadGuard};
 use tracing::{error, instrument, trace, warn};
 
 use crate::{
-    chain_worker::{ChainWorkerActor, ChainWorkerConfig, ChainWorkerRequest, DeliveryNotifier},
+    chain_worker::{
+        BlockOutcome, ChainWorkerActor, ChainWorkerConfig, ChainWorkerRequest, DeliveryNotifier,
+    },
     data_types::{ChainInfoQuery, ChainInfoResponse, CrossChainRequest},
     join_set_ext::{JoinSet, JoinSetExt},
     notifier::Notifier,
@@ -600,7 +602,7 @@ where
         &self,
         certificate: ConfirmedBlockCertificate,
         notify_when_messages_are_delivered: Option<oneshot::Sender<()>>,
-    ) -> Result<(ChainInfoResponse, NetworkActions), WorkerError> {
+    ) -> Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError> {
         let chain_id = certificate.block().header.chain_id;
         self.query_chain_worker(chain_id, move |callback| {
             ChainWorkerRequest::ProcessConfirmedBlock {
@@ -621,7 +623,7 @@ where
     async fn process_validated_block(
         &self,
         certificate: ValidatedBlockCertificate,
-    ) -> Result<(ChainInfoResponse, NetworkActions, bool), WorkerError> {
+    ) -> Result<(ChainInfoResponse, NetworkActions, BlockOutcome), WorkerError> {
         let chain_id = certificate.block().header.chain_id;
         self.query_chain_worker(chain_id, move |callback| {
             ChainWorkerRequest::ProcessValidatedBlock {
@@ -876,37 +878,25 @@ where
     ) -> Result<(ChainInfoResponse, NetworkActions), WorkerError> {
         trace!("{} <-- {:?}", self.nickname, certificate);
         #[cfg(with_metrics)]
-        let metrics_data = if self
-            .chain_state_view(certificate.block().header.chain_id)
-            .await?
-            .tip_state
-            .get()
-            .next_block_height
-            == certificate.block().header.height
-        {
-            Some((
-                certificate.inner().to_log_str(),
-                certificate.round.type_name(),
-                certificate.round.number(),
-                certificate.block().body.transactions.len() as u64,
-                certificate
-                    .signatures()
-                    .iter()
-                    .map(|(validator_name, _)| validator_name.to_string())
-                    .collect::<Vec<_>>(),
-            ))
-        } else {
-            // Block already processed or will only be preprocessed, no metrics to report.
-            None
-        };
+        let metrics_data = (
+            certificate.inner().to_log_str(),
+            certificate.round.type_name(),
+            certificate.round.number(),
+            certificate.block().body.transactions.len() as u64,
+            certificate
+                .signatures()
+                .iter()
+                .map(|(validator_name, _)| validator_name.to_string())
+                .collect::<Vec<_>>(),
+        );
 
-        let result = self
+        let (info, actions, _outcome) = self
             .process_confirmed_block(certificate, notify_when_messages_are_delivered)
             .await?;
 
         #[cfg(with_metrics)]
         {
-            if let Some(metrics_data) = metrics_data {
+            if matches!(_outcome, BlockOutcome::Processed) {
                 let (
                     certificate_log_str,
                     round_type,
@@ -931,7 +921,7 @@ where
                 }
             }
         }
-        Ok(result)
+        Ok((info, actions))
     }
 
     /// Processes a validated block certificate.
@@ -951,10 +941,10 @@ where
         #[cfg(with_metrics)]
         let cert_str = certificate.inner().to_log_str();
 
-        let (info, actions, _duplicated) = self.process_validated_block(certificate).await?;
+        let (info, actions, _outcome) = self.process_validated_block(certificate).await?;
         #[cfg(with_metrics)]
         {
-            if !_duplicated {
+            if matches!(_outcome, BlockOutcome::Processed) {
                 metrics::NUM_ROUNDS_IN_CERTIFICATE
                     .with_label_values(&[cert_str, round.type_name()])
                     .observe(round.number() as f64);