chore: Improve error definitions (#15)

# What ❔

- Improve formatting of errors defined using `thiserror::Error` macro.
- Use strongly typed errors for block validation.

## Why ❔

Improves code style; makes it possible to test errors without relying on
their messages.

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

@@ -5,27 +5,41 @@ use network::io::{ConsensusInputMessage, Target};
 use roles::validator;
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-#[allow(clippy::missing_docs_in_private_items)]
+/// Errors that can occur when processing a "replica commit" message.
+#[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Unexpected proposal.
     #[error("unexpected proposal")]
     UnexpectedProposal,
+    /// Past view or phase.
     #[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
     Old {
+        /// Current view.
         current_view: validator::ViewNumber,
+        /// Current phase.
         current_phase: validator::Phase,
     },
+    /// The processing node is not a lead for this message's view.
     #[error("we are not a leader for this message's view")]
     NotLeaderInView,
+    /// Duplicate message from a replica.
     #[error("duplicate message from a replica (existing message: {existing_message:?}")]
     DuplicateMessage {
+        /// Existing message from the same replica.
         existing_message: validator::ReplicaCommit,
     },
-    #[error("number of received messages is below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
+    /// Number of received messages is below threshold.
+    #[error(
+        "number of received messages is below threshold. waiting for more (received: {num_messages:?}, \
+         threshold: {threshold:?}"
+    )]
     NumReceivedBelowThreshold {
+        /// Number of received messages.
         num_messages: usize,
+        /// Threshold for message count.
         threshold: usize,
     },
+    /// Invalid message signature.
     #[error("invalid signature: {0:#}")]
     InvalidSignature(#[source] crypto::bls12_381::Error),
 }

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

@@ -7,34 +7,51 @@ use roles::validator;
 use std::collections::HashMap;
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-#[allow(clippy::missing_docs_in_private_items)]
+/// Errors that can occur when processing a "replica prepare" message.
+#[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
+    /// Past view or phase.
     #[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
     Old {
+        /// Current view.
         current_view: validator::ViewNumber,
+        /// Current phase.
         current_phase: validator::Phase,
     },
+    /// The node is not a leader for this message's view.
     #[error("we are not a leader for this message's view")]
     NotLeaderInView,
+    /// Duplicate message from a replica.
     #[error("duplicate message from a replica (existing message: {existing_message:?}")]
     Exists {
+        /// Existing message from the same replica.
         existing_message: validator::ReplicaPrepare,
     },
-    #[error("number of received messages below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
+    /// Number of received messages is below a threshold.
+    #[error(
+        "number of received messages below threshold. waiting for more (received: {num_messages:?}, \
+         threshold: {threshold:?}"
+    )]
     NumReceivedBelowThreshold {
+        /// Number of received messages.
         num_messages: usize,
+        /// Threshold for message count.
         threshold: usize,
     },
+    /// High QC of a future view.
     #[error(
         "high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}"
     )]
     HighQCOfFutureView {
+        /// Received high QC view.
         high_qc_view: validator::ViewNumber,
+        /// Current view.
         current_view: validator::ViewNumber,
     },
+    /// Invalid message signature.
     #[error("invalid signature: {0:#}")]
     InvalidSignature(#[source] crypto::bls12_381::Error),
+    /// Invalid `HighQC` message.
     #[error("invalid high QC: {0:#}")]
     InvalidHighQC(#[source] anyhow::Error),
 }

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

@@ -5,23 +5,34 @@ use concurrency::ctx;
 use roles::validator;
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-#[allow(clippy::missing_docs_in_private_items)]
+/// Errors that can occur when processing a "leader commit" message.
+#[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
-    #[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})]")]
+    /// Invalid leader.
+    #[error(
+        "invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
+    )]
     InvalidLeader {
+        /// Correct leader.
         correct_leader: validator::PublicKey,
+        /// Received leader.
         received_leader: validator::PublicKey,
     },
