PR fixes

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

@@ -3,52 +3,52 @@ use thiserror::Error;
 
 #[derive(Error, Debug)]
 #[allow(clippy::missing_docs_in_private_items)]
-pub(crate) enum ReplicaMessageError {
-    #[error("commit for a missing proposal")]
-    CommitMissingProposal,
-    #[error("commit for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
-    CommitOld {
+pub(crate) enum 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("prepare for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
-    PrepareOld {
+    #[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("commit for a view when we are not a leader")]
-    CommitWhenNotLeaderInView,
-    #[error("prepare for a view when we are not a leader")]
-    PrepareWhenNotLeaderInView,
-    #[error("commit duplicated (existing message: {existing_message:?}")]
-    CommitDuplicated { existing_message: String },
-    #[error("prepare duplicated (existing message: {existing_message:?}")]
-    PrepareDuplicated { existing_message: String },
-    #[error("commit while number of received messages below threshold, waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
-    CommitNumReceivedBelowThreshold {
+    #[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("prepare while number of received messages below threshold, waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
-    PrepareNumReceivedBelowThreshold {
+    #[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("prepare with high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}")]
-    PrepareHighQCOfFutureView {
+    #[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("commit with invalid signature")]
-    CommitInvalidSignature { inner_err: crypto::bls12_381::Error },
-    #[error("prepare with invalid signature")]
-    PrepareInvalidSignature { inner_err: crypto::bls12_381::Error },
-    #[error("prepare with invalid high QC")]
-    PrepareInvalidHighQC { inner_err: anyhow::Error },
+    #[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),
 }
 
-/// Needed due to inner errors.
-impl PartialEq for ReplicaMessageError {
+/// Needed due to source errors.
+impl PartialEq for Error {
     fn eq(&self, other: &Self) -> bool {
         self.to_string() == other.to_string()
     }

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

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::{inner::ConsensusInner, leader::error::ReplicaMessageError, metrics};
+use crate::{inner::ConsensusInner, leader::error::Error, metrics};
 use concurrency::{ctx, metrics::LatencyHistogramExt as _};
 use network::io::{ConsensusInputMessage, Target};
 use roles::validator;
@@ -12,7 +12,7 @@ impl StateMachine {
         ctx: &ctx::Ctx,
         consensus: &ConsensusInner,
         signed_message: validator::Signed<validator::ReplicaCommit>,
-    ) -> Result<(), ReplicaMessageError> {
+    ) -> Result<(), Error> {
         // ----------- Checking origin of the message --------------
 
         // Unwrap message.
@@ -21,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(ReplicaMessageError::CommitOld {
+            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(ReplicaMessageError::CommitWhenNotLeaderInView);
+            return Err(Error::ReplicaCommitWhenNotLeaderInView);
         }
 
         // If we already have a message from the same validator and for the same view, we discard it.
@@ -38,7 +38,7 @@ impl StateMachine {
             .get(&message.view)
             .and_then(|x| x.get(author))
         {
-            return Err(ReplicaMessageError::CommitDuplicated {
+            return Err(Error::ReplicaCommitExists {
                 existing_message: format!("{:?}", existing_message),
             });
         }
@@ -48,14 +48,14 @@ impl StateMachine {
         // Check the signature on the message.
         signed_message
             .verify()
-            .map_err(|err| ReplicaMessageError::CommitInvalidSignature { inner_err: err })?;
+            .map_err(|err| Error::ReplicaCommitInvalidSignature(err))?;
 
         // ----------- 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(ReplicaMessageError::CommitMissingProposal);
+            return Err(Error::ReplicaCommitMissingProposal);
         }
 
         // ----------- All checks finished. Now we process the message. --------------
@@ -70,7 +70,7 @@ impl StateMachine {
         let num_messages = self.commit_message_cache.get(&message.view).unwrap().len();
 
         if num_messages < consensus.threshold() {
-            return Err(ReplicaMessageError::CommitNumReceivedBelowThreshold {
+            return Err(Error::ReplicaCommitNumReceivedBelowThreshold {
                 num_messages,
                 threshold: consensus.threshold(),
             });

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

@@ -1,5 +1,5 @@
 use super::StateMachine;
-use crate::{inner::ConsensusInner, leader::error::ReplicaMessageError, metrics};
+use crate::{inner::ConsensusInner, leader::error::Error, metrics};
 use concurrency::ctx;
 use network::io::{ConsensusInputMessage, Target};
 use rand::Rng;
@@ -14,7 +14,7 @@ impl StateMachine {
         ctx: &ctx::Ctx,
         consensus: &ConsensusInner,
         signed_message: validator::Signed<validator::ReplicaPrepare>,
-    ) -> Result<(), ReplicaMessageError> {
+    ) -> Result<(), Error> {
         // ----------- Checking origin of the message --------------
 
         // Unwrap message.
@@ -23,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(ReplicaMessageError::PrepareOld {
+            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(ReplicaMessageError::PrepareWhenNotLeaderInView);
+            return Err(Error::ReplicaPrepareWhenNotLeaderInView);
         }
 
         // If we already have a message from the same validator and for the same view, we discard it.
@@ -40,7 +40,7 @@ impl StateMachine {
             .get(&message.view)
             .and_then(|x| x.get(author))
         {
-            return Err(ReplicaMessageError::PrepareDuplicated {
+            return Err(Error::ReplicaPrepareExists {
                 existing_message: format!("{:?}", existing_message),
             });
         }
@@ -50,21 +50,21 @@ impl StateMachine {
         // Check the signature on the message.
         signed_message
             .verify()
-            .map_err(|err| ReplicaMessageError::PrepareInvalidSignature { inner_err: err })?;
+            .map_err(|err| Error::ReplicaPrepareInvalidSignature(err))?;
 
         // ----------- Checking the contents of the message --------------
 
         // Verify the high QC.
         message
             .high_qc
             .verify(&consensus.validator_set, consensus.threshold())
-            .map_err(|err| ReplicaMessageError::PrepareInvalidHighQC { inner_err: err })?;
+            .map_err(|err| Error::ReplicaPrepareInvalidHighQC(err))?;
 
         // 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(ReplicaMessageError::PrepareHighQCOfFutureView {
+            return Err(Error::ReplicaPrepareHighQCOfFutureView {
                 high_qc_view: message.high_qc.message.view,
                 current_view: message.view,
             });
@@ -82,7 +82,7 @@ impl StateMachine {
         let num_messages = self.prepare_message_cache.get(&message.view).unwrap().len();
 
         if num_messages < consensus.threshold() {
-            return Err(ReplicaMessageError::PrepareNumReceivedBelowThreshold {
+            return Err(Error::ReplicaPrepareNumReceivedBelowThreshold {
                 num_messages,
                 threshold: consensus.threshold(),
             });

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

@@ -1,4 +1,4 @@
-use crate::{leader::error::ReplicaMessageError, testonly};
+use crate::{leader::error::Error, testonly};
 use concurrency::ctx;
 use rand::{rngs::StdRng, Rng, SeedableRng};
 use roles::validator;
@@ -36,6 +36,6 @@ async fn replica_commit() {
             &consensus.inner,
             test_replica_msg.cast().unwrap()
         ),
-        Err(ReplicaMessageError::CommitMissingProposal)
+        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(|err| Error::LeaderCommitInvalidSignature(err))?;
 
         // ----------- 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(|err| Error::LeaderCommitInvalidJustification(err))?;
 
         // ----------- All checks finished. Now we process the message. --------------