refactor: define custom error values for consensus crate

node/actors/consensus/src/leader/error.rs

@@ -1,21 +1,55 @@
+use roles::validator;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 #[allow(clippy::missing_docs_in_private_items)]
 pub(crate) enum Error {
-    #[error("Received replica commit message for a proposal that we don't have.")]
-    MissingProposal,
-    #[error(transparent)]
-    Other(#[from] anyhow::Error),
+    #[error("received replica commit message for a missing proposal")]
+    ReplicaCommitMissingProposal,
+    #[error("received replica commit message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
+    ReplicaCommitOld {
+        current_view: validator::ViewNumber,
+        current_phase: validator::Phase,
+    },
+    #[error("received replica prepare message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
+    ReplicaPrepareOld {
+        current_view: validator::ViewNumber,
+        current_phase: validator::Phase,
+    },
+    #[error("received replica commit message for a view when we are not a leader")]
+    ReplicaCommitWhenNotLeaderInView,
+    #[error("received replica prepare message for a view when we are not a leader")]
+    ReplicaPrepareWhenNotLeaderInView,
+    #[error("received replica commit message that already exists (existing message: {existing_message:?}")]
+    ReplicaCommitExists { existing_message: String },
+    #[error("received replica prepare message that already exists (existing message: {existing_message:?}")]
+    ReplicaPrepareExists { existing_message: String },
+    #[error("received replica commit message while number of received messages is below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
+    ReplicaCommitNumReceivedBelowThreshold {
+        num_messages: usize,
+        threshold: usize,
+    },
+    #[error("received replica prepare message while number of received messages below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
+    ReplicaPrepareNumReceivedBelowThreshold {
+        num_messages: usize,
+        threshold: usize,
+    },
+    #[error("received replica prepare message with high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}")]
+    ReplicaPrepareHighQCOfFutureView {
+        high_qc_view: validator::ViewNumber,
+        current_view: validator::ViewNumber,
+    },
+    #[error("received replica commit message with invalid signature")]
+    ReplicaCommitInvalidSignature(#[source] crypto::bls12_381::Error),
+    #[error("received replica prepare message with invalid signature")]
+    ReplicaPrepareInvalidSignature(#[source] crypto::bls12_381::Error),
+    #[error("received replica prepare message with invalid high QC")]
+    ReplicaPrepareInvalidHighQC(#[source] anyhow::Error),
 }
 
-// TODO: This is only a temporary measure because anyhow::Error doesn't implement
-// PartialEq. When we change all errors to thiserror, we can just derive PartialEq.
+/// Needed due to source errors.
 impl PartialEq for Error {
     fn eq(&self, other: &Self) -> bool {
-        matches!(
-            (self, other),
-            (Error::MissingProposal, Error::MissingProposal) | (Error::Other(_), Error::Other(_))
-        )
+        self.to_string() == other.to_string()
     }
 }

node/actors/consensus/src/leader/replica_commit.rs

@@ -1,6 +1,5 @@
 use super::StateMachine;
 use crate::{inner::ConsensusInner, leader::error::Error, metrics};
-use anyhow::{anyhow, Context};
 use concurrency::{ctx, metrics::LatencyHistogramExt as _};
 use network::io::{ConsensusInputMessage, Target};
 use roles::validator;
@@ -22,15 +21,15 @@ impl StateMachine {
 
         // If the message is from the "past", we discard it.
         if (message.view, validator::Phase::Commit) < (self.view, self.phase) {
-            return Err(Error::Other(anyhow!("Received replica commit message for a past view/phase.\nCurrent view: {:?}\nCurrent phase: {:?}",
-                self.view, self.phase)));
+            return Err(Error::ReplicaCommitOld {
+                current_view: self.view,
+                current_phase: self.phase,
+            });
         }
 
         // If the message is for a view when we are not a leader, we discard it.
         if consensus.view_leader(message.view) != consensus.secret_key.public() {
-            return Err(Error::Other(anyhow!(
-                "Received replica commit message for a view when we are not a leader."
-            )));
+            return Err(Error::ReplicaCommitWhenNotLeaderInView);
         }
 
         // If we already have a message from the same validator and for the same view, we discard it.
@@ -39,25 +38,24 @@ impl StateMachine {
             .get(&message.view)
             .and_then(|x| x.get(author))
         {
-            return Err(Error::Other(anyhow!(
-                "Received replica commit message that we already have.\nExisting message: {:?}",
-                existing_message
-            )));
+            return Err(Error::ReplicaCommitExists {
+                existing_message: format!("{:?}", existing_message),
+            });
         }
 
         // ----------- Checking the signed part of the message --------------
 
         // Check the signature on the message.
         signed_message
             .verify()
-            .with_context(|| "Received replica commit message with invalid signature.")?;
+            .map_err(Error::ReplicaCommitInvalidSignature)?;
 
         // ----------- Checking the contents of the message --------------
 
         // We only accept replica commit messages for proposals that we have cached. That's so
         // we don't need to store replica commit messages for different proposals.
         if self.block_proposal_cache != Some(message) {
-            return Err(Error::MissingProposal);
+            return Err(Error::ReplicaCommitMissingProposal);
         }
 
         // ----------- All checks finished. Now we process the message. --------------
@@ -72,9 +70,10 @@ impl StateMachine {
         let num_messages = self.commit_message_cache.get(&message.view).unwrap().len();
 
         if num_messages < consensus.threshold() {
-            return Err(Error::Other(anyhow!("Received replica commit message. Waiting for more messages.\nCurrent number of messages: {}\nThreshold: {}",
-            num_messages,
-            consensus.threshold())));
+            return Err(Error::ReplicaCommitNumReceivedBelowThreshold {
+                num_messages,
+                threshold: consensus.threshold(),
+            });
         }
 
         debug_assert!(num_messages == consensus.threshold());

node/actors/consensus/src/leader/replica_prepare.rs

@@ -1,6 +1,5 @@
 use super::StateMachine;
 use crate::{inner::ConsensusInner, leader::error::Error, metrics};
-use anyhow::{anyhow, Context as _};
 use concurrency::ctx;
 use network::io::{ConsensusInputMessage, Target};
 use rand::Rng;
@@ -24,15 +23,15 @@ impl StateMachine {
 
         // If the message is from the "past", we discard it.
         if (message.view, validator::Phase::Prepare) < (self.view, self.phase) {
-            return Err(Error::Other(anyhow!("Received replica prepare message for a past view/phase.\nCurrent view: {:?}\nCurrent phase: {:?}",
-                self.view, self.phase)));
+            return Err(Error::ReplicaPrepareOld {
+                current_view: self.view,
+                current_phase: self.phase,
+            });
         }
 
         // If the message is for a view when we are not a leader, we discard it.
         if consensus.view_leader(message.view) != consensus.secret_key.public() {
-            return Err(Error::Other(anyhow!(
-                "Received replica prepare message for a view when we are not a leader."
-            )));
+            return Err(Error::ReplicaPrepareWhenNotLeaderInView);
         }
 
         // If we already have a message from the same validator and for the same view, we discard it.
@@ -41,35 +40,34 @@ impl StateMachine {
             .get(&message.view)
             .and_then(|x| x.get(author))
         {
-            return Err(Error::Other(anyhow!(
-                "Received replica prepare message that we already have.\nExisting message: {:?}",
-                existing_message
-            )));
+            return Err(Error::ReplicaPrepareExists {
+                existing_message: format!("{:?}", existing_message),
+            });
         }
 
         // ----------- Checking the signed part of the message --------------
 
         // Check the signature on the message.
         signed_message
             .verify()
-            .with_context(|| "Received replica prepare message with invalid signature.")?;
+            .map_err(Error::ReplicaPrepareInvalidSignature)?;
 
         // ----------- Checking the contents of the message --------------
 
         // Verify the high QC.
         message
             .high_qc
             .verify(&consensus.validator_set, consensus.threshold())
-            .with_context(|| "Received replica prepare message with invalid high QC.")?;
+            .map_err(Error::ReplicaPrepareInvalidHighQC)?;
 
         // If the high QC is for a future view, we discard the message.
         // This check is not necessary for correctness, but it's useful to
         // guarantee that our proposals don't contain QCs from the future.
         if message.high_qc.message.view >= message.view {
-            return Err(Error::Other(anyhow!(
-                "Received replica prepare message with a high QC from the future.\nHigh QC view: {:?}\nCurrent view: {:?}",
-                message.high_qc.message.view, message.view
-            )));
+            return Err(Error::ReplicaPrepareHighQCOfFutureView {
+                high_qc_view: message.high_qc.message.view,
+                current_view: message.view,
+            });
         }
 
         // ----------- All checks finished. Now we process the message. --------------
@@ -84,9 +82,10 @@ impl StateMachine {
         let num_messages = self.prepare_message_cache.get(&message.view).unwrap().len();
 
         if num_messages < consensus.threshold() {
-            return Err(Error::Other(anyhow!("Received replica prepare message. Waiting for more messages.\nCurrent number of messages: {}\nThreshold: {}",
-            num_messages,
-            consensus.threshold())));
+            return Err(Error::ReplicaPrepareNumReceivedBelowThreshold {
+                num_messages,
+                threshold: consensus.threshold(),
+            });
         }
 
         // ----------- Creating the block proposal --------------

node/actors/consensus/src/leader/tests.rs

@@ -36,6 +36,6 @@ async fn replica_commit() {
             &consensus.inner,
             test_replica_msg.cast().unwrap()
         ),
-        Err(Error::MissingProposal)
+        Err(Error::ReplicaCommitMissingProposal)
     );
 }

node/actors/consensus/src/replica/error.rs

@@ -0,0 +1,68 @@
+use crate::validator;
+use roles::validator::BlockHash;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+#[allow(clippy::missing_docs_in_private_items)]
+pub(crate) enum Error {
+    #[error("received leader commit message with invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})]")]
+    LeaderCommitInvalidLeader {
+        correct_leader: validator::PublicKey,
+        received_leader: validator::PublicKey,
+    },
+    #[error("received leader prepare message with invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?}])")]
+    LeaderPrepareInvalidLeader {
+        correct_leader: validator::PublicKey,
+        received_leader: validator::PublicKey,
+    },
+    #[error("received leader commit message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
+    LeaderCommitOld {
+        current_view: validator::ViewNumber,
+        current_phase: validator::Phase,
+    },
+    #[error("received leader commit message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
+    LeaderPrepareOld {
+        current_view: validator::ViewNumber,
+        current_phase: validator::Phase,
+    },
+    #[error("received leader commit message with invalid signature")]
+    LeaderCommitInvalidSignature(#[source] crypto::bls12_381::Error),
+    #[error("received leader prepare message with invalid signature")]
+    LeaderPrepareInvalidSignature(#[source] crypto::bls12_381::Error),
+    #[error("received leader commit message with invalid justification")]
+    LeaderCommitInvalidJustification(#[source] anyhow::Error),
+    #[error("received leader prepare message with empty map in the justification")]
+    LeaderPrepareJustificationWithEmptyMap,
+    #[error("received leader prepare message with invalid PrepareQC")]
+    LeaderPrepareInvalidPrepareQC(#[source] anyhow::Error),
+    #[error("received leader prepare message with invalid high QC")]
+    LeaderPrepareInvalidHighQC(#[source] anyhow::Error),
+    #[error("received leader prepare message with high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}")]
+    LeaderPrepareHighQCOfFutureView {
+        high_qc_view: validator::ViewNumber,
+        current_view: validator::ViewNumber,
+    },
+    #[error("received leader prepare message with new block proposal when the previous proposal was not finalized")]
+    LeaderPrepareProposalWhenPreviousNotFinalized,
+    #[error("received leader prepare message with new block proposal with invalid parent hash (correct parent hash: {correct_parent_hash:#?}, received parent hash: {received_parent_hash:#?}, block: {block:?})")]
+    LeaderPrepareProposalInvalidParentHash {
+        correct_parent_hash: BlockHash,
+        received_parent_hash: BlockHash,
+        block: validator::Block,
+    },
+    #[error("received leader prepare message with block proposal with non-sequential number (correct proposal number: {correct_number}, received proposal number: {received_number}, block: {block:?})")]
+    LeaderPrepareProposalNonSequentialNumber {
+        correct_number: u64,
+        received_number: u64,
+        block: validator::Block,
+    },
+    #[error("received leader prepare message with block proposal with an oversized payload (payload size: {payload_size}, block: {block:?}")]
+    LeaderPrepareProposalOversizedPayload {
+        payload_size: usize,
+        block: validator::Block,
+    },
+    #[error("received leader prepare message with block re-proposal when the previous proposal was finalized")]
+    LeaderPrepareReproposalWhenFinalized,
+    #[error("received leader prepare message with block re-proposal of invalid block")]
+    LeaderPrepareReproposalInvalidBlock,
+}

node/actors/consensus/src/replica/leader_commit.rs

@@ -1,6 +1,6 @@
 use super::StateMachine;
-use crate::ConsensusInner;
-use anyhow::{bail, Context};
+use crate::{inner::ConsensusInner, replica::error::Error};
+
 use concurrency::ctx;
 use roles::validator;
 use tracing::instrument;
@@ -14,7 +14,7 @@ impl StateMachine {
         ctx: &ctx::Ctx,
         consensus: &ConsensusInner,
         signed_message: validator::Signed<validator::LeaderCommit>,
-    ) -> anyhow::Result<()> {
+    ) -> Result<(), Error> {
         // ----------- Checking origin of the message --------------
 
         // Unwrap message.
@@ -24,34 +24,34 @@ impl StateMachine {
 
         // Check that it comes from the correct leader.
         if author != &consensus.view_leader(view) {
-            bail!(
-                "Received leader commit message with invalid leader.\nCorrect leader: {:?}\nReceived leader: {:?}",
-                consensus.view_leader(view), author
-            );
+            return Err(Error::LeaderCommitInvalidLeader {
+                correct_leader: consensus.view_leader(view),
+                received_leader: author.clone(),
+            });
         }
 
         // If the message is from the "past", we discard it.
         if (view, validator::Phase::Commit) < (self.view, self.phase) {
-            bail!(
-                "Received leader commit message for a past view/phase.\nCurrent view: {:?}\nCurrent phase: {:?}",
-                self.view, self.phase
-            );
+            return Err(Error::LeaderCommitOld {
+                current_view: self.view,
+                current_phase: self.phase,
+            });
         }
 
         // ----------- Checking the signed part of the message --------------
 
         // Check the signature on the message.
         signed_message
             .verify()
-            .with_context(|| "Received leader commit message with invalid signature.")?;
+            .map_err(Error::LeaderCommitInvalidSignature)?;
 
         // ----------- Checking the justification of the message --------------
 
         // Verify the QuorumCertificate.
         message
             .justification
             .verify(&consensus.validator_set, consensus.threshold())
-            .with_context(|| "Received leader commit message with invalid justification.")?;
+            .map_err(Error::LeaderCommitInvalidJustification)?;
 
         // ----------- All checks finished. Now we process the message. --------------