+    /// Past view of phase.
     #[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
     Old {
+        /// Current view.
         current_view: validator::ViewNumber,
+        /// Current phase.
         current_phase: validator::Phase,
     },
+    /// Invalid message signature.
     #[error("invalid signature: {0:#}")]
     InvalidSignature(#[source] crypto::bls12_381::Error),
+    /// Invalid justification for the message.
     #[error("invalid justification: {0:#}")]
     InvalidJustification(#[source] anyhow::Error),
+    /// Internal error. Unlike other error types, this one isn't supposed to be easily recoverable.
     #[error("internal error: {0:#}")]
     Internal(#[from] anyhow::Error),
 }

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

@@ -7,61 +7,100 @@ use roles::validator;
 use std::collections::HashMap;
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-#[allow(clippy::missing_docs_in_private_items)]
+/// Errors that can occur when processing a "leader prepare" message.
+#[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
-    #[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?}])")]
+    /// Invalid leader.
+    #[error(
+        "invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})"
+    )]
     InvalidLeader {
+        /// Correct leader.
         correct_leader: validator::PublicKey,
+        /// Received leader.
         received_leader: validator::PublicKey,
     },
-    #[error("message for a past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
+    /// Message for a past view or phase.
+    #[error(
+        "message for a past view / phase (current view: {current_view:?}, current phase: {current_phase:?})"
+    )]
     Old {
+        /// Current view.
         current_view: validator::ViewNumber,
+        /// Current phase.
         current_phase: validator::Phase,
     },
+    /// Invalid message signature.
     #[error("invalid signature: {0:#}")]
     InvalidSignature(#[source] crypto::bls12_381::Error),
+    /// Invalid `PrepareQC` message.
     #[error("invalid PrepareQC: {0:#}")]
     InvalidPrepareQC(#[source] anyhow::Error),
+    /// Invalid `HighQC` message.
     #[error("invalid high QC: {0:#}")]
     InvalidHighQC(#[source] anyhow::Error),
+    /// High QC of a future view.
     #[error(
         "high QC of a future view (high QC view: {high_qc_view:?}, current view: {current_view:?}"
     )]
     HighQCOfFutureView {
+        /// Received high QC view.
         high_qc_view: validator::ViewNumber,
+        /// Current view.
         current_view: validator::ViewNumber,
     },
+    /// Previous proposal was not finalized.
     #[error("new block proposal when the previous proposal was not finalized")]
     ProposalWhenPreviousNotFinalized,
-    #[error("block proposal with invalid parent hash (correct parent hash: {correct_parent_hash:#?}, received parent hash: {received_parent_hash:#?}, block: {header:?})")]
+    /// Invalid parent hash.
+    #[error(
+        "block proposal with invalid parent hash (correct parent hash: {correct_parent_hash:#?}, \
+         received parent hash: {received_parent_hash:#?}, block: {header:?})"
+    )]
     ProposalInvalidParentHash {
+        /// Correct parent hash.
         correct_parent_hash: validator::BlockHeaderHash,
+        /// Received parent hash.
         received_parent_hash: validator::BlockHeaderHash,
+        /// Header including the incorrect parent hash.
         header: validator::BlockHeader,
     },
-    #[error("block proposal with non-sequential number (correct proposal number: {correct_number}, received proposal number: {received_number}, block: {header:?})")]
+    /// Non-sequential proposal number.
+    #[error(
+        "block proposal with non-sequential number (correct proposal number: {correct_number}, \
+         received proposal number: {received_number}, block: {header:?})"
+    )]
     ProposalNonSequentialNumber {
+        /// Correct proposal number.
         correct_number: validator::BlockNumber,
+        /// Received proposal number.
         received_number: validator::BlockNumber,
+        /// Header including the incorrect proposal number.
         header: validator::BlockHeader,
     },
+    /// Mismatched payload.
     #[error("block proposal with mismatched payload")]
     ProposalMismatchedPayload,
