lint

node/Cranky.toml

@@ -41,4 +41,5 @@ warn = [
 allow = [
   # Produces too many false positives.
   "clippy::redundant_locals",
+  "clippy::needless_pass_by_ref_mut",
 ]

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

@@ -133,9 +133,7 @@ impl StateMachine {
         let mut count: HashMap<_, usize> = HashMap::new();
 
         for vote in replica_messages.iter() {
-            *count
-                .entry(vote.msg.high_vote.proposal)
-                .or_default() += 1;
+            *count.entry(vote.msg.high_vote.proposal).or_default() += 1;
         }
 
         let highest_vote: Option<validator::BlockHeader> = count

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

@@ -7,14 +7,6 @@ use std::{
 };
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-pub(crate) enum Error {
-    #[error("ReplicaPrepare: {0}")]
-    ReplicaPrepare(#[from] super::replica_prepare::Error),
-    #[error("ReplicaCommit: {0}")]
-    ReplicaCommit(#[from] super::replica_commit::Error),
-}
-
 /// The StateMachine struct contains the state of the leader. This is a simple state machine. We just store
 /// replica messages and produce leader messages (including proposing blocks) when we reach the threshold for
 /// those messages. When participating in consensus we are not the leader most of the time.
@@ -67,20 +59,30 @@ impl StateMachine {
         input: validator::Signed<validator::ConsensusMsg>,
     ) {
         let now = ctx.now();
-        let (label, result) = match &input.msg {
-            validator::ConsensusMsg::ReplicaPrepare(_) => (
-                metrics::ConsensusMsgLabel::ReplicaPrepare,
-                self.process_replica_prepare(ctx, consensus, input.cast().unwrap())
-                    .map_err(Error::ReplicaPrepare),
-            ),
-            validator::ConsensusMsg::ReplicaCommit(_) => (
-                metrics::ConsensusMsgLabel::ReplicaCommit,
-                self.process_replica_commit(ctx, consensus, input.cast().unwrap())
-                    .map_err(Error::ReplicaCommit),
-            ),
+        let label = match &input.msg {
+            validator::ConsensusMsg::ReplicaPrepare(_) => {
+                let res = match self.process_replica_prepare(ctx, consensus, input.cast().unwrap())
+                {
+                    Err(err) => {
+                        tracing::warn!("{err:#}");
+                        Err(())
+                    }
+                    Ok(()) => Ok(()),
+                };
+                metrics::ConsensusMsgLabel::ReplicaPrepare.with_result(&res)
+            }
+            validator::ConsensusMsg::ReplicaCommit(_) => {
+                let res = match self.process_replica_commit(ctx, consensus, input.cast().unwrap()) {
+                    Err(err) => {
+                        tracing::warn!("{err:#}");
+                        Err(())
+                    }
+                    Ok(()) => Ok(()),
+                };
+                metrics::ConsensusMsgLabel::ReplicaCommit.with_result(&res)
+            }
             _ => unreachable!(),
         };
-        metrics::METRICS.leader_processing_latency[&label.with_result(&result)]
-            .observe_latency(ctx.now() - now);
+        metrics::METRICS.leader_processing_latency[&label].observe_latency(ctx.now() - now);
     }
 }

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

@@ -9,14 +9,6 @@ use std::{
 use storage::{ReplicaStateStore, StorageError};
 use tracing::instrument;
 
-#[derive(thiserror::Error, Debug)]
-pub(crate) enum Error {
-    #[error("LeaderPrepare: {0}")]
-    LeaderPrepare(#[from] super::leader_prepare::Error),
-    #[error("LeaderCommit: {0}")]
-    LeaderCommit(#[from] super::leader_commit::Error),
-}
-
 /// The StateMachine struct contains the state of the replica. This is the most complex state machine and is responsible
 /// for validating and voting on blocks. When participating in consensus we are always a replica.
 #[derive(Debug)]
@@ -123,7 +115,7 @@ impl StateMachine {
                             return Err(err).context("process_leader_prepare()")
                         }
                         Err(err) => {
-                            tracing::warn!("{err}");
+                            tracing::warn!("{err:#}");
                             Err(())
                         }
                         Ok(()) => Ok(()),
@@ -137,7 +129,7 @@ impl StateMachine {
                             return Err(err).context("process_leader_commit()")
                         }
                         Err(err) => {
-                            tracing::warn!("{err}");
+                            tracing::warn!("{err:#}");
                             Err(())
                         }
                         Ok(()) => Ok(()),

node/libs/roles/src/validator/conv.rs

@@ -1,4 +1,9 @@
-use super::{AggregateSignature, BlockHeader, BlockHeaderHash, BlockNumber, CommitQC, ConsensusMsg, FinalBlock, LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, Signature, Signed, Signers, ViewNumber};
+use super::{
+    AggregateSignature, BlockHeader, BlockHeaderHash, BlockNumber, CommitQC, ConsensusMsg,
+    FinalBlock, LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase,
+    PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, Signature, Signed,
+    Signers, ViewNumber,
+};
 use crate::node::SessionId;
 use ::schema::{read_required, required, ProtoFmt};
 use anyhow::Context as _;

node/libs/roles/src/validator/messages/block.rs

@@ -4,14 +4,31 @@ use super::CommitQC;
 use crypto::{sha256, ByteFmt, Text, TextFmt};
 use std::fmt;
 
+/// Version of the consensus algorithm used by the leader in a given view.
+/// Currently it is stored in the BlockHeader (TODO: consider whether it should be present in every
+/// consensus message).
+/// Currently it also determines the format and semantics of the block payload
+/// (TODO: consider whether the payload should be versioned separately).
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ProtocolVersion(pub(crate) u32);
 
+/// We use a hardcoded protocol version for now.
+/// Eventually validators should determine which version to use for which block by observing
+/// a relevant L1 contract (controlled by governance).
+///
+/// The validator binary has to support the current and next protocol version whenever
+/// a protocol version update is needed (so that it can dynamically switch from producing
+/// blocks for version X to version X+1).
 pub const CURRENT_VERSION: ProtocolVersion = ProtocolVersion(0);
 
+/// Payload of the block. Consensus algorithm does not interpret the payload
+/// (except for imposing a size limit for the payload). Proposing a payload
+/// for a new block and interpreting the payload of the finalized blocks
+/// should be implemented for the specific application of the consensus algorithm.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Payload(pub Vec<u8>);
 
+/// Hash of the Payload.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct PayloadHash(pub(crate) sha256::Sha256);
 
@@ -31,6 +48,7 @@ impl fmt::Debug for PayloadHash {
 }
 
 impl Payload {
+    /// Hash of the payload.
     pub fn hash(&self) -> PayloadHash {
         PayloadHash(sha256::Sha256::new(&self.0))
     }
@@ -111,6 +129,7 @@ impl BlockHeader {
         }
     }
 
+    /// Creates a child block for the given parent.
     pub fn new(parent: &BlockHeader, payload: PayloadHash) -> Self {
         Self {
             protocol_version: CURRENT_VERSION,
@@ -124,7 +143,9 @@ impl BlockHeader {
 /// A block that has been finalized by the consensus protocol.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct FinalBlock {
+    /// Header of the block.
     pub header: BlockHeader,
+    /// Payload of the block. Should match `header.payload` hash.
     pub payload: Payload,
     /// Justification for the block. What guarantees that the block is final.
     pub justification: CommitQC,

node/libs/roles/src/validator/messages/consensus.rs

@@ -100,8 +100,12 @@ pub struct ReplicaCommit {
 /// A Prepare message from a leader.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct LeaderPrepare {
+    /// The number of the current view.
     pub view: ViewNumber,
+    /// The header of the block that the leader is proposing.
     pub proposal: BlockHeader,
+    /// Payload of the block that the leader is proposing.
+    /// `None` iff this is a reproposal.
     pub proposal_payload: Option<Payload>,
     /// The PrepareQC that justifies this proposal from the leader.
     pub justification: PrepareQC,

node/libs/roles/src/validator/testonly.rs

@@ -1,4 +1,10 @@
-use super::{AggregateSignature, BlockHeader, BlockHeaderHash, BlockNumber, CommitQC, ConsensusMsg, FinalBlock, LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, SecretKey, Signature, Signed, Signers, ValidatorSet, ViewNumber};
+//! Test-only utilities.
+use super::{
+    AggregateSignature, BlockHeader, BlockHeaderHash, BlockNumber, CommitQC, ConsensusMsg,
+    FinalBlock, LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase,
+    PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, SecretKey, Signature,
+    Signed, Signers, ValidatorSet, ViewNumber,
+};
 use bit_vec::BitVec;
 use concurrency::time;
 use rand::{
@@ -8,6 +14,8 @@ use rand::{
 use std::sync::Arc;
 use utils::enum_util::Variant;
 
+/// Constructs a CommitQC with `CommitQC.message.proposal` matching header.
+/// WARNING: it is not a fully correct CommitQC.
 pub fn make_justification<R: Rng>(rng: &mut R, header: &BlockHeader) -> CommitQC {
     CommitQC {
         message: ReplicaCommit {
@@ -19,6 +27,8 @@ pub fn make_justification<R: Rng>(rng: &mut R, header: &BlockHeader) -> CommitQC
     }
 }
 
+/// Constructs a genesis block with random payload.
+/// WARNING: it is not a fully correct FinalBlock.
 pub fn make_genesis_block<R: Rng>(rng: &mut R) -> FinalBlock {
     let payload: Payload = rng.gen();
     let header = BlockHeader::genesis(payload.hash());
@@ -30,6 +40,8 @@ pub fn make_genesis_block<R: Rng>(rng: &mut R) -> FinalBlock {
     }
 }
 
+/// Constructs a random block with a given parent.
+/// WARNING: this is not a fully correct FinalBlock.
 pub fn make_block<R: Rng>(rng: &mut R, parent: &BlockHeader) -> FinalBlock {
     let payload: Payload = rng.gen();
     let header = BlockHeader::new(parent, payload.hash());

node/libs/storage/src/types.rs

@@ -53,9 +53,17 @@ impl DatabaseKey {
     }
 }
 
+/// A payload of a proposed block which is not known to be finalized yet.
+/// Replicas have to persist such proposed payloads for liveness:
+/// consensus may finalize a block without knowing a payload in case of reproposals.
+/// TODO(gprusak): we do not store the BlockHeader, because it is always
+///   available in the LeaderPrepare message. However for better
+///   visibility we might want to store it anyway.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Proposal {
+    /// Number of a block for which this payload has been proposed.
     pub number: BlockNumber,
+    /// Proposed payload.
     pub payload: validator::Payload,
 }