handle_eqc_voting changes

EspressoSystems · Jan 6, 2025 · 25e8939 · 25e8939
1 parent 7eb945c
commit 25e8939
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 42 deletions.
diff --git a/crates/task-impls/src/quorum_vote/mod.rs b/crates/task-impls/src/quorum_vote/mod.rs
@@ -499,7 +499,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
 
                 if version >= V::Epochs::VERSION && is_justify_qc_forming_eqc {
                     self.handle_eqc_voting(proposal, parent_leaf, event_sender, event_receiver)
-                        .await;
+                        .await?;
                 } else {
                     self.create_dependency_task_if_new(
                         proposal.data.view_number,
@@ -658,55 +658,56 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
         parent_leaf: &Leaf2<TYPES>,
         event_sender: Sender<Arc<HotShotEvent<TYPES>>>,
         event_receiver: Receiver<Arc<HotShotEvent<TYPES>>>,
-    ) {
+    ) -> Result<()> {
         tracing::info!("Reached end of epoch. Justify QC is for the last block in the epoch.");
         let proposed_leaf = Leaf2::from_quorum_proposal(&proposal.data);
         let parent_commitment = parent_leaf.commit();
-        if proposed_leaf.height() != parent_leaf.height()
-            || proposed_leaf.payload_commitment() != parent_leaf.payload_commitment()
-        {
-            tracing::error!("Justify QC is for the last block but it's not extended and a new block is proposed. Not voting!");
-            return;
-        }
+
+        ensure!(
+            proposed_leaf.height() == parent_leaf.height() && proposed_leaf.payload_commitment() == parent_leaf.payload_commitment(),
+            error!("Justify QC is for the last block but it's not extended and a new block is proposed. Not voting!")
+        );
 
         tracing::info!(
             "Reached end of epoch. Proposed leaf has the same height and payload as its parent."
         );
 
         let mut consensus_writer = self.consensus.write().await;
-        let Some(vid_shares) = consensus_writer
+
+        let vid_shares = consensus_writer
             .vid_shares()
             .get(&parent_leaf.view_number())
-        else {
-            tracing::warn!(
-                "Proposed leaf is the same as its parent but we don't have our VID for it"
-            );
-            return;
-        };
-        let Some(vid) = vid_shares.get(&self.public_key) else {
-            tracing::warn!(
+            .context(warn!(
                 "Proposed leaf is the same as its parent but we don't have our VID for it"
-            );
-            return;
-        };
+            ))?;
+
+        let vid = vid_shares.get(&self.public_key).context(warn!(
+            "Proposed leaf is the same as its parent but we don't have our VID for it"
+        ))?;
+
         let mut updated_vid = vid.clone();
         updated_vid.data.view_number = proposal.data.view_number;
         consensus_writer.update_vid_shares(updated_vid.data.view_number, updated_vid.clone());
+
         drop(consensus_writer);
 
-        if proposed_leaf.parent_commitment() != parent_commitment {
-            tracing::warn!("Proposed leaf parent commitment does not match parent leaf payload commitment. Aborting vote.");
-            return;
-        }
+        ensure!(
+            proposed_leaf.parent_commitment() == parent_commitment,
+            warn!("Proposed leaf parent commitment does not match parent leaf payload commitment. Aborting vote.")
+        );
+
         // Update our persistent storage of the proposal. If we cannot store the proposal return
         // and error so we don't vote
-        if let Err(e) = self.storage.write().await.append_proposal2(proposal).await {
-            tracing::error!("failed to store proposal, not voting.  error = {e:#}");
-            return;
-        }
+        self.storage
+            .write()
+            .await
+            .append_proposal2(proposal)
+            .await
+            .wrap()
+            .context(|e| error!("failed to store proposal, not voting. error = {}", e))?;
 
         // Update internal state
-        if let Err(e) = update_shared_state::<TYPES, I, V>(
+        update_shared_state::<TYPES, I, V>(
             OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)),
             event_sender.clone(),
             event_receiver.clone().deactivate(),
@@ -723,10 +724,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
             self.epoch_height,
         )
         .await
-        {
-            tracing::error!("Failed to update shared consensus state; error = {e:#}");
-            return;
-        }
+        .context(error!("Failed to update shared consensus state"))?;
 
         let current_block_number = proposed_leaf.height();
         let current_epoch = TYPES::Epoch::new(epoch_from_block_number(
@@ -757,7 +755,8 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
             )
             .await;
         }
-        if let Err(e) = submit_vote::<TYPES, I, V>(
+
+        submit_vote::<TYPES, I, V>(
             event_sender.clone(),
             Arc::clone(&self.membership),
             self.public_key.clone(),
@@ -771,9 +770,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
             self.epoch_height,
         )
         .await
-        {
-            tracing::debug!("Failed to vote; error = {e:#}");
-        }
+        .context(debug!("Failed to submit vote"))
     }
 }
 

diff --git a/crates/utils/src/anytrace.rs b/crates/utils/src/anytrace.rs
@@ -147,15 +147,15 @@ fn concatenate(error: &String, cause: &String) -> String {
 }
 
 /// Trait for converting error types to a `Result<T>`.
-pub trait Context<T> {
+pub trait Context<T, E> {
     /// Attach context to the given error.
     ///
     /// # Errors
     /// Propagates errors from `self`
-    fn context(self, error: Error) -> Result<T>;
+    fn context(self, error: E) -> Result<T>;
 }
 
-impl<T> Context<T> for Result<T> {
+impl<T> Context<T, Error> for Result<T> {
     fn context(self, error: Error) -> Result<T> {
         match self {
             Ok(t) => Ok(t),
@@ -167,7 +167,22 @@ impl<T> Context<T> for Result<T> {
     }
 }
 
-impl<T> Context<T> for Option<T> {
+impl<T, F> Context<T, F> for Result<T>
+where
+    F: Fn(Error) -> Error,
+{
+    fn context(self, error: F) -> Result<T> {
+        match self {
+            Ok(t) => Ok(t),
+            Err(cause) => Err(Error {
+                level: max(error(cause.clone()).level, cause.level),
+                message: concatenate(&error(cause.clone()).message, &format!("{cause}")),
+            }),
+        }
+    }
+}
+
+impl<T> Context<T, Error> for Option<T> {
     fn context(self, error: Error) -> Result<T> {
         match self {
             Some(t) => Ok(t),
@@ -176,7 +191,7 @@ impl<T> Context<T> for Option<T> {
     }
 }
 
-impl<'a, T> Context<&'a mut T> for &'a mut Option<T> {
+impl<'a, T> Context<&'a mut T, Error> for &'a mut Option<T> {
     fn context(self, error: Error) -> Result<&'a mut T> {
         match self {
             Some(t) => Ok(t),