+    /// Oversized payload.
     #[error(
         "block proposal with an oversized payload (payload size: {payload_size}, block: {header:?}"
     )]
     ProposalOversizedPayload {
+        /// Size of the payload.
         payload_size: usize,
+        /// Proposal header corresponding to the payload.
         header: validator::BlockHeader,
     },
+    /// Re-proposal without quorum.
     #[error("block re-proposal without quorum for the re-proposal")]
     ReproposalWithoutQuorum,
+    /// Re-proposal when the previous proposal was finalized.
     #[error("block re-proposal when the previous proposal was finalized")]
     ReproposalWhenFinalized,
+    /// Re-proposal of invalid block.
     #[error("block re-proposal of invalid block")]
     ReproposalInvalidBlock,
+    /// Internal error. Unlike other error types, this one isn't supposed to be easily recoverable.
     #[error("internal error: {0:#}")]
     Internal(#[from] anyhow::Error),
 }

node/actors/network/src/consensus/handshake/mod.rs

@@ -35,8 +35,8 @@ impl ProtoFmt for Handshake {
     }
 }
 
-/// Error returned by AuthStream::handshake.
-#[derive(thiserror::Error, Debug)]
+/// Error returned by handshake logic.
+#[derive(Debug, thiserror::Error)]
 pub(super) enum Error {
     #[error("session id mismatch")]
     SessionIdMismatch,

node/actors/network/src/gossip/handshake/mod.rs

@@ -41,15 +41,15 @@ impl ProtoFmt for Handshake {
     }
 }
 
-/// Error returned by AuthStream::handshake.
-#[derive(thiserror::Error, Debug)]
+/// Error returned by gossip handshake logic.
+#[derive(Debug, thiserror::Error)]
 pub(super) enum Error {
     #[error("session id mismatch")]
     SessionIdMismatch,
     #[error("unexpected peer")]
     PeerMismatch,
     #[error("validator signature")]
-    Signature(#[from] node::ErrInvalidSignature),
+    Signature(#[from] node::InvalidSignatureError),
     #[error("stream")]
     Stream(#[source] anyhow::Error),
 }

node/actors/network/src/io.rs

@@ -1,7 +1,6 @@
 #![allow(missing_docs)]
 use concurrency::oneshot;
 use roles::{node, validator};
-use thiserror::Error;
 
 /// All the messages that other actors can send to the Network actor.
 #[derive(Debug)]
@@ -84,7 +83,7 @@ impl SyncState {
 /// Errors returned from a [`GetBlockResponse`].
 ///
 /// Note that these errors don't include network-level errors, only app-level ones.
-#[derive(Debug, Error)]
+#[derive(Debug, thiserror::Error)]
 pub enum GetBlockError {
     /// Transient error: the node doesn't have the requested L2 block and plans to get it in the future
     /// by syncing.

node/actors/network/src/mux/mod.rs

@@ -143,7 +143,7 @@ impl Mux {
 }
 
 /// Error returned by `Mux::run()`.
-#[derive(thiserror::Error, Debug)]
+#[derive(Debug, thiserror::Error)]
 #[allow(clippy::missing_docs_in_private_items)]
 pub(crate) enum RunError {
     #[error("config: {0:#}")]

node/actors/network/src/mux/transient_stream.rs

@@ -20,7 +20,7 @@ pub(crate) struct ReadStream(pub(super) sync::ExclusiveLock<ReadReusableStream>)
 pub(crate) struct WriteStream(pub(super) sync::ExclusiveLock<WriteReusableStream>);
 
 /// Error returned by `ReadStream::read` in case the stream has been closed by peer.
-#[derive(thiserror::Error, Debug)]
+#[derive(Debug, thiserror::Error)]
 #[error("end of stream")]
 pub(crate) struct EndOfStream;
 

node/actors/sync_blocks/Cargo.toml

@@ -8,6 +8,7 @@ license.workspace = true
 
 [dependencies]
 anyhow.workspace = true
+thiserror.workspace = true
 tracing.workspace = true
 
 concurrency = { path = "../../libs/concurrency" }