diff --git a/node/Cranky.toml b/node/Cranky.toml index ffe11cbe..fdc9a236 100644 --- a/node/Cranky.toml +++ b/node/Cranky.toml @@ -41,4 +41,5 @@ warn = [ allow = [ # Produces too many false positives. "clippy::redundant_locals", + "clippy::needless_pass_by_ref_mut", ] diff --git a/node/actors/consensus/src/leader/replica_prepare.rs b/node/actors/consensus/src/leader/replica_prepare.rs index 4a169459..eefd2c10 100644 --- a/node/actors/consensus/src/leader/replica_prepare.rs +++ b/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 = count diff --git a/node/actors/consensus/src/leader/state_machine.rs b/node/actors/consensus/src/leader/state_machine.rs index e47f3f28..03cd6409 100644 --- a/node/actors/consensus/src/leader/state_machine.rs +++ b/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, ) { 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); } } diff --git a/node/actors/consensus/src/replica/state_machine.rs b/node/actors/consensus/src/replica/state_machine.rs index 21cae3f2..89df7b6d 100644 --- a/node/actors/consensus/src/replica/state_machine.rs +++ b/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(()), diff --git a/node/libs/roles/src/validator/conv.rs b/node/libs/roles/src/validator/conv.rs index d3c6e803..d9be555b 100644 --- a/node/libs/roles/src/validator/conv.rs +++ b/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 _; diff --git a/node/libs/roles/src/validator/messages/block.rs b/node/libs/roles/src/validator/messages/block.rs index 80e6736d..42c267a1 100644 --- a/node/libs/roles/src/validator/messages/block.rs +++ b/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); +/// 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, diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index caeba7d7..dfbc4862 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/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, /// The PrepareQC that justifies this proposal from the leader. pub justification: PrepareQC, diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index a9c16ceb..a0dd8efa 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/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(rng: &mut R, header: &BlockHeader) -> CommitQC { CommitQC { message: ReplicaCommit { @@ -19,6 +27,8 @@ pub fn make_justification(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(rng: &mut R) -> FinalBlock { let payload: Payload = rng.gen(); let header = BlockHeader::genesis(payload.hash()); @@ -30,6 +40,8 @@ pub fn make_genesis_block(rng: &mut R) -> FinalBlock { } } +/// Constructs a random block with a given parent. +/// WARNING: this is not a fully correct FinalBlock. pub fn make_block(rng: &mut R, parent: &BlockHeader) -> FinalBlock { let payload: Payload = rng.gen(); let header = BlockHeader::new(parent, payload.hash()); diff --git a/node/libs/storage/src/types.rs b/node/libs/storage/src/types.rs index 19bc0ece..9afdaca6 100644 --- a/node/libs/storage/src/types.rs +++ b/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, }