From b552993d2f2161d10ea36515b8bbc70639531b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Tue, 15 Oct 2024 02:35:21 +0100 Subject: [PATCH 01/10] First pass on the types. --- node/Cargo.lock | 24 +- node/libs/roles/src/validator/conv.rs | 4 +- .../roles/src/validator/messages/consensus.rs | 16 +- .../src/validator/messages/leader_commit.rs | 150 --------- .../src/validator/messages/leader_proposal.rs | 199 ++++++++++++ node/libs/roles/src/validator/messages/mod.rs | 10 +- node/libs/roles/src/validator/messages/msg.rs | 4 +- .../src/validator/messages/replica_commit.rs | 166 +++++++++- .../src/validator/messages/replica_prepare.rs | 55 ---- .../{leader_prepare.rs => replica_timeout.rs} | 293 ++++++++---------- .../roles/src/validator/messages/tests.rs | 4 +- node/libs/roles/src/validator/testonly.rs | 8 +- 12 files changed, 515 insertions(+), 418 deletions(-) delete mode 100644 node/libs/roles/src/validator/messages/leader_commit.rs create mode 100644 node/libs/roles/src/validator/messages/leader_proposal.rs delete mode 100644 node/libs/roles/src/validator/messages/replica_prepare.rs rename node/libs/roles/src/validator/messages/{leader_prepare.rs => replica_timeout.rs} (50%) diff --git a/node/Cargo.lock b/node/Cargo.lock index 346e2af7e..0709157a8 100644 --- a/node/Cargo.lock +++ b/node/Cargo.lock @@ -3221,7 +3221,7 @@ dependencies = [ [[package]] name = "tester" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "clap", @@ -3929,7 +3929,7 @@ dependencies = [ [[package]] name = "zksync_concurrency" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "assert_matches", @@ -3947,7 +3947,7 @@ dependencies = [ [[package]] name = "zksync_consensus_bft" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "assert_matches", @@ -3971,7 +3971,7 @@ dependencies = [ [[package]] name = "zksync_consensus_crypto" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "blst", @@ -3991,7 +3991,7 @@ dependencies = [ [[package]] name = "zksync_consensus_executor" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "async-trait", @@ -4013,7 +4013,7 @@ dependencies = [ [[package]] name = "zksync_consensus_network" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "assert_matches", @@ -4051,7 +4051,7 @@ dependencies = [ [[package]] name = "zksync_consensus_roles" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "assert_matches", @@ -4072,7 +4072,7 @@ dependencies = [ [[package]] name = "zksync_consensus_storage" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "assert_matches", @@ -4094,7 +4094,7 @@ dependencies = [ [[package]] name = "zksync_consensus_tools" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "async-trait", @@ -4129,7 +4129,7 @@ dependencies = [ [[package]] name = "zksync_consensus_utils" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "rand", @@ -4139,7 +4139,7 @@ dependencies = [ [[package]] name = "zksync_protobuf" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "bit-vec", @@ -4161,7 +4161,7 @@ dependencies = [ [[package]] name = "zksync_protobuf_build" -version = "0.4.0" +version = "0.5.0" dependencies = [ "anyhow", "heck", diff --git a/node/libs/roles/src/validator/conv.rs b/node/libs/roles/src/validator/conv.rs index 25f2bca3b..a2c5dbe5c 100644 --- a/node/libs/roles/src/validator/conv.rs +++ b/node/libs/roles/src/validator/conv.rs @@ -1,7 +1,7 @@ use super::{ AggregateSignature, Block, BlockHeader, BlockNumber, ChainId, CommitQC, Committee, ConsensusMsg, FinalBlock, ForkNumber, Genesis, GenesisHash, GenesisRaw, Justification, - LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, + LeaderCommit, LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PreGenesisBlock, PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, Signature, Signed, Signers, View, ViewNumber, WeightedValidator, }; @@ -265,7 +265,7 @@ impl ProtoFmt for ReplicaCommit { } } -impl ProtoFmt for LeaderPrepare { +impl ProtoFmt for LeaderProposal { type Proto = proto::LeaderPrepare; fn read(r: &Self::Proto) -> anyhow::Result { diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index f9895f724..9ff3a56c6 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -1,5 +1,5 @@ //! Messages related to the consensus protocol. -use super::{BlockNumber, LeaderCommit, LeaderPrepare, Msg, ReplicaCommit, ReplicaPrepare}; +use super::{BlockNumber, LeaderCommit, LeaderProposal, Msg, ReplicaCommit, ReplicaPrepare}; use crate::{attester, validator}; use anyhow::Context; use bit_vec::BitVec; @@ -351,7 +351,7 @@ impl Genesis { pub enum ConsensusMsg { ReplicaPrepare(ReplicaPrepare), ReplicaCommit(ReplicaCommit), - LeaderPrepare(LeaderPrepare), + LeaderPrepare(LeaderProposal), LeaderCommit(LeaderCommit), } @@ -406,7 +406,7 @@ impl Variant for ReplicaCommit { } } -impl Variant for LeaderPrepare { +impl Variant for LeaderProposal { fn insert(self) -> Msg { ConsensusMsg::LeaderPrepare(self).insert() } @@ -431,7 +431,7 @@ impl Variant for LeaderCommit { } /// View specification. -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct View { /// Genesis of the chain this view belongs to. pub genesis: GenesisHash, @@ -445,6 +445,14 @@ impl View { anyhow::ensure!(self.genesis == genesis.hash(), "genesis mismatch"); Ok(()) } + + /// Increments the view number. + pub fn next(self) -> Self { + Self { + genesis: self.genesis, + number: ViewNumber(self.number.0 + 1), + } + } } /// Struct that represents a bit map of validators. We use it to compactly store diff --git a/node/libs/roles/src/validator/messages/leader_commit.rs b/node/libs/roles/src/validator/messages/leader_commit.rs deleted file mode 100644 index bbcedf1bb..000000000 --- a/node/libs/roles/src/validator/messages/leader_commit.rs +++ /dev/null @@ -1,150 +0,0 @@ -use super::{BlockHeader, Genesis, ReplicaCommit, ReplicaCommitVerifyError, Signed, Signers, View}; -use crate::validator; - -/// A Commit message from a leader. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct LeaderCommit { - /// The CommitQC that justifies the message from the leader. - pub justification: CommitQC, -} - -impl LeaderCommit { - /// Verifies LeaderCommit. - pub fn verify(&self, genesis: &Genesis) -> Result<(), CommitQCVerifyError> { - self.justification.verify(genesis) - } - - /// View of this message. - pub fn view(&self) -> &View { - self.justification.view() - } -} - -/// A Commit Quorum Certificate. It is an aggregate of signed replica Commit messages. -/// The Quorum Certificate is supposed to be over identical messages, so we only need one message. -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] -pub struct CommitQC { - /// The replica Commit message that the QC is for. - pub message: ReplicaCommit, - /// The validators that signed this message. - pub signers: Signers, - /// The aggregate signature of the signed replica messages. - pub signature: validator::AggregateSignature, -} - -/// Error returned by `CommitQc::verify()`. -#[derive(thiserror::Error, Debug)] -pub enum CommitQCVerifyError { - /// Invalid message. - #[error(transparent)] - InvalidMessage(#[from] ReplicaCommitVerifyError), - /// Bad signer set. - #[error("signers set doesn't match genesis")] - BadSignersSet, - /// Weight not reached. - #[error("Signers have not reached threshold weight: got {got}, want {want}")] - NotEnoughSigners { - /// Got weight. - got: u64, - /// Want weight. - want: u64, - }, - /// Bad signature. - #[error("bad signature: {0:#}")] - BadSignature(#[source] anyhow::Error), -} - -/// Error returned by `CommitQC::add()`. -#[derive(thiserror::Error, Debug)] -pub enum CommitQCAddError { - /// Inconsistent messages. - #[error("Trying to add signature for a different message")] - InconsistentMessages, - /// Signer not present in the committee. - #[error("Signer not in committee: {signer:?}")] - SignerNotInCommittee { - /// Signer of the message. - signer: Box, - }, - /// Message already present in CommitQC. - #[error("Message already signed for CommitQC")] - Exists, -} - -impl CommitQC { - /// Header of the certified block. - pub fn header(&self) -> &BlockHeader { - &self.message.proposal - } - - /// View of this QC. - pub fn view(&self) -> &View { - &self.message.view - } - - /// Create a new empty instance for a given `ReplicaCommit` message and a validator set size. - pub fn new(message: ReplicaCommit, genesis: &Genesis) -> Self { - Self { - message, - signers: Signers::new(genesis.validators.len()), - signature: validator::AggregateSignature::default(), - } - } - - /// Add a validator's signature. - /// Signature is assumed to be already verified. - pub fn add( - &mut self, - msg: &Signed, - genesis: &Genesis, - ) -> Result<(), CommitQCAddError> { - use CommitQCAddError as Error; - if self.message != msg.msg { - return Err(Error::InconsistentMessages); - }; - let Some(i) = genesis.validators.index(&msg.key) else { - return Err(Error::SignerNotInCommittee { - signer: Box::new(msg.key.clone()), - }); - }; - if self.signers.0[i] { - return Err(Error::Exists); - }; - self.signers.0.set(i, true); - self.signature.add(&msg.sig); - Ok(()) - } - - /// Verifies the signature of the CommitQC. - pub fn verify(&self, genesis: &Genesis) -> Result<(), CommitQCVerifyError> { - use CommitQCVerifyError as Error; - self.message - .verify(genesis) - .map_err(Error::InvalidMessage)?; - if self.signers.len() != genesis.validators.len() { - return Err(Error::BadSignersSet); - } - - // Verify the signers' weight is enough. - let weight = genesis.validators.weight(&self.signers); - let threshold = genesis.validators.threshold(); - if weight < threshold { - return Err(Error::NotEnoughSigners { - got: weight, - want: threshold, - }); - } - - // Now we can verify the signature. - let messages_and_keys = genesis - .validators - .keys() - .enumerate() - .filter(|(i, _)| self.signers.0[*i]) - .map(|(_, pk)| (self.message.clone(), pk)); - - self.signature - .verify_messages(messages_and_keys) - .map_err(Error::BadSignature) - } -} diff --git a/node/libs/roles/src/validator/messages/leader_proposal.rs b/node/libs/roles/src/validator/messages/leader_proposal.rs new file mode 100644 index 000000000..236433de9 --- /dev/null +++ b/node/libs/roles/src/validator/messages/leader_proposal.rs @@ -0,0 +1,199 @@ +use super::{ + BlockHeader, BlockNumber, CommitQC, CommitQCVerifyError, Genesis, Payload, PayloadHash, + TimeoutQC, TimeoutQCVerifyError, View, +}; + +/// A Proposal message from the leader. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct LeaderProposal { + /// 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, + // What attests to the validity of this proposal. + pub justification: Justification, +} + +impl LeaderProposal { + /// View of the message. + pub fn view(&self) -> View { + self.justification.view() + } + + /// Verifies LeaderProposal. + pub fn verify(&self, genesis: &Genesis) -> Result<(), LeaderProposalVerifyError> { + // Check that the justification is valid. + self.justification + .verify(genesis) + .map_err(LeaderProposalVerifyError::Justification)?; + + // Get the implied block number and payload hash and check it against the proposal. + let (implied_block_number, implied_payload) = self.justification.get_implied_block(genesis); + + if self.proposal.number != implied_block_number { + return Err(LeaderProposalVerifyError::BadBlockNumber { + got: self.proposal.number, + want: implied_block_number, + }); + } + + if let Some(payload_hash) = implied_payload { + if self.proposal.payload != payload_hash { + return Err(LeaderProposalVerifyError::BadPayloadHash { + got: self.proposal.payload, + want: payload_hash, + }); + } + } + + // Check if we are correctly proposing a new block or re-proposing an old one. + if implied_payload.is_none() && self.proposal_payload.is_none() { + return Err(LeaderProposalVerifyError::ReproposalWhenPreviousFinalized); + } + + if implied_payload.is_some() && self.proposal_payload.is_some() { + return Err(LeaderProposalVerifyError::NewProposalWhenPreviousNotFinalized); + } + + // Check that the payload matches the header. + if let Some(payload) = &self.proposal_payload { + if payload.hash() != self.proposal.payload { + return Err(LeaderProposalVerifyError::MismatchedPayload { + header: self.proposal.payload, + payload: payload.hash(), + }); + } + } + + Ok(()) + } +} + +/// Error returned by `LeaderProposal::verify()`. +#[derive(thiserror::Error, Debug)] +pub enum LeaderProposalVerifyError { + /// Invalid Justification. + #[error("justification: {0:#}")] + Justification(JustificationVerifyError), + /// Bad block number. + #[error("bad block number: got {got:?}, want {want:?}")] + BadBlockNumber { + /// Received proposal number. + got: BlockNumber, + /// Correct proposal number. + want: BlockNumber, + }, + /// Bad payload hash on reproposal. + #[error("bad payload hash on reproposal: got {got:?}, want {want:?}")] + BadPayloadHash { + /// Received payload hash. + got: PayloadHash, + /// Correct payload hash. + want: PayloadHash, + }, + /// New block proposal when the previous proposal was not finalized. + #[error("new block proposal when the previous proposal was not finalized")] + NewProposalWhenPreviousNotFinalized, + /// Re-proposal when the previous proposal was finalized. + #[error("block re-proposal when the previous proposal was finalized")] + ReproposalWhenPreviousFinalized, + /// Mismatched payload. + #[error("block proposal with mismatched payload: header {header:?}, payload {payload:?}")] + MismatchedPayload { + /// Payload hash on block header. + header: PayloadHash, + /// Correct payload hash. + payload: PayloadHash, + }, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Justification { + // This proposal is being proposed after a view where we finalized a block. + // A commit QC is just a collection of commit votes (with at least + // QUORUM_WEIGHT) for the previous view. Note that the commit votes MUST + // be identical. + Commit(CommitQC), + // This proposal is being proposed after a view where we timed out. + // A timeout QC is just a collection of timeout votes (with at least + // QUORUM_WEIGHT) for the previous view. Unlike with the Commit QC, + // timeout votes don't need to be identical. + // The first proposal, for view 0, will always be a timeout. + Timeout(TimeoutQC), +} + +impl Justification { + fn view(&self) -> View { + match self { + Justification::Commit(qc) => qc.view().next(), + Justification::Timeout(qc) => qc.view.next(), + } + } + + fn verify(&self, genesis: &Genesis) -> Result<(), JustificationVerifyError> { + match self { + Justification::Commit(qc) => { + qc.verify(genesis).map_err(JustificationVerifyError::Commit) + } + Justification::Timeout(qc) => qc + .verify(genesis) + .map_err(JustificationVerifyError::Timeout), + } + } + + // This returns the BlockNumber that is implied by this justification. + // If the justification requires a block reproposal, it also returns + // the PayloadHash that must be reproposed. + fn get_implied_block(&self, genesis: &Genesis) -> (BlockNumber, Option) { + match self { + Justification::Commit(qc) => { + // The previous proposal was finalized, so we can propose a new block. + (qc.header().number.next(), None) + } + Justification::Timeout(qc) => { + // Get the high vote of the timeout QC, if it exists. We check if there are + // timeout votes with at least an added weight of SUBQUORUM_WEIGHT, + // that have a high vote field for the same block. A QC can have + // 0, 1 or 2 such blocks. + // If there's only 1 such block, then we say the QC has a high vote. + // If there are 0 or 2 such blocks, we say the QC has no high vote. + let high_vote = qc.high_vote(genesis); + + // Get the high commit QC of the timeout QC. We compare the high QC field of + // all timeout votes in the QC, and get the highest one, if it exists. + let high_qc = qc.high_qc(); + + if high_vote.is_some() + && (high_qc.is_none() + || high_vote.unwrap().number > high_qc.unwrap().header().number) + { + // There was some proposal last view that might have been finalized. + // We need to repropose it. + (high_vote.unwrap().number, Some(high_vote.unwrap().payload)) + } else { + // Either the previous proposal was finalized or we know for certain + // that it couldn't have been finalized. Either way, we can propose + // a new block. + let block_number = match high_qc { + Some(qc) => qc.header().number.next(), + None => BlockNumber(0), + }; + + (block_number, None) + } + } + } + } +} + +/// Error returned by `Justification::verify()`. +#[derive(thiserror::Error, Debug)] +pub enum JustificationVerifyError { + /// Invalid Timeout QC. + #[error("timeout qc: {0:#}")] + Timeout(TimeoutQCVerifyError), + /// Invalid Commit QC. + #[error("commit qc: {0:#}")] + Commit(CommitQCVerifyError), +} diff --git a/node/libs/roles/src/validator/messages/mod.rs b/node/libs/roles/src/validator/messages/mod.rs index 901896070..95a9f85e7 100644 --- a/node/libs/roles/src/validator/messages/mod.rs +++ b/node/libs/roles/src/validator/messages/mod.rs @@ -3,19 +3,17 @@ mod block; mod consensus; mod discovery; -mod leader_commit; -mod leader_prepare; +mod leader_proposal; mod msg; mod replica_commit; -mod replica_prepare; +mod replica_timeout; #[cfg(test)] mod tests; pub use block::*; pub use consensus::*; pub use discovery::*; -pub use leader_commit::*; -pub use leader_prepare::*; +pub use leader_proposal::*; pub use msg::*; pub use replica_commit::*; -pub use replica_prepare::*; +pub use replica_timeout::*; diff --git a/node/libs/roles/src/validator/messages/msg.rs b/node/libs/roles/src/validator/messages/msg.rs index cc5a047e3..9e870679a 100644 --- a/node/libs/roles/src/validator/messages/msg.rs +++ b/node/libs/roles/src/validator/messages/msg.rs @@ -116,9 +116,9 @@ impl + Clone> Signed { impl> Signed { /// Casts a signed message variant to sub/super variant. /// It is an equivalent of constructing/deconstructing enum values. - pub fn cast>(self) -> Result, BadVariantError> { + pub fn cast>(self) -> Result, BadVariantError> { Ok(Signed { - msg: V2::extract(self.msg.insert())?, + msg: V::extract(self.msg.insert())?, key: self.key, sig: self.sig, }) diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index aaeab7d52..6203fdbd2 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -1,15 +1,5 @@ -use super::{BlockHeader, Genesis, View}; - -/// Error returned by `ReplicaCommit::verify()`. -#[derive(thiserror::Error, Debug)] -pub enum ReplicaCommitVerifyError { - /// Invalid view. - #[error("view: {0:#}")] - View(anyhow::Error), - /// Bad block number. - #[error("block number < first block")] - BadBlockNumber, -} +use super::{BlockHeader, Genesis, Signed, Signers, View}; +use crate::validator; /// A Commit message from a replica. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -23,11 +13,157 @@ pub struct ReplicaCommit { impl ReplicaCommit { /// Verifies the message. pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaCommitVerifyError> { - use ReplicaCommitVerifyError as Error; - self.view.verify(genesis).map_err(Error::View)?; + self.view + .verify(genesis) + .map_err(ReplicaCommitVerifyError::View)?; + if self.proposal.number < genesis.first_block { - return Err(Error::BadBlockNumber); + return Err(ReplicaCommitVerifyError::BadBlockNumber); } + Ok(()) } } + +/// Error returned by `ReplicaCommit::verify()`. +#[derive(thiserror::Error, Debug)] +pub enum ReplicaCommitVerifyError { + /// Invalid view. + #[error("view: {0:#}")] + View(anyhow::Error), + /// Bad block number. + #[error("block number < first block")] + BadBlockNumber, +} + +/// A Commit Quorum Certificate. It is an aggregate of signed replica Commit messages. +/// The Quorum Certificate is supposed to be over identical messages, so we only need one message. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub struct CommitQC { + /// The replica Commit message that the QC is for. + pub message: ReplicaCommit, + /// The validators that signed this message. + pub signers: Signers, + /// The aggregate signature of the signed replica messages. + pub signature: validator::AggregateSignature, +} + +impl CommitQC { + /// Header of the certified block. + pub fn header(&self) -> &BlockHeader { + &self.message.proposal + } + + /// View of this QC. + pub fn view(&self) -> &View { + &self.message.view + } + + /// Create a new empty instance for a given `ReplicaCommit` message and a validator set size. + pub fn new(message: ReplicaCommit, genesis: &Genesis) -> Self { + Self { + message, + signers: Signers::new(genesis.validators.len()), + signature: validator::AggregateSignature::default(), + } + } + + /// Add a validator's signature. + /// Signature is assumed to be already verified. + pub fn add( + &mut self, + msg: &Signed, + genesis: &Genesis, + ) -> Result<(), CommitQCAddError> { + if self.message != msg.msg { + return Err(CommitQCAddError::InconsistentMessages); + }; + + let Some(i) = genesis.validators.index(&msg.key) else { + return Err(CommitQCAddError::SignerNotInCommittee { + signer: Box::new(msg.key.clone()), + }); + }; + + if self.signers.0[i] { + return Err(CommitQCAddError::Exists); + }; + + self.signers.0.set(i, true); + self.signature.add(&msg.sig); + + Ok(()) + } + + /// Verifies the signature of the CommitQC. + pub fn verify(&self, genesis: &Genesis) -> Result<(), CommitQCVerifyError> { + self.message + .verify(genesis) + .map_err(CommitQCVerifyError::InvalidMessage)?; + + if self.signers.len() != genesis.validators.len() { + return Err(CommitQCVerifyError::BadSignersSet); + } + + // Verify the signers' weight is enough. + let weight = genesis.validators.weight(&self.signers); + let threshold = genesis.validators.threshold(); + if weight < threshold { + return Err(CommitQCVerifyError::NotEnoughSigners { + got: weight, + want: threshold, + }); + } + + // Now we can verify the signature. + let messages_and_keys = genesis + .validators + .keys() + .enumerate() + .filter(|(i, _)| self.signers.0[*i]) + .map(|(_, pk)| (self.message.clone(), pk)); + + self.signature + .verify_messages(messages_and_keys) + .map_err(CommitQCVerifyError::BadSignature) + } +} + +/// Error returned by `CommitQc::verify()`. +#[derive(thiserror::Error, Debug)] +pub enum CommitQCVerifyError { + /// Invalid message. + #[error(transparent)] + InvalidMessage(#[from] ReplicaCommitVerifyError), + /// Bad signer set. + #[error("signers set doesn't match genesis")] + BadSignersSet, + /// Weight not reached. + #[error("Signers have not reached threshold weight: got {got}, want {want}")] + NotEnoughSigners { + /// Got weight. + got: u64, + /// Want weight. + want: u64, + }, + /// Bad signature. + #[error("bad signature: {0:#}")] + BadSignature(#[source] anyhow::Error), +} + +/// Error returned by `CommitQC::add()`. +#[derive(thiserror::Error, Debug)] +pub enum CommitQCAddError { + /// Inconsistent messages. + #[error("Trying to add signature for a different message")] + InconsistentMessages, + /// Signer not present in the committee. + #[error("Signer not in committee: {signer:?}")] + SignerNotInCommittee { + /// Signer of the message. + signer: Box, + }, + /// Message already present in CommitQC. + #[error("Message already signed for CommitQC")] + Exists, +} diff --git a/node/libs/roles/src/validator/messages/replica_prepare.rs b/node/libs/roles/src/validator/messages/replica_prepare.rs deleted file mode 100644 index 165830f45..000000000 --- a/node/libs/roles/src/validator/messages/replica_prepare.rs +++ /dev/null @@ -1,55 +0,0 @@ -use super::{ - CommitQC, CommitQCVerifyError, Genesis, ReplicaCommit, ReplicaCommitVerifyError, View, -}; - -/// A Prepare message from a replica. -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] -pub struct ReplicaPrepare { - /// View of this message. - pub view: View, - /// The highest block that the replica has committed to. - pub high_vote: Option, - /// The highest CommitQC that the replica has seen. - pub high_qc: Option, -} - -/// Error returned by `ReplicaPrepare::verify()`. -#[derive(thiserror::Error, Debug)] -pub enum ReplicaPrepareVerifyError { - /// View. - #[error("view: {0:#}")] - View(anyhow::Error), - /// FutureHighVoteView. - #[error("high vote from the future")] - HighVoteFutureView, - /// FutureHighQCView. - #[error("high qc from the future")] - HighQCFutureView, - /// HighVote. - #[error("high_vote: {0:#}")] - HighVote(ReplicaCommitVerifyError), - /// HighQC. - #[error("high_qc: {0:#}")] - HighQC(CommitQCVerifyError), -} - -impl ReplicaPrepare { - /// Verifies the message. - pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaPrepareVerifyError> { - use ReplicaPrepareVerifyError as Error; - self.view.verify(genesis).map_err(Error::View)?; - if let Some(v) = &self.high_vote { - if self.view.number <= v.view.number { - return Err(Error::HighVoteFutureView); - } - v.verify(genesis).map_err(Error::HighVote)?; - } - if let Some(qc) = &self.high_qc { - if self.view.number <= qc.view().number { - return Err(Error::HighQCFutureView); - } - qc.verify(genesis).map_err(Error::HighQC)?; - } - Ok(()) - } -} diff --git a/node/libs/roles/src/validator/messages/leader_prepare.rs b/node/libs/roles/src/validator/messages/replica_timeout.rs similarity index 50% rename from node/libs/roles/src/validator/messages/leader_prepare.rs rename to node/libs/roles/src/validator/messages/replica_timeout.rs index b5b9825f9..cf27a9b0b 100644 --- a/node/libs/roles/src/validator/messages/leader_prepare.rs +++ b/node/libs/roles/src/validator/messages/replica_timeout.rs @@ -1,69 +1,82 @@ use super::{ - BlockHeader, BlockNumber, CommitQC, Genesis, Payload, ReplicaPrepare, - ReplicaPrepareVerifyError, Signed, Signers, View, + BlockHeader, CommitQC, CommitQCVerifyError, Genesis, ReplicaCommit, ReplicaCommitVerifyError, + Signed, Signers, View, }; use crate::validator; use std::collections::{BTreeMap, HashMap}; -/// A quorum certificate of replica Prepare messages. Since not all Prepare messages are -/// identical (they have different high blocks and high QCs), we need to keep the high blocks -/// and high QCs in a map. We can still aggregate the signatures though. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct PrepareQC { - /// View of this QC. +/// A Timeout message from a replica. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub struct ReplicaTimeout { + /// View of this message. pub view: View, - /// Map from replica Prepare messages to the validators that signed them. - pub map: BTreeMap, - /// Aggregate signature of the replica Prepare messages. - pub signature: validator::AggregateSignature, + /// The highest block that the replica has committed to. + pub high_vote: Option, + /// The highest CommitQC that the replica has seen. + pub high_qc: Option, +} + +impl ReplicaTimeout { + /// Verifies the message. + pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaTimeoutVerifyError> { + self.view + .verify(genesis) + .map_err(ReplicaTimeoutVerifyError::View)?; + + if let Some(v) = &self.high_vote { + if self.view.number <= v.view.number { + return Err(ReplicaTimeoutVerifyError::HighVoteFutureView); + } + v.verify(genesis) + .map_err(ReplicaTimeoutVerifyError::HighVote)?; + } + + if let Some(qc) = &self.high_qc { + if self.view.number <= qc.view().number { + return Err(ReplicaTimeoutVerifyError::HighQCFutureView); + } + qc.verify(genesis) + .map_err(ReplicaTimeoutVerifyError::HighQC)?; + } + + Ok(()) + } } -/// Error returned by `PrepareQC::verify()`. +/// Error returned by `ReplicaTimeout::verify()`. #[derive(thiserror::Error, Debug)] -pub enum PrepareQCVerifyError { - /// Bad view. +pub enum ReplicaTimeoutVerifyError { + /// View. #[error("view: {0:#}")] View(anyhow::Error), - /// Inconsistent views. - #[error("inconsistent views of signed messages")] - InconsistentViews, - /// Invalid message. - #[error("msg[{0}]: {1:#}")] - InvalidMessage(usize, ReplicaPrepareVerifyError), - /// Bad message format. - #[error(transparent)] - BadFormat(anyhow::Error), - /// Weight not reached. - #[error("Signers have not reached threshold weight: got {got}, want {want}")] - NotEnoughSigners { - /// Got weight. - got: u64, - /// Want weight. - want: u64, - }, - /// Bad signature. - #[error("bad signature: {0:#}")] - BadSignature(#[source] anyhow::Error), + /// FutureHighVoteView. + #[error("high vote from the future")] + HighVoteFutureView, + /// FutureHighQCView. + #[error("high qc from the future")] + HighQCFutureView, + /// HighVote. + #[error("high_vote: {0:#}")] + HighVote(ReplicaCommitVerifyError), + /// HighQC. + #[error("high_qc: {0:#}")] + HighQC(CommitQCVerifyError), } -/// Error returned by `PrepareQC::add()`. -#[derive(thiserror::Error, Debug)] -pub enum PrepareQCAddError { - /// Inconsistent views. - #[error("Trying to add a message from a different view")] - InconsistentViews, - /// Signer not present in the committee. - #[error("Signer not in committee: {signer:?}")] - SignerNotInCommittee { - /// Signer of the message. - signer: Box, - }, - /// Message already present in PrepareQC. - #[error("Message already signed for PrepareQC")] - Exists, +/// A quorum certificate of replica Timeout messages. Since not all Timeout messages are +/// identical (they have different high blocks and high QCs), we need to keep the high blocks +/// and high QCs in a map. We can still aggregate the signatures though. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TimeoutQC { + /// View of this QC. + pub view: View, + /// Map from replica Timeout messages to the validators that signed them. + pub map: BTreeMap, + /// Aggregate signature of the replica Timeout messages. + pub signature: validator::AggregateSignature, } -impl PrepareQC { +impl TimeoutQC { /// Create a new empty instance for a given `ReplicaCommit` message and a validator set size. pub fn new(view: View) -> Self { Self { @@ -76,7 +89,7 @@ impl PrepareQC { /// Get the highest block voted and check if there's a quorum of votes for it. To have a quorum /// in this situation, we require 2*f+1 votes, where f is the maximum number of faulty replicas. /// Note that it is possible to have 2 quorums: vote A and vote B, each with >2f weight, in a single - /// PrepareQC (even in the unweighted case, because QC contains n-f signatures, not 4f+1). In such a + /// TimeoutQC (even in the unweighted case, because QC contains n-f signatures, not 4f+1). In such a /// situation we say that there is no high vote. pub fn high_vote(&self, genesis: &Genesis) -> Option { let mut count: HashMap<_, u64> = HashMap::new(); @@ -109,58 +122,62 @@ impl PrepareQC { // TODO: verify the message inside instead. pub fn add( &mut self, - msg: &Signed, + msg: &Signed, genesis: &Genesis, - ) -> Result<(), PrepareQCAddError> { - use PrepareQCAddError as Error; + ) -> Result<(), TimeoutQCAddError> { if msg.msg.view != self.view { - return Err(Error::InconsistentViews); + return Err(TimeoutQCAddError::InconsistentViews); } + let Some(i) = genesis.validators.index(&msg.key) else { - return Err(Error::SignerNotInCommittee { + return Err(TimeoutQCAddError::SignerNotInCommittee { signer: Box::new(msg.key.clone()), }); }; + if self.map.values().any(|s| s.0[i]) { - return Err(Error::Exists); + return Err(TimeoutQCAddError::Exists); }; + let e = self .map .entry(msg.msg.clone()) .or_insert_with(|| Signers::new(genesis.validators.len())); e.0.set(i, true); self.signature.add(&msg.sig); + Ok(()) } - /// Verifies the integrity of the PrepareQC. - pub fn verify(&self, genesis: &Genesis) -> Result<(), PrepareQCVerifyError> { - use PrepareQCVerifyError as Error; - self.view.verify(genesis).map_err(Error::View)?; + /// Verifies the integrity of the TimeoutQC. + pub fn verify(&self, genesis: &Genesis) -> Result<(), TimeoutQCVerifyError> { + self.view + .verify(genesis) + .map_err(TimeoutQCVerifyError::View)?; let mut sum = Signers::new(genesis.validators.len()); - // Check the ReplicaPrepare messages. + // Check the ReplicaTimeout messages. for (i, (msg, signers)) in self.map.iter().enumerate() { if msg.view != self.view { - return Err(Error::InconsistentViews); + return Err(TimeoutQCVerifyError::InconsistentViews); } if signers.len() != sum.len() { - return Err(Error::BadFormat(anyhow::format_err!( + return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( "msg[{i}].signers has wrong length" ))); } if signers.is_empty() { - return Err(Error::BadFormat(anyhow::format_err!( + return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( "msg[{i}] has no signers assigned" ))); } if !(&sum & signers).is_empty() { - return Err(Error::BadFormat(anyhow::format_err!( + return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( "overlapping signature sets for different messages" ))); } msg.verify(genesis) - .map_err(|err| Error::InvalidMessage(i, err))?; + .map_err(|err| TimeoutQCVerifyError::InvalidMessage(i, err))?; sum |= signers; } @@ -168,7 +185,7 @@ impl PrepareQC { let weight = genesis.validators.weight(&sum); let threshold = genesis.validators.threshold(); if weight < threshold { - return Err(Error::NotEnoughSigners { + return Err(TimeoutQCVerifyError::NotEnoughSigners { got: weight, want: threshold, }); @@ -186,10 +203,10 @@ impl PrepareQC { // TODO(gprusak): This reaggregating is suboptimal. self.signature .verify_messages(messages_and_keys) - .map_err(Error::BadSignature) + .map_err(TimeoutQCVerifyError::BadSignature) } - /// Calculates the weight of current PrepareQC signing validators + /// Calculates the weight of current TimeoutQC signing validators pub fn weight(&self, committee: &validator::Committee) -> u64 { self.map .values() @@ -198,103 +215,47 @@ impl PrepareQC { } } -/// A Prepare message from a leader. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct LeaderPrepare { - /// 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, -} - -/// Error returned by `LeaderPrepare::verify()`. +/// Error returned by `TimeoutQC::verify()`. #[derive(thiserror::Error, Debug)] -pub enum LeaderPrepareVerifyError { - /// Justification - #[error("justification: {0:#}")] - Justification(PrepareQCVerifyError), - /// Bad block number. - #[error("bad block number: got {got:?}, want {want:?}")] - BadBlockNumber { - /// Correct proposal number. - want: BlockNumber, - /// Received proposal number. - got: BlockNumber, +pub enum TimeoutQCVerifyError { + /// Bad view. + #[error("view: {0:#}")] + View(anyhow::Error), + /// Inconsistent views. + #[error("inconsistent views of signed messages")] + InconsistentViews, + /// Invalid message. + #[error("msg[{0}]: {1:#}")] + InvalidMessage(usize, ReplicaTimeoutVerifyError), + /// Bad message format. + #[error(transparent)] + BadFormat(anyhow::Error), + /// Weight not reached. + #[error("Signers have not reached threshold weight: got {got}, want {want}")] + NotEnoughSigners { + /// Got weight. + got: u64, + /// Want weight. + want: u64, }, - /// New block proposal when the previous proposal was not finalized. - #[error("new block proposal when the previous proposal was not finalized")] - ProposalWhenPreviousNotFinalized, - /// Mismatched payload. - #[error("block proposal with mismatched payload")] - ProposalMismatchedPayload, - /// 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, - /// Reproposed a bad block. - #[error("Reproposed a bad block")] - ReproposalBadBlock, + /// Bad signature. + #[error("bad signature: {0:#}")] + BadSignature(#[source] anyhow::Error), } -impl LeaderPrepare { - /// View of the message. - pub fn view(&self) -> &View { - &self.justification.view - } - - /// Verifies LeaderPrepare. - pub fn verify(&self, genesis: &Genesis) -> Result<(), LeaderPrepareVerifyError> { - use LeaderPrepareVerifyError as Error; - self.justification - .verify(genesis) - .map_err(Error::Justification)?; - let high_vote = self.justification.high_vote(genesis); - let high_qc = self.justification.high_qc(); - - // Check that the proposal is valid. - match &self.proposal_payload { - // The leader proposed a new block. - Some(payload) => { - // Check that payload matches the header - if self.proposal.payload != payload.hash() { - return Err(Error::ProposalMismatchedPayload); - } - // Check that we finalized the previous block. - if high_vote.is_some() - && high_vote.as_ref() != high_qc.map(|qc| &qc.message.proposal) - { - return Err(Error::ProposalWhenPreviousNotFinalized); - } - let want_number = match high_qc { - Some(qc) => qc.header().number.next(), - None => genesis.first_block, - }; - if self.proposal.number != want_number { - return Err(Error::BadBlockNumber { - got: self.proposal.number, - want: want_number, - }); - } - } - None => { - let Some(high_vote) = &high_vote else { - return Err(Error::ReproposalWithoutQuorum); - }; - if let Some(high_qc) = &high_qc { - if high_vote.number == high_qc.header().number { - return Err(Error::ReproposalWhenFinalized); - } - } - if high_vote != &self.proposal { - return Err(Error::ReproposalBadBlock); - } - } - } - Ok(()) - } +/// Error returned by `TimeoutQC::add()`. +#[derive(thiserror::Error, Debug)] +pub enum TimeoutQCAddError { + /// Inconsistent views. + #[error("Trying to add a message from a different view")] + InconsistentViews, + /// Signer not present in the committee. + #[error("Signer not in committee: {signer:?}")] + SignerNotInCommittee { + /// Signer of the message. + signer: Box, + }, + /// Message already present in TimeoutQC. + #[error("Message already signed for TimeoutQC")] + Exists, } diff --git a/node/libs/roles/src/validator/messages/tests.rs b/node/libs/roles/src/validator/messages/tests.rs index c4757280a..f9968c3b1 100644 --- a/node/libs/roles/src/validator/messages/tests.rs +++ b/node/libs/roles/src/validator/messages/tests.rs @@ -313,8 +313,8 @@ mod version1 { } /// Hardcoded `LeaderPrepare`. - fn leader_prepare() -> LeaderPrepare { - LeaderPrepare { + fn leader_prepare() -> LeaderProposal { + LeaderProposal { proposal: block_header(), proposal_payload: Some(payload()), justification: prepare_qc(), diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index 39c4f8cee..31f36b56d 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/node/libs/roles/src/validator/testonly.rs @@ -2,7 +2,7 @@ use super::{ AggregateSignature, Block, BlockHeader, BlockNumber, ChainId, CommitQC, Committee, ConsensusMsg, FinalBlock, ForkNumber, Genesis, GenesisHash, GenesisRaw, Justification, - LeaderCommit, LeaderPrepare, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, + LeaderCommit, LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PreGenesisBlock, PrepareQC, ProofOfPossession, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, SecretKey, Signature, Signed, Signers, View, ViewNumber, WeightedValidator, }; @@ -411,9 +411,9 @@ impl Distribution for Standard { } } -impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> LeaderPrepare { - LeaderPrepare { +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> LeaderProposal { + LeaderProposal { proposal: rng.gen(), proposal_payload: rng.gen(), justification: rng.gen(), From 29a0fb1f65d542823894f64bb2ddf27c78587268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Tue, 15 Oct 2024 15:52:46 +0100 Subject: [PATCH 02/10] Second pass on the types. --- node/actors/bft/src/leader/replica_commit.rs | 2 +- node/actors/bft/src/leader/replica_prepare.rs | 2 +- node/actors/bft/src/leader/tests.rs | 2 +- node/actors/bft/src/testonly/ut_harness.rs | 4 +- node/actors/bft/src/tests.rs | 2 +- .../roles/src/validator/messages/block.rs | 14 +- .../roles/src/validator/messages/committee.rs | 190 ++++++++ .../roles/src/validator/messages/consensus.rs | 437 ++---------------- .../roles/src/validator/messages/genesis.rs | 162 +++++++ .../src/validator/messages/leader_proposal.rs | 36 +- node/libs/roles/src/validator/messages/mod.rs | 28 ++ .../src/validator/messages/replica_commit.rs | 2 +- .../validator/messages/replica_new_view.rs | 33 ++ .../src/validator/messages/replica_timeout.rs | 2 +- node/libs/roles/src/validator/tests.rs | 4 +- 15 files changed, 491 insertions(+), 429 deletions(-) create mode 100644 node/libs/roles/src/validator/messages/committee.rs create mode 100644 node/libs/roles/src/validator/messages/genesis.rs create mode 100644 node/libs/roles/src/validator/messages/replica_new_view.rs diff --git a/node/actors/bft/src/leader/replica_commit.rs b/node/actors/bft/src/leader/replica_commit.rs index a28d82433..08b57e16e 100644 --- a/node/actors/bft/src/leader/replica_commit.rs +++ b/node/actors/bft/src/leader/replica_commit.rs @@ -116,7 +116,7 @@ impl StateMachine { .retain(|view_number, _| active_views.contains(view_number)); // Now we check if we have enough weight to continue. - if weight < self.config.genesis().validators.threshold() { + if weight < self.config.genesis().validators.quorum_threshold() { return Ok(()); }; diff --git a/node/actors/bft/src/leader/replica_prepare.rs b/node/actors/bft/src/leader/replica_prepare.rs index 060caf771..57186e82d 100644 --- a/node/actors/bft/src/leader/replica_prepare.rs +++ b/node/actors/bft/src/leader/replica_prepare.rs @@ -127,7 +127,7 @@ impl StateMachine { .retain(|view_number, _| active_views.contains(view_number)); // Now we check if we have enough weight to continue. - if weight < self.config.genesis().validators.threshold() { + if weight < self.config.genesis().validators.quorum_threshold() { return Ok(()); } diff --git a/node/actors/bft/src/leader/tests.rs b/node/actors/bft/src/leader/tests.rs index 6b03681d0..3c0683362 100644 --- a/node/actors/bft/src/leader/tests.rs +++ b/node/actors/bft/src/leader/tests.rs @@ -374,7 +374,7 @@ async fn replica_prepare_different_messages() { let mut replica_commit_result = None; // The rest of the validators until threshold sign other_replica_prepare - for i in validators / 2..util.genesis().validators.threshold() as usize { + for i in validators / 2..util.genesis().validators.quorum_threshold() as usize { replica_commit_result = util .process_replica_prepare(ctx, util.keys[i].sign_msg(other_replica_prepare.clone())) .await diff --git a/node/actors/bft/src/testonly/ut_harness.rs b/node/actors/bft/src/testonly/ut_harness.rs index 53b2acbe6..406ea4dfa 100644 --- a/node/actors/bft/src/testonly/ut_harness.rs +++ b/node/actors/bft/src/testonly/ut_harness.rs @@ -221,7 +221,7 @@ impl UTHarness { let res = self.process_replica_prepare(ctx, msg).await; match ( (i + 1) as u64 * self.genesis().validators.iter().next().unwrap().weight - < self.genesis().validators.threshold(), + < self.genesis().validators.quorum_threshold(), first_match, ) { (true, _) => assert!(res.unwrap().is_none()), @@ -256,7 +256,7 @@ impl UTHarness { .process_replica_commit(ctx, key.sign_msg(msg.clone())); match ( (i + 1) as u64 * self.genesis().validators.iter().next().unwrap().weight - < self.genesis().validators.threshold(), + < self.genesis().validators.quorum_threshold(), first_match, ) { (true, _) => res.unwrap(), diff --git a/node/actors/bft/src/tests.rs b/node/actors/bft/src/tests.rs index 13e0dd1a2..ea071661b 100644 --- a/node/actors/bft/src/tests.rs +++ b/node/actors/bft/src/tests.rs @@ -24,7 +24,7 @@ async fn run_test(behavior: Behavior, network: Network) { let mut nodes = vec![(behavior, 1u64); NODES]; // validator::threshold(NODES) will calculate required nodes to validate a message // given each node weight is 1 - let honest_nodes_amount = validator::threshold(NODES as u64) as usize; + let honest_nodes_amount = validator::quorum_threshold(NODES as u64) as usize; for n in &mut nodes[0..honest_nodes_amount] { n.0 = Behavior::Honest; } diff --git a/node/libs/roles/src/validator/messages/block.rs b/node/libs/roles/src/validator/messages/block.rs index 2fa3c7658..cf1070701 100644 --- a/node/libs/roles/src/validator/messages/block.rs +++ b/node/libs/roles/src/validator/messages/block.rs @@ -21,6 +21,13 @@ impl fmt::Debug for Payload { } } +impl Payload { + /// Hash of the payload. + pub fn hash(&self) -> PayloadHash { + PayloadHash(Keccak256::new(&self.0)) + } +} + /// Hash of the Payload. #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct PayloadHash(pub(crate) Keccak256); @@ -44,13 +51,6 @@ impl fmt::Debug for PayloadHash { } } -impl Payload { - /// Hash of the payload. - pub fn hash(&self) -> PayloadHash { - PayloadHash(Keccak256::new(&self.0)) - } -} - /// Sequential number of the block. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct BlockNumber(pub u64); diff --git a/node/libs/roles/src/validator/messages/committee.rs b/node/libs/roles/src/validator/messages/committee.rs new file mode 100644 index 000000000..d090ea9b2 --- /dev/null +++ b/node/libs/roles/src/validator/messages/committee.rs @@ -0,0 +1,190 @@ +//! Messages related to the consensus protocol. +use super::{max_faulty_weight, quorum_threshold, subquorum_threshold, Signers, ViewNumber}; +use crate::validator; +use anyhow::Context; +use num_bigint::BigUint; +use std::collections::BTreeMap; +use zksync_consensus_crypto::keccak256::Keccak256; + +/// A struct that represents a set of validators. It is used to store the current validator set. +/// We represent each validator by its validator public key. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Committee { + vec: Vec, + indexes: BTreeMap, + total_weight: u64, +} + +impl std::ops::Deref for Committee { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.vec + } +} + +impl Committee { + /// Creates a new Committee from a list of validator public keys. + pub fn new(validators: impl IntoIterator) -> anyhow::Result { + let mut map = BTreeMap::new(); + let mut total_weight: u64 = 0; + for v in validators { + anyhow::ensure!( + !map.contains_key(&v.key), + "Duplicate validator in validator Committee" + ); + anyhow::ensure!(v.weight > 0, "Validator weight has to be a positive value"); + total_weight = total_weight + .checked_add(v.weight) + .context("Sum of weights overflows in validator Committee")?; + map.insert(v.key.clone(), v); + } + anyhow::ensure!( + !map.is_empty(), + "Validator Committee must contain at least one validator" + ); + let vec: Vec<_> = map.into_values().collect(); + Ok(Self { + indexes: vec + .iter() + .enumerate() + .map(|(i, v)| (v.key.clone(), i)) + .collect(), + vec, + total_weight, + }) + } + + /// Iterates over validator keys. + pub fn keys(&self) -> impl Iterator { + self.vec.iter().map(|v| &v.key) + } + + /// Returns the number of validators. + #[allow(clippy::len_without_is_empty)] // a valid `Committee` is always non-empty by construction + pub fn len(&self) -> usize { + self.vec.len() + } + + /// Returns true if the given validator is in the validator committee. + pub fn contains(&self, validator: &validator::PublicKey) -> bool { + self.indexes.contains_key(validator) + } + + /// Get validator by its index in the committee. + pub fn get(&self, index: usize) -> Option<&WeightedValidator> { + self.vec.get(index) + } + + /// Get the index of a validator in the committee. + pub fn index(&self, validator: &validator::PublicKey) -> Option { + self.indexes.get(validator).copied() + } + + /// Computes the leader for the given view. + pub fn view_leader( + &self, + view_number: ViewNumber, + leader_selection: &LeaderSelectionMode, + ) -> validator::PublicKey { + match &leader_selection { + LeaderSelectionMode::RoundRobin => { + let index = view_number.0 as usize % self.len(); + self.get(index).unwrap().key.clone() + } + LeaderSelectionMode::Weighted => { + let eligibility = LeaderSelectionMode::leader_weighted_eligibility( + view_number.0, + self.total_weight, + ); + let mut offset = 0; + for val in &self.vec { + offset += val.weight; + if eligibility < offset { + return val.key.clone(); + } + } + unreachable!() + } + LeaderSelectionMode::Sticky(pk) => { + let index = self.index(pk).unwrap(); + self.get(index).unwrap().key.clone() + } + LeaderSelectionMode::Rota(pks) => { + let index = view_number.0 as usize % pks.len(); + let index = self.index(&pks[index]).unwrap(); + self.get(index).unwrap().key.clone() + } + } + } + + /// Signature weight threshold for this validator committee. + pub fn quorum_threshold(&self) -> u64 { + quorum_threshold(self.total_weight()) + } + + /// Signature weight threshold for this validator committee to trigger a reproposal. + pub fn subquorum_threshold(&self) -> u64 { + subquorum_threshold(self.total_weight()) + } + + /// Maximal weight of faulty replicas allowed in this validator committee. + pub fn max_faulty_weight(&self) -> u64 { + max_faulty_weight(self.total_weight()) + } + + /// Compute the sum of signers weights. + /// Panics if signers length does not match the number of validators in committee + pub fn weight(&self, signers: &Signers) -> u64 { + assert_eq!(self.vec.len(), signers.len()); + self.vec + .iter() + .enumerate() + .filter(|(i, _)| signers.0[*i]) + .map(|(_, v)| v.weight) + .sum() + } + + /// Sum of all validators' weight in the committee + pub fn total_weight(&self) -> u64 { + self.total_weight + } +} + +/// Validator representation inside a Committee. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WeightedValidator { + /// Validator key + pub key: validator::PublicKey, + /// Validator weight inside the Committee. + pub weight: Weight, +} + +/// Voting weight; +pub type Weight = u64; + +/// The mode used for selecting leader for a given view. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum LeaderSelectionMode { + /// Select in a round-robin fashion, based on validators' index within the set. + RoundRobin, + /// Select based on a sticky assignment to a specific validator. + Sticky(validator::PublicKey), + /// Select pseudo-randomly, based on validators' weights. + Weighted, + /// Select on a rotation of specific validator keys. + Rota(Vec), +} + +impl LeaderSelectionMode { + /// Calculates the pseudo-random eligibility of a leader based on the input and total weight. + pub fn leader_weighted_eligibility(input: u64, total_weight: u64) -> u64 { + let input_bytes = input.to_be_bytes(); + let hash = Keccak256::new(&input_bytes); + let hash_big = BigUint::from_bytes_be(hash.as_bytes()); + let total_weight_big = BigUint::from(total_weight); + let ret_big = hash_big % total_weight_big; + // Assumes that `ret_big` does not exceed 64 bits due to the modulo operation with a 64 bits-capped value. + ret_big.to_u64_digits()[0] + } +} diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index 9ff3a56c6..bc03648df 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -1,393 +1,54 @@ //! Messages related to the consensus protocol. -use super::{BlockNumber, LeaderCommit, LeaderProposal, Msg, ReplicaCommit, ReplicaPrepare}; -use crate::{attester, validator}; -use anyhow::Context; +use super::{ + Genesis, GenesisHash, LeaderProposal, Msg, ReplicaCommit, ReplicaNewView, ReplicaTimeout, +}; use bit_vec::BitVec; -use num_bigint::BigUint; -use std::{collections::BTreeMap, fmt, hash::Hash}; -use zksync_consensus_crypto::{keccak256::Keccak256, ByteFmt, Text, TextFmt}; +use std::{fmt, hash::Hash}; use zksync_consensus_utils::enum_util::{BadVariantError, Variant}; -/// Version of the consensus algorithm that the validator is using. -/// It allows to prevent misinterpretation of messages signed by validators -/// using different versions of the binaries. -#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct ProtocolVersion(pub u32); - -impl ProtocolVersion { - /// 0 - development version; deprecated. - /// 1 - development version - pub const CURRENT: Self = Self(1); - - /// Returns the integer corresponding to this version. - pub fn as_u32(self) -> u32 { - self.0 - } - - /// Checks protocol version compatibility. - pub fn compatible(&self, other: &ProtocolVersion) -> bool { - // Currently using comparison. - // This can be changed later to apply a minimum supported version. - self.0 == other.0 - } -} - -impl TryFrom for ProtocolVersion { - type Error = anyhow::Error; - - fn try_from(value: u32) -> Result { - // Currently, consensus doesn't define restrictions on the possible version. Unsupported - // versions are filtered out on the BFT actor level instead. - Ok(Self(value)) - } -} - -/// Number of the fork. Newer fork has higher number. -#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct ForkNumber(pub u64); - -impl ForkNumber { - /// Next fork number. - pub fn next(self) -> Self { - Self(self.0 + 1) - } -} - -/// The mode used for selecting leader for a given view. -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum LeaderSelectionMode { - /// Select in a round-robin fashion, based on validators' index within the set. - RoundRobin, - - /// Select based on a sticky assignment to a specific validator. - Sticky(validator::PublicKey), - - /// Select pseudo-randomly, based on validators' weights. - Weighted, - - /// Select on a rotation of specific validator keys. - Rota(Vec), -} - -/// Calculates the pseudo-random eligibility of a leader based on the input and total weight. -pub(crate) fn leader_weighted_eligibility(input: u64, total_weight: u64) -> u64 { - let input_bytes = input.to_be_bytes(); - let hash = Keccak256::new(&input_bytes); - let hash_big = BigUint::from_bytes_be(hash.as_bytes()); - let total_weight_big = BigUint::from(total_weight); - let ret_big = hash_big % total_weight_big; - // Assumes that `ret_big` does not exceed 64 bits due to the modulo operation with a 64 bits-capped value. - ret_big.to_u64_digits()[0] -} - -/// A struct that represents a set of validators. It is used to store the current validator set. -/// We represent each validator by its validator public key. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Committee { - vec: Vec, - indexes: BTreeMap, - total_weight: u64, -} - -impl std::ops::Deref for Committee { - type Target = Vec; - - fn deref(&self) -> &Self::Target { - &self.vec - } -} - -impl Committee { - /// Creates a new Committee from a list of validator public keys. - pub fn new(validators: impl IntoIterator) -> anyhow::Result { - let mut map = BTreeMap::new(); - let mut total_weight: u64 = 0; - for v in validators { - anyhow::ensure!( - !map.contains_key(&v.key), - "Duplicate validator in validator Committee" - ); - anyhow::ensure!(v.weight > 0, "Validator weight has to be a positive value"); - total_weight = total_weight - .checked_add(v.weight) - .context("Sum of weights overflows in validator Committee")?; - map.insert(v.key.clone(), v); - } - anyhow::ensure!( - !map.is_empty(), - "Validator Committee must contain at least one validator" - ); - let vec: Vec<_> = map.into_values().collect(); - Ok(Self { - indexes: vec - .iter() - .enumerate() - .map(|(i, v)| (v.key.clone(), i)) - .collect(), - vec, - total_weight, - }) - } - - /// Iterates over validator keys. - pub fn keys(&self) -> impl Iterator { - self.vec.iter().map(|v| &v.key) - } - - /// Returns the number of validators. - #[allow(clippy::len_without_is_empty)] // a valid `Committee` is always non-empty by construction - pub fn len(&self) -> usize { - self.vec.len() - } - - /// Returns true if the given validator is in the validator committee. - pub fn contains(&self, validator: &validator::PublicKey) -> bool { - self.indexes.contains_key(validator) - } - - /// Get validator by its index in the committee. - pub fn get(&self, index: usize) -> Option<&WeightedValidator> { - self.vec.get(index) - } - - /// Get the index of a validator in the committee. - pub fn index(&self, validator: &validator::PublicKey) -> Option { - self.indexes.get(validator).copied() - } - - /// Computes the leader for the given view. - pub fn view_leader( - &self, - view_number: ViewNumber, - leader_selection: &LeaderSelectionMode, - ) -> validator::PublicKey { - match &leader_selection { - LeaderSelectionMode::RoundRobin => { - let index = view_number.0 as usize % self.len(); - self.get(index).unwrap().key.clone() - } - LeaderSelectionMode::Weighted => { - let eligibility = leader_weighted_eligibility(view_number.0, self.total_weight); - let mut offset = 0; - for val in &self.vec { - offset += val.weight; - if eligibility < offset { - return val.key.clone(); - } - } - unreachable!() - } - LeaderSelectionMode::Sticky(pk) => { - let index = self.index(pk).unwrap(); - self.get(index).unwrap().key.clone() - } - LeaderSelectionMode::Rota(pks) => { - let index = view_number.0 as usize % pks.len(); - let index = self.index(&pks[index]).unwrap(); - self.get(index).unwrap().key.clone() - } - } - } - - /// Signature weight threshold for this validator committee. - pub fn threshold(&self) -> u64 { - threshold(self.total_weight()) - } - - /// Maximal weight of faulty replicas allowed in this validator committee. - pub fn max_faulty_weight(&self) -> u64 { - max_faulty_weight(self.total_weight()) - } - - /// Compute the sum of signers weights. - /// Panics if signers length does not match the number of validators in committee - pub fn weight(&self, signers: &Signers) -> u64 { - assert_eq!(self.vec.len(), signers.len()); - self.vec - .iter() - .enumerate() - .filter(|(i, _)| signers.0[*i]) - .map(|(_, v)| v.weight) - .sum() - } - - /// Sum of all validators' weight in the committee - pub fn total_weight(&self) -> u64 { - self.total_weight - } -} - -/// Calculate the consensus threshold, the minimum votes' weight for any consensus action to be valid, -/// for a given committee total weight. -pub fn threshold(total_weight: u64) -> u64 { - total_weight - max_faulty_weight(total_weight) -} - -/// Calculate the maximum allowed weight for faulty replicas, for a given total weight. -pub fn max_faulty_weight(total_weight: u64) -> u64 { - // Calculate the allowed maximum weight of faulty replicas. We want the following relationship to hold: - // n = 5*f + 1 - // for n total weight and f faulty weight. This results in the following formula for the maximum - // weight of faulty replicas: - // f = floor((n - 1) / 5) - (total_weight - 1) / 5 -} - -/// Ethereum CHAIN_ID -/// `https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md` -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct ChainId(pub u64); - -/// Genesis of the blockchain, unique for each blockchain instance. -#[derive(Debug, Clone, PartialEq)] -pub struct GenesisRaw { - /// ID of the blockchain. - pub chain_id: ChainId, - /// Number of the fork. Should be incremented every time the genesis is updated, - /// i.e. whenever a hard fork is performed. - pub fork_number: ForkNumber, - /// Protocol version used by this fork. - pub protocol_version: ProtocolVersion, - /// First block of a fork. - pub first_block: BlockNumber, - /// Set of validators of the chain. - pub validators: Committee, - /// Set of attesters of the chain. - pub attesters: Option, - /// The mode used for selecting leader for a given view. - pub leader_selection: LeaderSelectionMode, -} - -/// Hash of the genesis specification. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct GenesisHash(pub(crate) Keccak256); - -impl GenesisRaw { - /// Constructs Genesis with cached hash. - pub fn with_hash(self) -> Genesis { - let hash = GenesisHash(Keccak256::new(&zksync_protobuf::canonical(&self))); - Genesis(self, hash) - } -} - -impl TextFmt for GenesisHash { - fn decode(text: Text) -> anyhow::Result { - text.strip("genesis_hash:keccak256:")? - .decode_hex() - .map(Self) - } - - fn encode(&self) -> String { - format!( - "genesis_hash:keccak256:{}", - hex::encode(ByteFmt::encode(&self.0)) - ) - } -} - -impl fmt::Debug for GenesisHash { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt.write_str(&TextFmt::encode(self)) - } -} - -/// Genesis with cached hash. -#[derive(Clone)] -pub struct Genesis(GenesisRaw, GenesisHash); - -impl std::ops::Deref for Genesis { - type Target = GenesisRaw; - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl PartialEq for Genesis { - fn eq(&self, other: &Self) -> bool { - self.1 == other.1 - } -} - -impl fmt::Debug for Genesis { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - self.0.fmt(fmt) - } -} - -impl Genesis { - /// Verifies correctness. - pub fn verify(&self) -> anyhow::Result<()> { - if let LeaderSelectionMode::Sticky(pk) = &self.leader_selection { - if self.validators.index(pk).is_none() { - anyhow::bail!("leader_selection sticky mode public key is not in committee"); - } - } else if let LeaderSelectionMode::Rota(pks) = &self.leader_selection { - for pk in pks { - if self.validators.index(pk).is_none() { - anyhow::bail!( - "leader_selection rota mode public key is not in committee: {pk:?}" - ); - } - } - } - - Ok(()) - } - - /// Computes the leader for the given view. - pub fn view_leader(&self, view: ViewNumber) -> validator::PublicKey { - self.validators.view_leader(view, &self.leader_selection) - } - - /// Hash of the genesis. - pub fn hash(&self) -> GenesisHash { - self.1 - } -} - /// Consensus messages. #[allow(missing_docs)] #[derive(Clone, Debug, PartialEq, Eq)] pub enum ConsensusMsg { - ReplicaPrepare(ReplicaPrepare), + LeaderProposal(LeaderProposal), ReplicaCommit(ReplicaCommit), - LeaderPrepare(LeaderProposal), - LeaderCommit(LeaderCommit), + ReplicaNewView(ReplicaNewView), + ReplicaTimeout(ReplicaTimeout), } impl ConsensusMsg { /// ConsensusMsg variant name. pub fn label(&self) -> &'static str { match self { - Self::ReplicaPrepare(_) => "ReplicaPrepare", + Self::LeaderProposal(_) => "LeaderProposal", Self::ReplicaCommit(_) => "ReplicaCommit", - Self::LeaderPrepare(_) => "LeaderPrepare", - Self::LeaderCommit(_) => "LeaderCommit", + Self::ReplicaNewView(_) => "ReplicaNewView", + Self::ReplicaTimeout(_) => "ReplicaTimeout", } } /// View of this message. - pub fn view(&self) -> &View { + pub fn view(&self) -> View { match self { - Self::ReplicaPrepare(m) => &m.view, - Self::ReplicaCommit(m) => &m.view, - Self::LeaderPrepare(m) => m.view(), - Self::LeaderCommit(m) => m.view(), + Self::LeaderProposal(msg) => msg.view(), + Self::ReplicaCommit(msg) => msg.view, + Self::ReplicaNewView(msg) => msg.view(), + Self::ReplicaTimeout(msg) => msg.view, } } /// Hash of the genesis that defines the chain. - pub fn genesis(&self) -> &GenesisHash { - &self.view().genesis + pub fn genesis(&self) -> GenesisHash { + self.view().genesis } } -impl Variant for ReplicaPrepare { +impl Variant for LeaderProposal { fn insert(self) -> Msg { - ConsensusMsg::ReplicaPrepare(self).insert() + ConsensusMsg::LeaderProposal(self).insert() } fn extract(msg: Msg) -> Result { - let ConsensusMsg::ReplicaPrepare(this) = Variant::extract(msg)? else { + let ConsensusMsg::LeaderProposal(this) = Variant::extract(msg)? else { return Err(BadVariantError); }; Ok(this) @@ -406,24 +67,24 @@ impl Variant for ReplicaCommit { } } -impl Variant for LeaderProposal { +impl Variant for ReplicaNewView { fn insert(self) -> Msg { - ConsensusMsg::LeaderPrepare(self).insert() + ConsensusMsg::ReplicaNewView(self).insert() } fn extract(msg: Msg) -> Result { - let ConsensusMsg::LeaderPrepare(this) = Variant::extract(msg)? else { + let ConsensusMsg::ReplicaNewView(this) = Variant::extract(msg)? else { return Err(BadVariantError); }; Ok(this) } } -impl Variant for LeaderCommit { +impl Variant for ReplicaTimeout { fn insert(self) -> Msg { - ConsensusMsg::LeaderCommit(self).insert() + ConsensusMsg::ReplicaTimeout(self).insert() } fn extract(msg: Msg) -> Result { - let ConsensusMsg::LeaderCommit(this) = Variant::extract(msg)? else { + let ConsensusMsg::ReplicaTimeout(this) = Variant::extract(msg)? else { return Err(BadVariantError); }; Ok(this) @@ -455,6 +116,23 @@ impl View { } } +/// A struct that represents a view number. +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct ViewNumber(pub u64); + +impl ViewNumber { + /// Get the next view number. + pub fn next(self) -> Self { + Self(self.0 + 1) + } +} + +impl fmt::Display for ViewNumber { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, formatter) + } +} + /// Struct that represents a bit map of validators. We use it to compactly store /// which validators signed a given message. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -504,23 +182,6 @@ impl std::ops::BitAnd for &Signers { } } -/// A struct that represents a view number. -#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct ViewNumber(pub u64); - -impl ViewNumber { - /// Get the next view number. - pub fn next(self) -> Self { - Self(self.0 + 1) - } -} - -impl fmt::Display for ViewNumber { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, formatter) - } -} - /// An enum that represents the current phase of the consensus. #[allow(missing_docs)] #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -528,15 +189,3 @@ pub enum Phase { Prepare, Commit, } - -/// Validator representation inside a Committee. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct WeightedValidator { - /// Validator key - pub key: validator::PublicKey, - /// Validator weight inside the Committee. - pub weight: Weight, -} - -/// Voting weight; -pub type Weight = u64; diff --git a/node/libs/roles/src/validator/messages/genesis.rs b/node/libs/roles/src/validator/messages/genesis.rs new file mode 100644 index 000000000..57e5e1cd1 --- /dev/null +++ b/node/libs/roles/src/validator/messages/genesis.rs @@ -0,0 +1,162 @@ +//! Messages related to the consensus protocol. +use super::{BlockNumber, Committee, LeaderSelectionMode, ViewNumber}; +use crate::{attester, validator}; +use std::{fmt, hash::Hash}; +use zksync_consensus_crypto::{keccak256::Keccak256, ByteFmt, Text, TextFmt}; + +/// Genesis of the blockchain, unique for each blockchain instance. +#[derive(Debug, Clone, PartialEq)] +pub struct GenesisRaw { + /// ID of the blockchain. + pub chain_id: ChainId, + /// Number of the fork. Should be incremented every time the genesis is updated, + /// i.e. whenever a hard fork is performed. + pub fork_number: ForkNumber, + /// Protocol version used by this fork. + pub protocol_version: ProtocolVersion, + /// First block of a fork. + pub first_block: BlockNumber, + /// Set of validators of the chain. + pub validators: Committee, + /// Set of attesters of the chain. + pub attesters: Option, + /// The mode used for selecting leader for a given view. + pub leader_selection: LeaderSelectionMode, +} + +impl GenesisRaw { + /// Constructs Genesis with cached hash. + pub fn with_hash(self) -> Genesis { + let hash = GenesisHash(Keccak256::new(&zksync_protobuf::canonical(&self))); + Genesis(self, hash) + } +} + +/// Hash of the genesis specification. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct GenesisHash(pub(crate) Keccak256); + +impl TextFmt for GenesisHash { + fn decode(text: Text) -> anyhow::Result { + text.strip("genesis_hash:keccak256:")? + .decode_hex() + .map(Self) + } + + fn encode(&self) -> String { + format!( + "genesis_hash:keccak256:{}", + hex::encode(ByteFmt::encode(&self.0)) + ) + } +} + +impl fmt::Debug for GenesisHash { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.write_str(&TextFmt::encode(self)) + } +} + +/// Genesis with cached hash. +#[derive(Clone)] +pub struct Genesis(GenesisRaw, GenesisHash); + +impl std::ops::Deref for Genesis { + type Target = GenesisRaw; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl PartialEq for Genesis { + fn eq(&self, other: &Self) -> bool { + self.1 == other.1 + } +} + +impl fmt::Debug for Genesis { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(fmt) + } +} + +impl Genesis { + /// Verifies correctness. + pub fn verify(&self) -> anyhow::Result<()> { + if let LeaderSelectionMode::Sticky(pk) = &self.leader_selection { + if self.validators.index(pk).is_none() { + anyhow::bail!("leader_selection sticky mode public key is not in committee"); + } + } else if let LeaderSelectionMode::Rota(pks) = &self.leader_selection { + for pk in pks { + if self.validators.index(pk).is_none() { + anyhow::bail!( + "leader_selection rota mode public key is not in committee: {pk:?}" + ); + } + } + } + + Ok(()) + } + + /// Computes the leader for the given view. + pub fn view_leader(&self, view: ViewNumber) -> validator::PublicKey { + self.validators.view_leader(view, &self.leader_selection) + } + + /// Hash of the genesis. + pub fn hash(&self) -> GenesisHash { + self.1 + } +} + +/// Version of the consensus algorithm that the validator is using. +/// It allows to prevent misinterpretation of messages signed by validators +/// using different versions of the binaries. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct ProtocolVersion(pub u32); + +impl ProtocolVersion { + /// 0 - development version; deprecated. + /// 1 - development version + pub const CURRENT: Self = Self(1); + + /// Returns the integer corresponding to this version. + pub fn as_u32(self) -> u32 { + self.0 + } + + /// Checks protocol version compatibility. + pub fn compatible(&self, other: &ProtocolVersion) -> bool { + // Currently using comparison. + // This can be changed later to apply a minimum supported version. + self.0 == other.0 + } +} + +impl TryFrom for ProtocolVersion { + type Error = anyhow::Error; + + fn try_from(value: u32) -> Result { + // Currently, consensus doesn't define restrictions on the possible version. Unsupported + // versions are filtered out on the BFT actor level instead. + Ok(Self(value)) + } +} + +/// Number of the fork. Newer fork has higher number. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct ForkNumber(pub u64); + +impl ForkNumber { + /// Next fork number. + pub fn next(self) -> Self { + Self(self.0 + 1) + } +} + +/// Ethereum CHAIN_ID +/// `https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md` +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct ChainId(pub u64); diff --git a/node/libs/roles/src/validator/messages/leader_proposal.rs b/node/libs/roles/src/validator/messages/leader_proposal.rs index 236433de9..7fa52d7e1 100644 --- a/node/libs/roles/src/validator/messages/leader_proposal.rs +++ b/node/libs/roles/src/validator/messages/leader_proposal.rs @@ -12,7 +12,7 @@ pub struct LeaderProposal { /// `None` iff this is a reproposal. pub proposal_payload: Option, // What attests to the validity of this proposal. - pub justification: Justification, + pub justification: ProposalJustification, } impl LeaderProposal { @@ -75,7 +75,7 @@ impl LeaderProposal { pub enum LeaderProposalVerifyError { /// Invalid Justification. #[error("justification: {0:#}")] - Justification(JustificationVerifyError), + Justification(ProposalJustificationVerifyError), /// Bad block number. #[error("bad block number: got {got:?}, want {want:?}")] BadBlockNumber { @@ -109,7 +109,7 @@ pub enum LeaderProposalVerifyError { } #[derive(Clone, Debug, PartialEq, Eq)] -pub enum Justification { +pub enum ProposalJustification { // This proposal is being proposed after a view where we finalized a block. // A commit QC is just a collection of commit votes (with at least // QUORUM_WEIGHT) for the previous view. Note that the commit votes MUST @@ -123,35 +123,35 @@ pub enum Justification { Timeout(TimeoutQC), } -impl Justification { - fn view(&self) -> View { +impl ProposalJustification { + pub fn view(&self) -> View { match self { - Justification::Commit(qc) => qc.view().next(), - Justification::Timeout(qc) => qc.view.next(), + ProposalJustification::Commit(qc) => qc.view().next(), + ProposalJustification::Timeout(qc) => qc.view.next(), } } - fn verify(&self, genesis: &Genesis) -> Result<(), JustificationVerifyError> { + pub fn verify(&self, genesis: &Genesis) -> Result<(), ProposalJustificationVerifyError> { match self { - Justification::Commit(qc) => { - qc.verify(genesis).map_err(JustificationVerifyError::Commit) - } - Justification::Timeout(qc) => qc + ProposalJustification::Commit(qc) => qc + .verify(genesis) + .map_err(ProposalJustificationVerifyError::Commit), + ProposalJustification::Timeout(qc) => qc .verify(genesis) - .map_err(JustificationVerifyError::Timeout), + .map_err(ProposalJustificationVerifyError::Timeout), } } // This returns the BlockNumber that is implied by this justification. // If the justification requires a block reproposal, it also returns // the PayloadHash that must be reproposed. - fn get_implied_block(&self, genesis: &Genesis) -> (BlockNumber, Option) { + pub fn get_implied_block(&self, genesis: &Genesis) -> (BlockNumber, Option) { match self { - Justification::Commit(qc) => { + ProposalJustification::Commit(qc) => { // The previous proposal was finalized, so we can propose a new block. (qc.header().number.next(), None) } - Justification::Timeout(qc) => { + ProposalJustification::Timeout(qc) => { // Get the high vote of the timeout QC, if it exists. We check if there are // timeout votes with at least an added weight of SUBQUORUM_WEIGHT, // that have a high vote field for the same block. A QC can have @@ -187,9 +187,9 @@ impl Justification { } } -/// Error returned by `Justification::verify()`. +/// Error returned by `ProposalJustification::verify()`. #[derive(thiserror::Error, Debug)] -pub enum JustificationVerifyError { +pub enum ProposalJustificationVerifyError { /// Invalid Timeout QC. #[error("timeout qc: {0:#}")] Timeout(TimeoutQCVerifyError), diff --git a/node/libs/roles/src/validator/messages/mod.rs b/node/libs/roles/src/validator/messages/mod.rs index 95a9f85e7..797cd6251 100644 --- a/node/libs/roles/src/validator/messages/mod.rs +++ b/node/libs/roles/src/validator/messages/mod.rs @@ -1,19 +1,47 @@ //! Messages exchanged between validators. mod block; +mod committee; mod consensus; mod discovery; +mod genesis; mod leader_proposal; mod msg; mod replica_commit; +mod replica_new_view; mod replica_timeout; #[cfg(test)] mod tests; pub use block::*; +pub use committee::*; pub use consensus::*; pub use discovery::*; +pub use genesis::*; pub use leader_proposal::*; pub use msg::*; pub use replica_commit::*; +pub use replica_new_view::*; pub use replica_timeout::*; + +/// Calculate the maximum allowed weight for faulty replicas, for a given total weight. +pub fn max_faulty_weight(total_weight: u64) -> u64 { + // Calculate the allowed maximum weight of faulty replicas. We want the following relationship to hold: + // n = 5*f + 1 + // for n total weight and f faulty weight. This results in the following formula for the maximum + // weight of faulty replicas: + // f = floor((n - 1) / 5) + (total_weight - 1) / 5 +} + +/// Calculate the consensus quorum threshold, the minimum votes' weight necessary to finalize a block, +/// for a given committee total weight. +pub fn quorum_threshold(total_weight: u64) -> u64 { + total_weight - max_faulty_weight(total_weight) +} + +/// Calculate the consensus subquorum threshold, the minimum votes' weight necessary to trigger a reproposal, +/// for a given committee total weight. +pub fn subquorum_threshold(total_weight: u64) -> u64 { + total_weight - 3 * max_faulty_weight(total_weight) +} diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index 6203fdbd2..1c645959b 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -107,7 +107,7 @@ impl CommitQC { // Verify the signers' weight is enough. let weight = genesis.validators.weight(&self.signers); - let threshold = genesis.validators.threshold(); + let threshold = genesis.validators.quorum_threshold(); if weight < threshold { return Err(CommitQCVerifyError::NotEnoughSigners { got: weight, diff --git a/node/libs/roles/src/validator/messages/replica_new_view.rs b/node/libs/roles/src/validator/messages/replica_new_view.rs new file mode 100644 index 000000000..e127f02ba --- /dev/null +++ b/node/libs/roles/src/validator/messages/replica_new_view.rs @@ -0,0 +1,33 @@ +use super::{Genesis, ProposalJustification, ProposalJustificationVerifyError, View}; + +/// A NewView message from a replica. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ReplicaNewView { + /// What attests to the validity of this view change. + pub justification: ProposalJustification, +} + +impl ReplicaNewView { + /// View of the message. + pub fn view(&self) -> View { + self.justification.view() + } + + /// Verifies ReplicaNewView. + pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaNewViewVerifyError> { + // Check that the justification is valid. + self.justification + .verify(genesis) + .map_err(ReplicaNewViewVerifyError::Justification)?; + + Ok(()) + } +} + +/// Error returned by `ReplicaNewView::verify()`. +#[derive(thiserror::Error, Debug)] +pub enum ReplicaNewViewVerifyError { + /// Invalid Justification. + #[error("justification: {0:#}")] + Justification(ProposalJustificationVerifyError), +} diff --git a/node/libs/roles/src/validator/messages/replica_timeout.rs b/node/libs/roles/src/validator/messages/replica_timeout.rs index cf27a9b0b..0c81f7408 100644 --- a/node/libs/roles/src/validator/messages/replica_timeout.rs +++ b/node/libs/roles/src/validator/messages/replica_timeout.rs @@ -183,7 +183,7 @@ impl TimeoutQC { // Verify the signers' weight is enough. let weight = genesis.validators.weight(&sum); - let threshold = genesis.validators.threshold(); + let threshold = genesis.validators.quorum_threshold(); if weight < threshold { return Err(TimeoutQCVerifyError::NotEnoughSigners { got: weight, diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index eafa3a9e7..3348ae943 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -239,7 +239,7 @@ fn test_commit_qc() { .take(i) .map(|w| w.weight) .sum(); - if expected_weight >= setup1.genesis.validators.threshold() { + if expected_weight >= setup1.genesis.validators.quorum_threshold() { qc.verify(&setup1.genesis).unwrap(); } else { assert_matches!( @@ -340,7 +340,7 @@ fn test_prepare_qc() { .take(n) .map(|w| w.weight) .sum(); - if expected_weight >= setup1.genesis.validators.threshold() { + if expected_weight >= setup1.genesis.validators.quorum_threshold() { qc.verify(&setup1.genesis).unwrap(); } else { assert_matches!( From 06ddbc068740c4c0a35037f73dfb246cc867dbd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Tue, 15 Oct 2024 16:54:08 +0100 Subject: [PATCH 03/10] Proto definitions and conversion --- .../roles/src/proto/validator/messages.proto | 50 ++++--- node/libs/roles/src/validator/conv.rs | 123 +++++++++++------- .../roles/src/validator/messages/committee.rs | 24 +++- .../roles/src/validator/messages/consensus.rs | 1 + node/libs/roles/src/validator/messages/mod.rs | 22 ---- node/libs/roles/src/validator/messages/msg.rs | 4 +- 6 files changed, 133 insertions(+), 91 deletions(-) diff --git a/node/libs/roles/src/proto/validator/messages.proto b/node/libs/roles/src/proto/validator/messages.proto index 833087045..2b7038c52 100644 --- a/node/libs/roles/src/proto/validator/messages.proto +++ b/node/libs/roles/src/proto/validator/messages.proto @@ -39,57 +39,69 @@ message Block { message View { reserved 1,2; reserved "protocol_version","fork"; + optional GenesisHash genesis = 4; // required optional uint64 number = 3; // required; ViewNumber } message ConsensusMsg { - oneof t {// required - ReplicaPrepare replica_prepare = 1; + reserved 1, 3, 4; + reserved "replica_prepare", "leader_prepare", "leader_commit"; + + oneof t { // required ReplicaCommit replica_commit = 2; - LeaderPrepare leader_prepare = 3; - LeaderCommit leader_commit = 4; + ReplicaTimeout replica_timeout = 5; + ReplicaNewView replica_new_view = 6; + LeaderProposal leader_proposal = 7; } } -message ReplicaPrepare { +message ReplicaCommit { + optional View view = 1; // required + optional BlockHeader proposal = 2; // required +} + +message ReplicaTimeout { optional View view = 1; // required optional ReplicaCommit high_vote = 2; // optional optional CommitQC high_qc = 3; // optional } -message ReplicaCommit { - optional View view = 1; // required - optional BlockHeader proposal = 2; // required +message ReplicaNewView { + optional ProposalJustification justification = 1; // required } -message LeaderPrepare { +message LeaderProposal { optional BlockHeader proposal = 1; // required optional bytes proposal_payload = 2; // optional (depending on justification) - optional PrepareQC justification = 3; // required + optional ProposalJustification justification = 3; // required } -message LeaderCommit { - optional CommitQC justification = 1; // required +message CommitQC { + optional ReplicaCommit msg = 1; // required + optional std.BitVector signers = 2; // required + optional AggregateSignature sig = 3; // required } -message PrepareQC { +message TimeoutQC { optional View view = 4; // required - repeated ReplicaPrepare msgs = 1; // required + repeated ReplicaTimeout msgs = 1; // required repeated std.BitVector signers = 2; // required optional AggregateSignature sig = 3; // required } -message CommitQC { - optional ReplicaCommit msg = 1; // required - optional std.BitVector signers = 2; // required - optional AggregateSignature sig = 3; // required +message ProposalJustification { + oneof t { // required + CommitQC commit_qc = 1; + TimeoutQC timeout_qc = 2; + } } message Phase { - oneof t { + oneof t { // required std.Void prepare = 1; std.Void commit = 2; + std.Void timeout = 3; } } diff --git a/node/libs/roles/src/validator/conv.rs b/node/libs/roles/src/validator/conv.rs index a2c5dbe5c..3dade7880 100644 --- a/node/libs/roles/src/validator/conv.rs +++ b/node/libs/roles/src/validator/conv.rs @@ -1,9 +1,9 @@ use super::{ AggregateSignature, Block, BlockHeader, BlockNumber, ChainId, CommitQC, Committee, ConsensusMsg, FinalBlock, ForkNumber, Genesis, GenesisHash, GenesisRaw, Justification, - LeaderCommit, LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, - PreGenesisBlock, PrepareQC, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaPrepare, - Signature, Signed, Signers, View, ViewNumber, WeightedValidator, + LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PreGenesisBlock, + ProposalJustification, ProtocolVersion, PublicKey, ReplicaCommit, ReplicaNewView, + ReplicaTimeout, Signature, Signed, Signers, TimeoutQC, View, ViewNumber, WeightedValidator, }; use crate::{ attester::{self, WeightedAttester}, @@ -186,12 +186,16 @@ impl ProtoFmt for ConsensusMsg { fn read(r: &Self::Proto) -> anyhow::Result { use proto::consensus_msg::T; Ok(match r.t.as_ref().context("missing")? { - T::ReplicaPrepare(r) => { - Self::ReplicaPrepare(ProtoFmt::read(r).context("ReplicaPrepare")?) - } T::ReplicaCommit(r) => Self::ReplicaCommit(ProtoFmt::read(r).context("ReplicaCommit")?), - T::LeaderPrepare(r) => Self::LeaderPrepare(ProtoFmt::read(r).context("LeaderPrepare")?), - T::LeaderCommit(r) => Self::LeaderCommit(ProtoFmt::read(r).context("LeaderCommit")?), + T::ReplicaTimeout(r) => { + Self::ReplicaTimeout(ProtoFmt::read(r).context("ReplicaTimeout")?) + } + T::ReplicaNewView(r) => { + Self::ReplicaNewView(ProtoFmt::read(r).context("ReplicaNewView")?) + } + T::LeaderProposal(r) => { + Self::LeaderProposal(ProtoFmt::read(r).context("LeaderProposal")?) + } }) } @@ -199,10 +203,10 @@ impl ProtoFmt for ConsensusMsg { use proto::consensus_msg::T; let t = match self { - Self::ReplicaPrepare(x) => T::ReplicaPrepare(x.build()), Self::ReplicaCommit(x) => T::ReplicaCommit(x.build()), - Self::LeaderPrepare(x) => T::LeaderPrepare(x.build()), - Self::LeaderCommit(x) => T::LeaderCommit(x.build()), + Self::ReplicaTimeout(x) => T::ReplicaTimeout(x.build()), + Self::ReplicaNewView(x) => T::ReplicaNewView(x.build()), + Self::LeaderProposal(x) => T::LeaderProposal(x.build()), }; Self::Proto { t: Some(t) } @@ -227,101 +231,109 @@ impl ProtoFmt for View { } } -impl ProtoFmt for ReplicaPrepare { - type Proto = proto::ReplicaPrepare; +impl ProtoFmt for ReplicaCommit { + type Proto = proto::ReplicaCommit; fn read(r: &Self::Proto) -> anyhow::Result { Ok(Self { view: read_required(&r.view).context("view")?, - high_vote: read_optional(&r.high_vote).context("high_vote")?, - high_qc: read_optional(&r.high_qc).context("high_qc")?, + proposal: read_required(&r.proposal).context("proposal")?, }) } fn build(&self) -> Self::Proto { Self::Proto { view: Some(self.view.build()), - high_vote: self.high_vote.as_ref().map(ProtoFmt::build), - high_qc: self.high_qc.as_ref().map(ProtoFmt::build), + proposal: Some(self.proposal.build()), } } } -impl ProtoFmt for ReplicaCommit { - type Proto = proto::ReplicaCommit; +impl ProtoFmt for ReplicaTimeout { + type Proto = proto::ReplicaTimeout; fn read(r: &Self::Proto) -> anyhow::Result { Ok(Self { view: read_required(&r.view).context("view")?, - proposal: read_required(&r.proposal).context("proposal")?, + high_vote: read_optional(&r.high_vote).context("high_vote")?, + high_qc: read_optional(&r.high_qc).context("high_qc")?, }) } fn build(&self) -> Self::Proto { Self::Proto { view: Some(self.view.build()), - proposal: Some(self.proposal.build()), + high_vote: self.high_vote.as_ref().map(ProtoFmt::build), + high_qc: self.high_qc.as_ref().map(ProtoFmt::build), } } } -impl ProtoFmt for LeaderProposal { - type Proto = proto::LeaderPrepare; +impl ProtoFmt for ReplicaNewView { + type Proto = proto::ReplicaNewView; fn read(r: &Self::Proto) -> anyhow::Result { Ok(Self { - proposal: read_required(&r.proposal).context("proposal")?, - proposal_payload: r.proposal_payload.as_ref().map(|p| Payload(p.clone())), justification: read_required(&r.justification).context("justification")?, }) } fn build(&self) -> Self::Proto { Self::Proto { - proposal: Some(self.proposal.build()), - proposal_payload: self.proposal_payload.as_ref().map(|p| p.0.clone()), justification: Some(self.justification.build()), } } } -impl ProtoFmt for LeaderCommit { - type Proto = proto::LeaderCommit; +impl ProtoFmt for LeaderProposal { + type Proto = proto::LeaderProposal; fn read(r: &Self::Proto) -> anyhow::Result { Ok(Self { + proposal: read_required(&r.proposal).context("proposal")?, + proposal_payload: r.proposal_payload.as_ref().map(|p| Payload(p.clone())), justification: read_required(&r.justification).context("justification")?, }) } fn build(&self) -> Self::Proto { Self::Proto { + proposal: Some(self.proposal.build()), + proposal_payload: self.proposal_payload.as_ref().map(|p| p.0.clone()), justification: Some(self.justification.build()), } } } -impl ProtoFmt for Signers { - type Proto = zksync_protobuf::proto::std::BitVector; +impl ProtoFmt for CommitQC { + type Proto = proto::CommitQc; fn read(r: &Self::Proto) -> anyhow::Result { - Ok(Self(ProtoFmt::read(r)?)) + Ok(Self { + message: read_required(&r.msg).context("msg")?, + signers: read_required(&r.signers).context("signers")?, + signature: read_required(&r.sig).context("sig")?, + }) } fn build(&self) -> Self::Proto { - self.0.build() + Self::Proto { + msg: Some(self.message.build()), + signers: Some(self.signers.build()), + sig: Some(self.signature.build()), + } } } -impl ProtoFmt for PrepareQC { - type Proto = proto::PrepareQc; +impl ProtoFmt for TimeoutQC { + type Proto = proto::TimeoutQc; fn read(r: &Self::Proto) -> anyhow::Result { let mut map = BTreeMap::new(); for (msg, signers) in r.msgs.iter().zip(r.signers.iter()) { map.insert( - ReplicaPrepare::read(msg).context("msg")?, + ReplicaTimeout::read(msg).context("msg")?, Signers::read(signers).context("signers")?, ); } @@ -349,23 +361,38 @@ impl ProtoFmt for PrepareQC { } } -impl ProtoFmt for CommitQC { - type Proto = proto::CommitQc; +impl ProtoFmt for ProposalJustification { + type Proto = proto::ProposalJustification; fn read(r: &Self::Proto) -> anyhow::Result { - Ok(Self { - message: read_required(&r.msg).context("msg")?, - signers: read_required(&r.signers).context("signers")?, - signature: read_required(&r.sig).context("sig")?, + use proto::proposal_justification::T; + Ok(match r.t.as_ref().context("missing")? { + T::CommitQc(r) => Self::Commit(ProtoFmt::read(r).context("Commit")?), + T::TimeoutQc(r) => Self::Timeout(ProtoFmt::read(r).context("Timeout")?), }) } fn build(&self) -> Self::Proto { - Self::Proto { - msg: Some(self.message.build()), - signers: Some(self.signers.build()), - sig: Some(self.signature.build()), - } + use proto::proposal_justification::T; + + let t = match self { + Self::Commit(x) => T::CommitQc(x.build()), + Self::Timeout(x) => T::TimeoutQc(x.build()), + }; + + Self::Proto { t: Some(t) } + } +} + +impl ProtoFmt for Signers { + type Proto = zksync_protobuf::proto::std::BitVector; + + fn read(r: &Self::Proto) -> anyhow::Result { + Ok(Self(ProtoFmt::read(r)?)) + } + + fn build(&self) -> Self::Proto { + self.0.build() } } @@ -377,6 +404,7 @@ impl ProtoFmt for Phase { Ok(match required(&r.t)? { T::Prepare(_) => Self::Prepare, T::Commit(_) => Self::Commit, + T::Timeout(_) => Self::Timeout, }) } @@ -385,6 +413,7 @@ impl ProtoFmt for Phase { let t = match self { Self::Prepare => T::Prepare(zksync_protobuf::proto::std::Void {}), Self::Commit => T::Commit(zksync_protobuf::proto::std::Void {}), + Self::Timeout => T::Timeout(zksync_protobuf::proto::std::Void {}), }; Self::Proto { t: Some(t) } } diff --git a/node/libs/roles/src/validator/messages/committee.rs b/node/libs/roles/src/validator/messages/committee.rs index d090ea9b2..d5f1fd649 100644 --- a/node/libs/roles/src/validator/messages/committee.rs +++ b/node/libs/roles/src/validator/messages/committee.rs @@ -1,5 +1,5 @@ //! Messages related to the consensus protocol. -use super::{max_faulty_weight, quorum_threshold, subquorum_threshold, Signers, ViewNumber}; +use super::{Signers, ViewNumber}; use crate::validator; use anyhow::Context; use num_bigint::BigUint; @@ -188,3 +188,25 @@ impl LeaderSelectionMode { ret_big.to_u64_digits()[0] } } + +/// Calculate the maximum allowed weight for faulty replicas, for a given total weight. +pub fn max_faulty_weight(total_weight: u64) -> u64 { + // Calculate the allowed maximum weight of faulty replicas. We want the following relationship to hold: + // n = 5*f + 1 + // for n total weight and f faulty weight. This results in the following formula for the maximum + // weight of faulty replicas: + // f = floor((n - 1) / 5) + (total_weight - 1) / 5 +} + +/// Calculate the consensus quorum threshold, the minimum votes' weight necessary to finalize a block, +/// for a given committee total weight. +pub fn quorum_threshold(total_weight: u64) -> u64 { + total_weight - max_faulty_weight(total_weight) +} + +/// Calculate the consensus subquorum threshold, the minimum votes' weight necessary to trigger a reproposal, +/// for a given committee total weight. +pub fn subquorum_threshold(total_weight: u64) -> u64 { + total_weight - 3 * max_faulty_weight(total_weight) +} diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index bc03648df..a13426e12 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -188,4 +188,5 @@ impl std::ops::BitAnd for &Signers { pub enum Phase { Prepare, Commit, + Timeout, } diff --git a/node/libs/roles/src/validator/messages/mod.rs b/node/libs/roles/src/validator/messages/mod.rs index 797cd6251..5153f8327 100644 --- a/node/libs/roles/src/validator/messages/mod.rs +++ b/node/libs/roles/src/validator/messages/mod.rs @@ -23,25 +23,3 @@ pub use msg::*; pub use replica_commit::*; pub use replica_new_view::*; pub use replica_timeout::*; - -/// Calculate the maximum allowed weight for faulty replicas, for a given total weight. -pub fn max_faulty_weight(total_weight: u64) -> u64 { - // Calculate the allowed maximum weight of faulty replicas. We want the following relationship to hold: - // n = 5*f + 1 - // for n total weight and f faulty weight. This results in the following formula for the maximum - // weight of faulty replicas: - // f = floor((n - 1) / 5) - (total_weight - 1) / 5 -} - -/// Calculate the consensus quorum threshold, the minimum votes' weight necessary to finalize a block, -/// for a given committee total weight. -pub fn quorum_threshold(total_weight: u64) -> u64 { - total_weight - max_faulty_weight(total_weight) -} - -/// Calculate the consensus subquorum threshold, the minimum votes' weight necessary to trigger a reproposal, -/// for a given committee total weight. -pub fn subquorum_threshold(total_weight: u64) -> u64 { - total_weight - 3 * max_faulty_weight(total_weight) -} diff --git a/node/libs/roles/src/validator/messages/msg.rs b/node/libs/roles/src/validator/messages/msg.rs index 9e870679a..7fbe6b067 100644 --- a/node/libs/roles/src/validator/messages/msg.rs +++ b/node/libs/roles/src/validator/messages/msg.rs @@ -116,9 +116,9 @@ impl + Clone> Signed { impl> Signed { /// Casts a signed message variant to sub/super variant. /// It is an equivalent of constructing/deconstructing enum values. - pub fn cast>(self) -> Result, BadVariantError> { + pub fn cast>(self) -> Result, BadVariantError> { Ok(Signed { - msg: V::extract(self.msg.insert())?, + msg: U::extract(self.msg.insert())?, key: self.key, sig: self.sig, }) From 8994590bda75cb56d938c5b88f93fa96bf52e681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Tue, 15 Oct 2024 17:02:48 +0100 Subject: [PATCH 04/10] Fixed testonly. Types now build. --- node/libs/roles/src/validator/testonly.rs | 51 ++++++++++++++--------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index 31f36b56d..c9800a4f1 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/node/libs/roles/src/validator/testonly.rs @@ -2,9 +2,10 @@ use super::{ AggregateSignature, Block, BlockHeader, BlockNumber, ChainId, CommitQC, Committee, ConsensusMsg, FinalBlock, ForkNumber, Genesis, GenesisHash, GenesisRaw, Justification, - LeaderCommit, LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, - PreGenesisBlock, PrepareQC, ProofOfPossession, ProtocolVersion, PublicKey, ReplicaCommit, - ReplicaPrepare, SecretKey, Signature, Signed, Signers, View, ViewNumber, WeightedValidator, + LeaderProposal, Msg, MsgHash, NetAddress, Payload, PayloadHash, Phase, PreGenesisBlock, + ProofOfPossession, ProposalJustification, ProtocolVersion, PublicKey, ReplicaCommit, + ReplicaNewView, ReplicaTimeout, SecretKey, Signature, Signed, Signers, TimeoutQC, View, + ViewNumber, WeightedValidator, }; use crate::{attester, validator::LeaderSelectionMode}; use bit_vec::BitVec; @@ -392,9 +393,9 @@ impl Distribution for Standard { } } -impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> ReplicaPrepare { - ReplicaPrepare { +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> ReplicaTimeout { + ReplicaTimeout { view: rng.gen(), high_vote: rng.gen(), high_qc: rng.gen(), @@ -411,30 +412,30 @@ impl Distribution for Standard { } } -impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> LeaderProposal { - LeaderProposal { - proposal: rng.gen(), - proposal_payload: rng.gen(), +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> ReplicaNewView { + ReplicaNewView { justification: rng.gen(), } } } -impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> LeaderCommit { - LeaderCommit { +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> LeaderProposal { + LeaderProposal { + proposal: rng.gen(), + proposal_payload: rng.gen(), justification: rng.gen(), } } } -impl Distribution for Standard { - fn sample(&self, rng: &mut R) -> PrepareQC { +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> TimeoutQC { let n = rng.gen_range(1..11); let map = (0..n).map(|_| (rng.gen(), rng.gen())).collect(); - PrepareQC { + TimeoutQC { view: rng.gen(), map, signature: rng.gen(), @@ -452,6 +453,16 @@ impl Distribution for Standard { } } +impl Distribution for Standard { + fn sample(&self, rng: &mut R) -> ProposalJustification { + match rng.gen_range(0..2) { + 0 => ProposalJustification::Commit(rng.gen()), + 1 => ProposalJustification::Timeout(rng.gen()), + _ => unreachable!(), + } + } +} + impl Distribution for Standard { fn sample(&self, rng: &mut R) -> Signers { Signers(BitVec::from_bytes(&rng.gen::<[u8; 4]>())) @@ -523,10 +534,10 @@ impl Distribution for Standard { impl Distribution for Standard { fn sample(&self, rng: &mut R) -> ConsensusMsg { match rng.gen_range(0..4) { - 0 => ConsensusMsg::ReplicaPrepare(rng.gen()), + 0 => ConsensusMsg::LeaderProposal(rng.gen()), 1 => ConsensusMsg::ReplicaCommit(rng.gen()), - 2 => ConsensusMsg::LeaderPrepare(rng.gen()), - 3 => ConsensusMsg::LeaderCommit(rng.gen()), + 2 => ConsensusMsg::ReplicaNewView(rng.gen()), + 3 => ConsensusMsg::ReplicaTimeout(rng.gen()), _ => unreachable!(), } } From 304850e0613d3eb32df01973eef3faca32ae048f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Wed, 16 Oct 2024 01:07:45 +0100 Subject: [PATCH 05/10] Fixed tests in messages. It does confirm that hash and signature of ReplicaCommit didn't change. --- .../src/validator/messages/leader_proposal.rs | 30 ++-- .../roles/src/validator/messages/tests.rs | 128 ++++++++++-------- node/libs/roles/src/validator/mod.rs | 4 +- 3 files changed, 91 insertions(+), 71 deletions(-) diff --git a/node/libs/roles/src/validator/messages/leader_proposal.rs b/node/libs/roles/src/validator/messages/leader_proposal.rs index 7fa52d7e1..613c6f696 100644 --- a/node/libs/roles/src/validator/messages/leader_proposal.rs +++ b/node/libs/roles/src/validator/messages/leader_proposal.rs @@ -11,7 +11,7 @@ pub struct LeaderProposal { /// Payload of the block that the leader is proposing. /// `None` iff this is a reproposal. pub proposal_payload: Option, - // What attests to the validity of this proposal. + /// What attests to the validity of this proposal. pub justification: ProposalJustification, } @@ -108,22 +108,25 @@ pub enum LeaderProposalVerifyError { }, } +/// Justification for a proposal. This is either a Commit QC or a Timeout QC. +/// The first proposal, for view 0, will always be a timeout. #[derive(Clone, Debug, PartialEq, Eq)] pub enum ProposalJustification { - // This proposal is being proposed after a view where we finalized a block. - // A commit QC is just a collection of commit votes (with at least - // QUORUM_WEIGHT) for the previous view. Note that the commit votes MUST - // be identical. + /// This proposal is being proposed after a view where we finalized a block. + /// A commit QC is just a collection of commit votes (with at least + /// QUORUM_WEIGHT) for the previous view. Note that the commit votes MUST + /// be identical. Commit(CommitQC), - // This proposal is being proposed after a view where we timed out. - // A timeout QC is just a collection of timeout votes (with at least - // QUORUM_WEIGHT) for the previous view. Unlike with the Commit QC, - // timeout votes don't need to be identical. - // The first proposal, for view 0, will always be a timeout. + /// This proposal is being proposed after a view where we timed out. + /// A timeout QC is just a collection of timeout votes (with at least + /// QUORUM_WEIGHT) for the previous view. Unlike with the Commit QC, + /// timeout votes don't need to be identical. + /// The first proposal, for view 0, will always be a timeout. Timeout(TimeoutQC), } impl ProposalJustification { + /// View of the justification. pub fn view(&self) -> View { match self { ProposalJustification::Commit(qc) => qc.view().next(), @@ -131,6 +134,7 @@ impl ProposalJustification { } } + /// Verifies the justification. pub fn verify(&self, genesis: &Genesis) -> Result<(), ProposalJustificationVerifyError> { match self { ProposalJustification::Commit(qc) => qc @@ -142,9 +146,9 @@ impl ProposalJustification { } } - // This returns the BlockNumber that is implied by this justification. - // If the justification requires a block reproposal, it also returns - // the PayloadHash that must be reproposed. + /// This returns the BlockNumber that is implied by this justification. + /// If the justification requires a block reproposal, it also returns + /// the PayloadHash that must be reproposed. pub fn get_implied_block(&self, genesis: &Genesis) -> (BlockNumber, Option) { match self { ProposalJustification::Commit(qc) => { diff --git a/node/libs/roles/src/validator/messages/tests.rs b/node/libs/roles/src/validator/messages/tests.rs index f9968c3b1..7f58184c3 100644 --- a/node/libs/roles/src/validator/messages/tests.rs +++ b/node/libs/roles/src/validator/messages/tests.rs @@ -8,7 +8,14 @@ use zksync_concurrency::ctx; use zksync_consensus_crypto::Text; use zksync_consensus_utils::enum_util::Variant as _; -/// Hardcoded secret keys. +/// Hardcoded view numbers. +fn views() -> impl Iterator { + [8394532, 2297897, 9089304, 7203483, 9982111] + .into_iter() + .map(ViewNumber) +} + +/// Hardcoded validator secret keys. fn validator_keys() -> Vec { [ "validator:secret:bls12_381:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", @@ -22,6 +29,7 @@ fn validator_keys() -> Vec { .collect() } +/// Hardcoded attester secret keys. fn attester_keys() -> Vec { [ "attester:secret:secp256k1:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", @@ -33,7 +41,7 @@ fn attester_keys() -> Vec { .collect() } -/// Hardcoded committee. +/// Hardcoded validator committee. fn validator_committee() -> Committee { Committee::new( validator_keys() @@ -47,6 +55,7 @@ fn validator_committee() -> Committee { .unwrap() } +/// Hardcoded attester committee. fn attester_committee() -> attester::Committee { attester::Committee::new( attester_keys() @@ -70,7 +79,7 @@ fn payload() -> Payload { /// Checks that the order of validators in a committee is stable. #[test] -fn committee_change_detector() { +fn validator_committee_change_detector() { let committee = validator_committee(); let got: Vec = validator_keys() .iter() @@ -128,13 +137,6 @@ fn test_rota() { } } -/// Hardcoded view numbers. -fn views() -> impl Iterator { - [8394532, 2297897, 9089304, 7203483, 9982111] - .into_iter() - .map(ViewNumber) -} - /// Checks that leader schedule is stable. #[test] fn roundrobin_change_detector() { @@ -166,7 +168,7 @@ fn weighted_change_detector() { mod version1 { use super::*; - /// Hardcoded genesis. + /// Hardcoded genesis with no attesters. fn genesis_empty_attesters() -> Genesis { GenesisRaw { chain_id: ChainId(1337), @@ -181,7 +183,7 @@ mod version1 { .with_hash() } - /// Hardcoded genesis. + /// Hardcoded genesis with attesters. fn genesis_with_attesters() -> Genesis { GenesisRaw { chain_id: ChainId(1337), @@ -199,7 +201,6 @@ mod version1 { /// Note that genesis is NOT versioned by ProtocolVersion. /// Even if it was, ALL versions of genesis need to be supported FOREVER, /// unless we introduce dynamic regenesis. - /// FIXME: This fails with the new attester committee. #[test] fn genesis_hash_change_detector_empty_attesters() { let want: GenesisHash = Text::new( @@ -213,7 +214,6 @@ mod version1 { /// Note that genesis is NOT versioned by ProtocolVersion. /// Even if it was, ALL versions of genesis need to be supported FOREVER, /// unless we introduce dynamic regenesis. - /// FIXME: This fails with the new attester committee. #[test] fn genesis_hash_change_detector_nonempty_attesters() { let want: GenesisHash = Text::new( @@ -233,21 +233,6 @@ mod version1 { assert!(genesis.verify().is_err()) } - /// asserts that msg.hash()==hash and that sig is a - /// valid signature of msg (signed by `keys()[0]`). - #[track_caller] - fn change_detector(msg: Msg, hash: &str, sig: &str) { - let key = validator_keys()[0].clone(); - (|| { - let hash: MsgHash = Text::new(hash).decode()?; - let sig: Signature = Text::new(sig).decode()?; - sig.verify_hash(&hash, &key.public())?; - anyhow::Ok(()) - })() - .with_context(|| format!("\n{:?},\n{:?}", msg.hash(), key.sign_hash(&msg.hash()),)) - .unwrap(); - } - /// Hardcoded view. fn view() -> View { View { @@ -284,43 +269,74 @@ mod version1 { x } - /// Hardcoded `LeaderCommit`. - fn leader_commit() -> LeaderCommit { - LeaderCommit { - justification: commit_qc(), + /// Hardcoded `ReplicaNewView`. + fn replica_new_view() -> ReplicaNewView { + ReplicaNewView { + justification: ProposalJustification::Commit(commit_qc()), } } - /// Hardcoded `ReplicaPrepare` - fn replica_prepare() -> ReplicaPrepare { - ReplicaPrepare { + /// Hardcoded `ReplicaTimeout` + fn replica_timeout() -> ReplicaTimeout { + ReplicaTimeout { view: view(), high_vote: Some(replica_commit()), high_qc: Some(commit_qc()), } } - /// Hardcoded `PrepareQC`. - fn prepare_qc() -> PrepareQC { - let mut x = PrepareQC::new(view()); + /// Hardcoded `TimeoutQC`. + fn timeout_qc() -> TimeoutQC { + let mut x = TimeoutQC::new(view()); let genesis = genesis_empty_attesters(); - let replica_prepare = replica_prepare(); + let replica_timeout = replica_timeout(); for k in validator_keys() { - x.add(&k.sign_msg(replica_prepare.clone()), &genesis) + x.add(&k.sign_msg(replica_timeout.clone()), &genesis) .unwrap(); } x } - /// Hardcoded `LeaderPrepare`. - fn leader_prepare() -> LeaderProposal { + /// Hardcoded `LeaderProposal`. + fn leader_proposal() -> LeaderProposal { LeaderProposal { proposal: block_header(), proposal_payload: Some(payload()), - justification: prepare_qc(), + justification: ProposalJustification::Timeout(timeout_qc()), } } + /// Asserts that msg.hash()==hash and that sig is a + /// valid signature of msg (signed by `keys()[0]`). + #[track_caller] + fn change_detector(msg: Msg, hash: &str, sig: &str) { + let key = validator_keys()[0].clone(); + + (|| { + // Decode hash and signature. + let hash: MsgHash = Text::new(hash).decode()?; + let sig: Signature = Text::new(sig).decode()?; + + // Check if msg.hash() is equal to hash. + if msg.hash() != hash { + return Err(anyhow::anyhow!("Hash mismatch")); + } + + // Check if sig is a valid signature of hash. + sig.verify_hash(&hash, &key.public())?; + + anyhow::Ok(()) + })() + .with_context(|| { + format!( + "\nIntended hash: {:?}\nIntended signature: {:?}", + msg.hash(), + key.sign_hash(&msg.hash()), + ) + }) + .unwrap(); + } + #[test] fn replica_commit_change_detector() { change_detector( @@ -331,29 +347,29 @@ mod version1 { } #[test] - fn leader_commit_change_detector() { + fn replica_new_view_change_detector() { change_detector( - leader_commit().insert(), - "validator_msg:keccak256:53b8d7dc77a5ba8b81cd7b46ddc19c224ef46245c26cb7ae12239acc1bf86eda", - "validator:signature:bls12_381:81a154a93a8b607031319915728be97c03c3014a4746050f7a32cde98cabe4fbd2b6d6b79400601a71f50350842d1d64", + replica_new_view().insert(), + "validator_msg:keccak256:be1d87c8fcabeb1dd6aebef8564994830f206997d3cf240ad19281a8ef84fbd1", + "validator:signature:bls12_381:92640683902cd5c092fe8732f556d2bb3e0e209665ba4fe8e6f9ce2572a43700a63e82c3fbfd803fd217b3027ca843ca", ); } #[test] - fn replica_prepare_change_detector() { + fn replica_timeout_change_detector() { change_detector( - replica_prepare().insert(), - "validator_msg:keccak256:700cf26d50f463cfa908f914d1febb1cbd00ee9d3a691b644f49146ed3e6ac40", - "validator:signature:bls12_381:a7cbdf9b8d13ebc39f4a13d654ec30acccd247d46fc6121eb1220256cfc212b418aac85400176e8797d8eb91aa70ae78", + replica_timeout().insert(), + "validator_msg:keccak256:c3a2dbbb01fa4884effdbad3cdfea7c03f3fa0d1b8ae15b221ac83e1d7abb9df", + "validator:signature:bls12_381:acb18d66816a0e7e2c1fbaa2d5045ead1a41eba8692ef0cdbf3f3d3f5a5616fa77903cc45326090cca1468912419d824", ); } #[test] - fn leader_prepare_change_detector() { + fn leader_proposal_change_detector() { change_detector( - leader_prepare().insert(), - "validator_msg:keccak256:aaaaa6b7b232ef5b7c797953ce2a042c024137d7b8f449a1ad8a535730bc269b", - "validator:signature:bls12_381:a1926f460fa63470544cc9213e6378f45d75dff3055924766a81ff696a6a6e85ee583707911bb7fef4d1f74b7b28132f", + leader_proposal().insert(), + "validator_msg:keccak256:ecf7cd55c9b44ae5c361d23067af4f3bf5aa0f4d170f4d33b8ce4214c9963c6b", + "validator:signature:bls12_381:b650efd18dd800363aafd8ca67e3b6f14963f99b094fb497a3edad56816108a4e835ffcec714320e6fe1b42c30057005", ); } } diff --git a/node/libs/roles/src/validator/mod.rs b/node/libs/roles/src/validator/mod.rs index 2091a1ada..78b7a191d 100644 --- a/node/libs/roles/src/validator/mod.rs +++ b/node/libs/roles/src/validator/mod.rs @@ -1,7 +1,7 @@ //! Validator role implementation. -#[cfg(test)] -mod tests; +//#[cfg(test)] +//mod tests; mod conv; mod keys; From d1de7c195382c8b2833563128fe62c9fd5b08e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Wed, 16 Oct 2024 01:12:25 +0100 Subject: [PATCH 06/10] Fixed validator tests. All tests in roles pass now. --- node/libs/roles/src/validator/mod.rs | 4 +-- node/libs/roles/src/validator/tests.rs | 40 +++++++++++++------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/node/libs/roles/src/validator/mod.rs b/node/libs/roles/src/validator/mod.rs index 78b7a191d..2091a1ada 100644 --- a/node/libs/roles/src/validator/mod.rs +++ b/node/libs/roles/src/validator/mod.rs @@ -1,7 +1,7 @@ //! Validator role implementation. -//#[cfg(test)] -//mod tests; +#[cfg(test)] +mod tests; mod conv; mod keys; diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index 3348ae943..6bba151bd 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -90,7 +90,7 @@ fn test_schema_encoding() { test_encode_random::(rng); test_encode_random::(rng); test_encode_random::>(rng); - test_encode_random::(rng); + test_encode_random::(rng); test_encode_random::(rng); test_encode_random::(rng); test_encode_random::(rng); @@ -197,8 +197,8 @@ fn make_commit_qc(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> Commit qc } -fn make_replica_prepare(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> ReplicaPrepare { - ReplicaPrepare { +fn make_replica_timeout(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> ReplicaTimeout { + ReplicaTimeout { view: make_view(view, setup), high_vote: { let view = ViewNumber(rng.gen_range(0..view.0)); @@ -306,8 +306,8 @@ fn test_commit_qc_add_errors() { } #[test] -fn test_prepare_qc() { - use PrepareQCVerifyError as Error; +fn test_timeout_qc() { + use TimeoutQCVerifyError as Error; let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); @@ -321,11 +321,11 @@ fn test_prepare_qc() { let view: ViewNumber = rng.gen(); let msgs: Vec<_> = (0..3) - .map(|_| make_replica_prepare(rng, view, &setup1)) + .map(|_| make_replica_timeout(rng, view, &setup1)) .collect(); for n in 0..setup1.validator_keys.len() + 1 { - let mut qc = PrepareQC::new(msgs[0].view.clone()); + let mut qc = TimeoutQC::new(msgs[0].view.clone()); for key in &setup1.validator_keys[0..n] { qc.add( &key.sign_msg(msgs.choose(rng).unwrap().clone()), @@ -364,12 +364,12 @@ fn test_prepare_qc_high_vote() { let setup = Setup::new(rng, 6); let view_num: ViewNumber = rng.gen(); - let msg_a = make_replica_prepare(rng, view_num, &setup); - let msg_b = make_replica_prepare(rng, view_num, &setup); - let msg_c = make_replica_prepare(rng, view_num, &setup); + let msg_a = make_replica_timeout(rng, view_num, &setup); + let msg_b = make_replica_timeout(rng, view_num, &setup); + let msg_c = make_replica_timeout(rng, view_num, &setup); // Case with 1 subquorum. - let mut qc = PrepareQC::new(msg_a.view.clone()); + let mut qc = TimeoutQC::new(msg_a.view.clone()); for key in &setup.validator_keys { qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) @@ -379,7 +379,7 @@ fn test_prepare_qc_high_vote() { assert!(qc.high_vote(&setup.genesis).is_some()); // Case with 2 subquorums. - let mut qc = PrepareQC::new(msg_a.view.clone()); + let mut qc = TimeoutQC::new(msg_a.view.clone()); for key in &setup.validator_keys[0..3] { qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) @@ -394,7 +394,7 @@ fn test_prepare_qc_high_vote() { assert!(qc.high_vote(&setup.genesis).is_none()); // Case with no subquorums. - let mut qc = PrepareQC::new(msg_a.view.clone()); + let mut qc = TimeoutQC::new(msg_a.view.clone()); for key in &setup.validator_keys[0..2] { qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) @@ -416,14 +416,14 @@ fn test_prepare_qc_high_vote() { #[test] fn test_prepare_qc_add_errors() { - use PrepareQCAddError as Error; + use TimeoutQCAddError as Error; let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); let setup = Setup::new(rng, 2); let view = rng.gen(); - let msg = make_replica_prepare(rng, view, &setup); - let mut qc = PrepareQC::new(msg.view.clone()); - let msg = make_replica_prepare(rng, view, &setup); + let msg = make_replica_timeout(rng, view, &setup); + let mut qc = TimeoutQC::new(msg.view.clone()); + let msg = make_replica_timeout(rng, view, &setup); // Add the message assert_matches!( @@ -461,7 +461,7 @@ fn test_prepare_qc_add_errors() { ); // Try to add a message for a validator that already added another message - let msg2 = make_replica_prepare(rng, view, &setup); + let msg2 = make_replica_timeout(rng, view, &setup); assert_matches!( qc.add(&setup.validator_keys[0].sign_msg(msg2), &setup.genesis), Err(Error::Exists { .. }) @@ -488,8 +488,8 @@ fn test_validator_committee_weights() { let sums = [1000, 1600, 2400, 8400, 9300, 10000]; let view: ViewNumber = rng.gen(); - let msg = make_replica_prepare(rng, view, &setup); - let mut qc = PrepareQC::new(msg.view.clone()); + let msg = make_replica_timeout(rng, view, &setup); + let mut qc = TimeoutQC::new(msg.view.clone()); for (n, weight) in sums.iter().enumerate() { let key = &setup.validator_keys[n]; qc.add(&key.sign_msg(msg.clone()), &setup.genesis).unwrap(); From 6ad24205b09d04d5e6df1ae841732828cea4ffd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Wed, 16 Oct 2024 02:21:20 +0100 Subject: [PATCH 07/10] Nits. --- .../src/validator/messages/leader_proposal.rs | 33 +++--- .../src/validator/messages/replica_commit.rs | 88 ++++++++------ .../validator/messages/replica_new_view.rs | 2 +- .../src/validator/messages/replica_timeout.rs | 108 +++++++++++------- node/libs/roles/src/validator/tests.rs | 6 +- 5 files changed, 145 insertions(+), 92 deletions(-) diff --git a/node/libs/roles/src/validator/messages/leader_proposal.rs b/node/libs/roles/src/validator/messages/leader_proposal.rs index 613c6f696..c421e0543 100644 --- a/node/libs/roles/src/validator/messages/leader_proposal.rs +++ b/node/libs/roles/src/validator/messages/leader_proposal.rs @@ -3,7 +3,7 @@ use super::{ TimeoutQC, TimeoutQCVerifyError, View, }; -/// A Proposal message from the leader. +/// A proposal message from the leader. #[derive(Clone, Debug, PartialEq, Eq)] pub struct LeaderProposal { /// The header of the block that the leader is proposing. @@ -56,7 +56,7 @@ impl LeaderProposal { return Err(LeaderProposalVerifyError::NewProposalWhenPreviousNotFinalized); } - // Check that the payload matches the header. + // Check that the payload matches the header, if it exists. if let Some(payload) = &self.proposal_payload { if payload.hash() != self.proposal.payload { return Err(LeaderProposalVerifyError::MismatchedPayload { @@ -74,10 +74,10 @@ impl LeaderProposal { #[derive(thiserror::Error, Debug)] pub enum LeaderProposalVerifyError { /// Invalid Justification. - #[error("justification: {0:#}")] + #[error("Invalid justification: {0:#}")] Justification(ProposalJustificationVerifyError), /// Bad block number. - #[error("bad block number: got {got:?}, want {want:?}")] + #[error("Bad block number: got {got:?}, want {want:?}")] BadBlockNumber { /// Received proposal number. got: BlockNumber, @@ -85,7 +85,7 @@ pub enum LeaderProposalVerifyError { want: BlockNumber, }, /// Bad payload hash on reproposal. - #[error("bad payload hash on reproposal: got {got:?}, want {want:?}")] + #[error("Bad payload hash on reproposal: got {got:?}, want {want:?}")] BadPayloadHash { /// Received payload hash. got: PayloadHash, @@ -93,13 +93,13 @@ pub enum LeaderProposalVerifyError { want: PayloadHash, }, /// New block proposal when the previous proposal was not finalized. - #[error("new block proposal when the previous proposal was not finalized")] + #[error("New block proposal when the previous proposal was not finalized")] NewProposalWhenPreviousNotFinalized, /// Re-proposal when the previous proposal was finalized. - #[error("block re-proposal when the previous proposal was finalized")] + #[error("Block re-proposal when the previous proposal was finalized")] ReproposalWhenPreviousFinalized, /// Mismatched payload. - #[error("block proposal with mismatched payload: header {header:?}, payload {payload:?}")] + #[error("Block proposal with mismatched payload: header {header:?}, payload {payload:?}")] MismatchedPayload { /// Payload hash on block header. header: PayloadHash, @@ -166,8 +166,13 @@ impl ProposalJustification { // Get the high commit QC of the timeout QC. We compare the high QC field of // all timeout votes in the QC, and get the highest one, if it exists. + // The high QC always exists, unless no block has been finalized yet in the chain. let high_qc = qc.high_qc(); + // If there was a high vote in the timeout QC, and either there was no high QC + // in the timeout QC, or the high vote is for a higher block than the high QC, + // then we need to repropose the high vote. + #[allow(clippy::unnecessary_unwrap)] // using a match would be more verbose if high_vote.is_some() && (high_qc.is_none() || high_vote.unwrap().number > high_qc.unwrap().header().number) @@ -177,8 +182,8 @@ impl ProposalJustification { (high_vote.unwrap().number, Some(high_vote.unwrap().payload)) } else { // Either the previous proposal was finalized or we know for certain - // that it couldn't have been finalized. Either way, we can propose - // a new block. + // that it couldn't have been finalized (because there is no high vote). + // Either way, we can propose a new block. let block_number = match high_qc { Some(qc) => qc.header().number.next(), None => BlockNumber(0), @@ -194,10 +199,10 @@ impl ProposalJustification { /// Error returned by `ProposalJustification::verify()`. #[derive(thiserror::Error, Debug)] pub enum ProposalJustificationVerifyError { - /// Invalid Timeout QC. - #[error("timeout qc: {0:#}")] + /// Invalid timeout QC. + #[error("Invalid timeout QC: {0:#}")] Timeout(TimeoutQCVerifyError), - /// Invalid Commit QC. - #[error("commit qc: {0:#}")] + /// Invalid commit QC. + #[error("Invalid commit QC: {0:#}")] Commit(CommitQCVerifyError), } diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index 1c645959b..6d4b98f4a 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -1,7 +1,7 @@ use super::{BlockHeader, Genesis, Signed, Signers, View}; use crate::validator; -/// A Commit message from a replica. +/// A commit message from a replica. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ReplicaCommit { /// View of this message. @@ -36,11 +36,11 @@ pub enum ReplicaCommitVerifyError { BadBlockNumber, } -/// A Commit Quorum Certificate. It is an aggregate of signed replica Commit messages. -/// The Quorum Certificate is supposed to be over identical messages, so we only need one message. +/// A Commit Quorum Certificate. It is an aggregate of signed ReplicaCommit messages. +/// The Commit Quorum Certificate is over identical messages, so we only need one message. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CommitQC { - /// The replica Commit message that the QC is for. + /// The ReplicaCommit message that the QC is for. pub message: ReplicaCommit, /// The validators that signed this message. pub signers: Signers, @@ -68,39 +68,54 @@ impl CommitQC { } } - /// Add a validator's signature. - /// Signature is assumed to be already verified. + /// Add a validator's signature. This also verifies the message and the signature before adding. pub fn add( &mut self, msg: &Signed, genesis: &Genesis, ) -> Result<(), CommitQCAddError> { - if self.message != msg.msg { - return Err(CommitQCAddError::InconsistentMessages); - }; - + // Check if the signer is in the committee. let Some(i) = genesis.validators.index(&msg.key) else { return Err(CommitQCAddError::SignerNotInCommittee { signer: Box::new(msg.key.clone()), }); }; + // Check if already have a message from the same signer. if self.signers.0[i] { - return Err(CommitQCAddError::Exists); + return Err(CommitQCAddError::DuplicateSigner { + signer: Box::new(msg.key.clone()), + }); }; + // Verify the signature. + msg.verify().map_err(CommitQCAddError::BadSignature)?; + + // Check that the message is consistent with the CommitQC. + if self.message != msg.msg { + return Err(CommitQCAddError::InconsistentMessages); + }; + + // Check that the message itself is valid. + msg.msg + .verify(genesis) + .map_err(CommitQCAddError::InvalidMessage)?; + + // Add the signer to the signers map, and the signature to the aggregate signature. self.signers.0.set(i, true); self.signature.add(&msg.sig); Ok(()) } - /// Verifies the signature of the CommitQC. + /// Verifies the integrity of the CommitQC. pub fn verify(&self, genesis: &Genesis) -> Result<(), CommitQCVerifyError> { + // Check that the message is valid. self.message .verify(genesis) .map_err(CommitQCVerifyError::InvalidMessage)?; + // Check that the signers set has the same size as the validator set. if self.signers.len() != genesis.validators.len() { return Err(CommitQCVerifyError::BadSignersSet); } @@ -129,14 +144,40 @@ impl CommitQC { } } -/// Error returned by `CommitQc::verify()`. +/// Error returned by `CommitQC::add()`. +#[derive(thiserror::Error, Debug)] +pub enum CommitQCAddError { + /// Signer not present in the committee. + #[error("Signer not in committee: {signer:?}")] + SignerNotInCommittee { + /// Signer of the message. + signer: Box, + }, + /// Message from the same signer already present in QC. + #[error("Message from the same signer already in QC: {signer:?}")] + DuplicateSigner { + /// Signer of the message. + signer: Box, + }, + /// Bad signature. + #[error("Bad signature: {0:#}")] + BadSignature(#[source] anyhow::Error), + /// Inconsistent messages. + #[error("Trying to add signature for a different message")] + InconsistentMessages, + /// Invalid message. + #[error("Invalid message: {0:#}")] + InvalidMessage(ReplicaCommitVerifyError), +} + +/// Error returned by `CommitQC::verify()`. #[derive(thiserror::Error, Debug)] pub enum CommitQCVerifyError { /// Invalid message. #[error(transparent)] InvalidMessage(#[from] ReplicaCommitVerifyError), /// Bad signer set. - #[error("signers set doesn't match genesis")] + #[error("Signers set doesn't match validator set")] BadSignersSet, /// Weight not reached. #[error("Signers have not reached threshold weight: got {got}, want {want}")] @@ -147,23 +188,6 @@ pub enum CommitQCVerifyError { want: u64, }, /// Bad signature. - #[error("bad signature: {0:#}")] + #[error("Bad signature: {0:#}")] BadSignature(#[source] anyhow::Error), } - -/// Error returned by `CommitQC::add()`. -#[derive(thiserror::Error, Debug)] -pub enum CommitQCAddError { - /// Inconsistent messages. - #[error("Trying to add signature for a different message")] - InconsistentMessages, - /// Signer not present in the committee. - #[error("Signer not in committee: {signer:?}")] - SignerNotInCommittee { - /// Signer of the message. - signer: Box, - }, - /// Message already present in CommitQC. - #[error("Message already signed for CommitQC")] - Exists, -} diff --git a/node/libs/roles/src/validator/messages/replica_new_view.rs b/node/libs/roles/src/validator/messages/replica_new_view.rs index e127f02ba..79b93e250 100644 --- a/node/libs/roles/src/validator/messages/replica_new_view.rs +++ b/node/libs/roles/src/validator/messages/replica_new_view.rs @@ -1,6 +1,6 @@ use super::{Genesis, ProposalJustification, ProposalJustificationVerifyError, View}; -/// A NewView message from a replica. +/// A new view message from a replica. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ReplicaNewView { /// What attests to the validity of this view change. diff --git a/node/libs/roles/src/validator/messages/replica_timeout.rs b/node/libs/roles/src/validator/messages/replica_timeout.rs index 0c81f7408..02f4b4481 100644 --- a/node/libs/roles/src/validator/messages/replica_timeout.rs +++ b/node/libs/roles/src/validator/messages/replica_timeout.rs @@ -5,7 +5,7 @@ use super::{ use crate::validator; use std::collections::{BTreeMap, HashMap}; -/// A Timeout message from a replica. +/// A timeout message from a replica. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct ReplicaTimeout { /// View of this message. @@ -63,21 +63,21 @@ pub enum ReplicaTimeoutVerifyError { HighQC(CommitQCVerifyError), } -/// A quorum certificate of replica Timeout messages. Since not all Timeout messages are -/// identical (they have different high blocks and high QCs), we need to keep the high blocks -/// and high QCs in a map. We can still aggregate the signatures though. +/// A quorum certificate of ReplicaTimeout messages. Since not all ReplicaTimeout messages are +/// identical (they have different high blocks and high QCs), we need to keep the ReplicaTimeout +/// messages in a map. We can still aggregate the signatures though. #[derive(Clone, Debug, PartialEq, Eq)] pub struct TimeoutQC { /// View of this QC. pub view: View, /// Map from replica Timeout messages to the validators that signed them. pub map: BTreeMap, - /// Aggregate signature of the replica Timeout messages. + /// Aggregate signature of the ReplicaTimeout messages. pub signature: validator::AggregateSignature, } impl TimeoutQC { - /// Create a new empty instance for a given `ReplicaCommit` message and a validator set size. + /// Create a new empty TimeoutQC for a given view. pub fn new(view: View) -> Self { Self { view, @@ -86,11 +86,10 @@ impl TimeoutQC { } } - /// Get the highest block voted and check if there's a quorum of votes for it. To have a quorum - /// in this situation, we require 2*f+1 votes, where f is the maximum number of faulty replicas. - /// Note that it is possible to have 2 quorums: vote A and vote B, each with >2f weight, in a single - /// TimeoutQC (even in the unweighted case, because QC contains n-f signatures, not 4f+1). In such a - /// situation we say that there is no high vote. + /// Get the highest block voted and check if there's a subquorum of votes for it. To have a subquorum + /// in this situation, we require n-3*f votes, where f is the maximum number of faulty replicas. + /// Note that it is possible to have 2 subquorums: vote A and vote B, each with >n-3*f weight, in a single + /// TimeoutQC. In such a situation we say that there is no high vote. pub fn high_vote(&self, genesis: &Genesis) -> Option { let mut count: HashMap<_, u64> = HashMap::new(); for (msg, signers) in &self.map { @@ -99,7 +98,7 @@ impl TimeoutQC { } } - let min = 2 * genesis.validators.max_faulty_weight() + 1; + let min = genesis.validators.subquorum_threshold(); let mut high_votes: Vec<_> = count.into_iter().filter(|x| x.1 >= min).collect(); if high_votes.len() == 1 { @@ -117,28 +116,40 @@ impl TimeoutQC { .max_by_key(|qc| qc.view().number) } - /// Add a validator's signed message. - /// Message is assumed to be already verified. - // TODO: verify the message inside instead. + /// Add a validator's signed message. This also verifies the message and the signature before adding. pub fn add( &mut self, msg: &Signed, genesis: &Genesis, ) -> Result<(), TimeoutQCAddError> { - if msg.msg.view != self.view { - return Err(TimeoutQCAddError::InconsistentViews); - } - + // Check if the signer is in the committee. let Some(i) = genesis.validators.index(&msg.key) else { return Err(TimeoutQCAddError::SignerNotInCommittee { signer: Box::new(msg.key.clone()), }); }; + // Check if already have a message from the same signer. if self.map.values().any(|s| s.0[i]) { - return Err(TimeoutQCAddError::Exists); + return Err(TimeoutQCAddError::DuplicateSigner { + signer: Box::new(msg.key.clone()), + }); }; + // Verify the signature. + msg.verify().map_err(TimeoutQCAddError::BadSignature)?; + + // Check that the view is consistent with the TimeoutQC. + if msg.msg.view != self.view { + return Err(TimeoutQCAddError::InconsistentViews); + }; + + // Check that the message itself is valid. + msg.msg + .verify(genesis) + .map_err(TimeoutQCAddError::InvalidMessage)?; + + // Add the message plus signer to the map, and the signature to the aggregate signature. let e = self .map .entry(msg.msg.clone()) @@ -154,6 +165,7 @@ impl TimeoutQC { self.view .verify(genesis) .map_err(TimeoutQCVerifyError::View)?; + let mut sum = Signers::new(genesis.validators.len()); // Check the ReplicaTimeout messages. @@ -178,10 +190,11 @@ impl TimeoutQC { } msg.verify(genesis) .map_err(|err| TimeoutQCVerifyError::InvalidMessage(i, err))?; + sum |= signers; } - // Verify the signers' weight is enough. + // Check if the signers' weight is enough. let weight = genesis.validators.weight(&sum); let threshold = genesis.validators.quorum_threshold(); if weight < threshold { @@ -190,6 +203,7 @@ impl TimeoutQC { want: threshold, }); } + // Now we can verify the signature. let messages_and_keys = self.map.clone().into_iter().flat_map(|(msg, signers)| { genesis @@ -200,6 +214,7 @@ impl TimeoutQC { .map(|(_, pk)| (msg.clone(), pk)) .collect::>() }); + // TODO(gprusak): This reaggregating is suboptimal. self.signature .verify_messages(messages_and_keys) @@ -215,17 +230,43 @@ impl TimeoutQC { } } +/// Error returned by `TimeoutQC::add()`. +#[derive(thiserror::Error, Debug)] +pub enum TimeoutQCAddError { + /// Signer not present in the committee. + #[error("Signer not in committee: {signer:?}")] + SignerNotInCommittee { + /// Signer of the message. + signer: Box, + }, + /// Message from the same signer already present in QC. + #[error("Message from the same signer already in QC: {signer:?}")] + DuplicateSigner { + /// Signer of the message. + signer: Box, + }, + /// Bad signature. + #[error("Bad signature: {0:#}")] + BadSignature(#[source] anyhow::Error), + /// Inconsistent views. + #[error("Trying to add a message from a different view")] + InconsistentViews, + /// Invalid message. + #[error("Invalid message: {0:#}")] + InvalidMessage(ReplicaTimeoutVerifyError), +} + /// Error returned by `TimeoutQC::verify()`. #[derive(thiserror::Error, Debug)] pub enum TimeoutQCVerifyError { /// Bad view. - #[error("view: {0:#}")] + #[error("Bad view: {0:#}")] View(anyhow::Error), /// Inconsistent views. - #[error("inconsistent views of signed messages")] + #[error("Inconsistent views of signed messages")] InconsistentViews, /// Invalid message. - #[error("msg[{0}]: {1:#}")] + #[error("Invalid message: number [{0}], {1:#}")] InvalidMessage(usize, ReplicaTimeoutVerifyError), /// Bad message format. #[error(transparent)] @@ -239,23 +280,6 @@ pub enum TimeoutQCVerifyError { want: u64, }, /// Bad signature. - #[error("bad signature: {0:#}")] + #[error("Bad signature: {0:#}")] BadSignature(#[source] anyhow::Error), } - -/// Error returned by `TimeoutQC::add()`. -#[derive(thiserror::Error, Debug)] -pub enum TimeoutQCAddError { - /// Inconsistent views. - #[error("Trying to add a message from a different view")] - InconsistentViews, - /// Signer not present in the committee. - #[error("Signer not in committee: {signer:?}")] - SignerNotInCommittee { - /// Signer of the message. - signer: Box, - }, - /// Message already present in TimeoutQC. - #[error("Message already signed for TimeoutQC")] - Exists, -} diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index 6bba151bd..f2616d499 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -295,7 +295,7 @@ fn test_commit_qc_add_errors() { &setup.validator_keys[0].sign_msg(msg.clone()), &setup.genesis ), - Err(Error::Exists { .. }) + Err(Error::DuplicateSigner { .. }) ); // Add same message signed by another validator. @@ -457,14 +457,14 @@ fn test_prepare_qc_add_errors() { &setup.validator_keys[0].sign_msg(msg.clone()), &setup.genesis ), - Err(Error::Exists { .. }) + Err(Error::DuplicateSigner { .. }) ); // Try to add a message for a validator that already added another message let msg2 = make_replica_timeout(rng, view, &setup); assert_matches!( qc.add(&setup.validator_keys[0].sign_msg(msg2), &setup.genesis), - Err(Error::Exists { .. }) + Err(Error::DuplicateSigner { .. }) ); // Add same message signed by another validator. From 125263179f71608a1245445a93de9ca007226f1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Sun, 20 Oct 2024 19:51:22 +0100 Subject: [PATCH 08/10] More tests. --- node/actors/bft/src/leader/tests.rs | 2 +- node/actors/bft/src/replica/tests.rs | 2 +- node/libs/crypto/src/keccak256/mod.rs | 2 +- node/libs/roles/src/validator/keys/mod.rs | 2 + .../roles/src/validator/keys/signature.rs | 58 +-- node/libs/roles/src/validator/keys/tests.rs | 60 +++ .../roles/src/validator/messages/block.rs | 1 + .../roles/src/validator/messages/committee.rs | 3 +- .../roles/src/validator/messages/consensus.rs | 13 + .../roles/src/validator/messages/genesis.rs | 6 +- .../src/validator/messages/replica_commit.rs | 4 +- .../roles/src/validator/messages/tests.rs | 375 --------------- .../src/validator/messages/tests/block.rs | 71 +++ .../src/validator/messages/tests/committee.rs | 198 ++++++++ .../src/validator/messages/tests/consensus.rs | 92 ++++ .../src/validator/messages/tests/genesis.rs | 30 ++ .../messages/tests/leader_proposal.rs | 97 ++++ .../roles/src/validator/messages/tests/mod.rs | 192 ++++++++ .../messages/tests/replica_commit.rs | 95 ++++ .../messages/tests/replica_timeout.rs | 174 +++++++ .../src/validator/messages/tests/versions.rs | 102 ++++ node/libs/roles/src/validator/testonly.rs | 192 ++++++-- node/libs/roles/src/validator/tests.rs | 436 +----------------- 23 files changed, 1314 insertions(+), 893 deletions(-) create mode 100644 node/libs/roles/src/validator/keys/tests.rs delete mode 100644 node/libs/roles/src/validator/messages/tests.rs create mode 100644 node/libs/roles/src/validator/messages/tests/block.rs create mode 100644 node/libs/roles/src/validator/messages/tests/committee.rs create mode 100644 node/libs/roles/src/validator/messages/tests/consensus.rs create mode 100644 node/libs/roles/src/validator/messages/tests/genesis.rs create mode 100644 node/libs/roles/src/validator/messages/tests/leader_proposal.rs create mode 100644 node/libs/roles/src/validator/messages/tests/mod.rs create mode 100644 node/libs/roles/src/validator/messages/tests/replica_commit.rs create mode 100644 node/libs/roles/src/validator/messages/tests/replica_timeout.rs create mode 100644 node/libs/roles/src/validator/messages/tests/versions.rs diff --git a/node/actors/bft/src/leader/tests.rs b/node/actors/bft/src/leader/tests.rs index 3c0683362..e64533848 100644 --- a/node/actors/bft/src/leader/tests.rs +++ b/node/actors/bft/src/leader/tests.rs @@ -564,7 +564,7 @@ async fn replica_commit_bad_chain() { assert_matches!( res, Err(replica_commit::Error::InvalidMessage( - validator::ReplicaCommitVerifyError::View(_) + validator::ReplicaCommitVerifyError::BadView(_) )) ); Ok(()) diff --git a/node/actors/bft/src/replica/tests.rs b/node/actors/bft/src/replica/tests.rs index 10d7407dc..afb2a8aa3 100644 --- a/node/actors/bft/src/replica/tests.rs +++ b/node/actors/bft/src/replica/tests.rs @@ -550,7 +550,7 @@ async fn leader_commit_bad_chain() { res, Err(leader_commit::Error::InvalidMessage( validator::CommitQCVerifyError::InvalidMessage( - validator::ReplicaCommitVerifyError::View(_) + validator::ReplicaCommitVerifyError::BadView(_) ) )) ); diff --git a/node/libs/crypto/src/keccak256/mod.rs b/node/libs/crypto/src/keccak256/mod.rs index 1c94c2435..ce7bd7538 100644 --- a/node/libs/crypto/src/keccak256/mod.rs +++ b/node/libs/crypto/src/keccak256/mod.rs @@ -7,7 +7,7 @@ mod test; pub mod testonly; /// Keccak256 hash. -#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Keccak256(pub(crate) [u8; 32]); impl Keccak256 { diff --git a/node/libs/roles/src/validator/keys/mod.rs b/node/libs/roles/src/validator/keys/mod.rs index f8d18c948..a48160157 100644 --- a/node/libs/roles/src/validator/keys/mod.rs +++ b/node/libs/roles/src/validator/keys/mod.rs @@ -4,6 +4,8 @@ mod aggregate_signature; mod public_key; mod secret_key; mod signature; +#[cfg(test)] +mod tests; pub use aggregate_signature::AggregateSignature; pub use public_key::PublicKey; diff --git a/node/libs/roles/src/validator/keys/signature.rs b/node/libs/roles/src/validator/keys/signature.rs index 76e3419c0..d5ed9583f 100644 --- a/node/libs/roles/src/validator/keys/signature.rs +++ b/node/libs/roles/src/validator/keys/signature.rs @@ -19,14 +19,9 @@ impl Signature { } } -/// Proof of possession of a validator secret key. -#[derive(Clone, PartialEq, Eq)] -pub struct ProofOfPossession(pub(crate) bls12_381::ProofOfPossession); - -impl ProofOfPossession { - /// Verifies the proof against the public key. - pub fn verify(&self, pk: &PublicKey) -> anyhow::Result<()> { - self.0.verify(&pk.0) +impl fmt::Debug for Signature { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str(&TextFmt::encode(self)) } } @@ -39,15 +34,6 @@ impl ByteFmt for Signature { } } -impl ByteFmt for ProofOfPossession { - fn encode(&self) -> Vec { - ByteFmt::encode(&self.0) - } - fn decode(bytes: &[u8]) -> anyhow::Result { - ByteFmt::decode(bytes).map(Self) - } -} - impl TextFmt for Signature { fn encode(&self) -> String { format!( @@ -62,6 +48,32 @@ impl TextFmt for Signature { } } +/// Proof of possession of a validator secret key. +#[derive(Clone, PartialEq, Eq)] +pub struct ProofOfPossession(pub(crate) bls12_381::ProofOfPossession); + +impl ProofOfPossession { + /// Verifies the proof against the public key. + pub fn verify(&self, pk: &PublicKey) -> anyhow::Result<()> { + self.0.verify(&pk.0) + } +} + +impl fmt::Debug for ProofOfPossession { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str(&TextFmt::encode(self)) + } +} + +impl ByteFmt for ProofOfPossession { + fn encode(&self) -> Vec { + ByteFmt::encode(&self.0) + } + fn decode(bytes: &[u8]) -> anyhow::Result { + ByteFmt::decode(bytes).map(Self) + } +} + impl TextFmt for ProofOfPossession { fn encode(&self) -> String { format!( @@ -75,15 +87,3 @@ impl TextFmt for ProofOfPossession { .map(Self) } } - -impl fmt::Debug for ProofOfPossession { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str(&TextFmt::encode(self)) - } -} - -impl fmt::Debug for Signature { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str(&TextFmt::encode(self)) - } -} diff --git a/node/libs/roles/src/validator/keys/tests.rs b/node/libs/roles/src/validator/keys/tests.rs new file mode 100644 index 000000000..b52e87783 --- /dev/null +++ b/node/libs/roles/src/validator/keys/tests.rs @@ -0,0 +1,60 @@ +use super::*; +use crate::validator::MsgHash; +use rand::Rng as _; +use std::vec; +use zksync_concurrency::ctx; + +#[test] +fn test_signature_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + let msg1: MsgHash = rng.gen(); + let msg2: MsgHash = rng.gen(); + + let key1: SecretKey = rng.gen(); + let key2: SecretKey = rng.gen(); + + let sig1 = key1.sign_hash(&msg1); + + // Matching key and message. + sig1.verify_hash(&msg1, &key1.public()).unwrap(); + + // Mismatching message. + assert!(sig1.verify_hash(&msg2, &key1.public()).is_err()); + + // Mismatching key. + assert!(sig1.verify_hash(&msg1, &key2.public()).is_err()); +} + +#[test] +fn test_agg_signature_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + let msg1: MsgHash = rng.gen(); + let msg2: MsgHash = rng.gen(); + + let key1: SecretKey = rng.gen(); + let key2: SecretKey = rng.gen(); + + let sig1 = key1.sign_hash(&msg1); + let sig2 = key2.sign_hash(&msg2); + + let agg_sig = AggregateSignature::aggregate(vec![&sig1, &sig2]); + + // Matching key and message. + agg_sig + .verify_hash([(msg1, &key1.public()), (msg2, &key2.public())].into_iter()) + .unwrap(); + + // Mismatching message. + assert!(agg_sig + .verify_hash([(msg2, &key1.public()), (msg1, &key2.public())].into_iter()) + .is_err()); + + // Mismatching key. + assert!(agg_sig + .verify_hash([(msg1, &key2.public()), (msg2, &key1.public())].into_iter()) + .is_err()); +} diff --git a/node/libs/roles/src/validator/messages/block.rs b/node/libs/roles/src/validator/messages/block.rs index cf1070701..7db2628bb 100644 --- a/node/libs/roles/src/validator/messages/block.rs +++ b/node/libs/roles/src/validator/messages/block.rs @@ -102,6 +102,7 @@ impl FinalBlock { /// Creates a new finalized block. pub fn new(payload: Payload, justification: CommitQC) -> Self { assert_eq!(justification.header().payload, payload.hash()); + Self { payload, justification, diff --git a/node/libs/roles/src/validator/messages/committee.rs b/node/libs/roles/src/validator/messages/committee.rs index d5f1fd649..018dbb4fb 100644 --- a/node/libs/roles/src/validator/messages/committee.rs +++ b/node/libs/roles/src/validator/messages/committee.rs @@ -24,7 +24,8 @@ impl std::ops::Deref for Committee { } impl Committee { - /// Creates a new Committee from a list of validator public keys. + /// Creates a new Committee from a list of validator public keys. Note that the order of the given validators + /// is NOT preserved in the committee. pub fn new(validators: impl IntoIterator) -> anyhow::Result { let mut map = BTreeMap::new(); let mut total_weight: u64 = 0; diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index a13426e12..1b723714d 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -114,6 +114,14 @@ impl View { number: ViewNumber(self.number.0 + 1), } } + + /// Decrements the view number. + pub fn prev(self) -> Option { + self.number.prev().map(|number| Self { + genesis: self.genesis, + number, + }) + } } /// A struct that represents a view number. @@ -125,6 +133,11 @@ impl ViewNumber { pub fn next(self) -> Self { Self(self.0 + 1) } + + /// Get the previous view number. + pub fn prev(self) -> Option { + self.0.checked_sub(1).map(Self) + } } impl fmt::Display for ViewNumber { diff --git a/node/libs/roles/src/validator/messages/genesis.rs b/node/libs/roles/src/validator/messages/genesis.rs index 57e5e1cd1..9af95411c 100644 --- a/node/libs/roles/src/validator/messages/genesis.rs +++ b/node/libs/roles/src/validator/messages/genesis.rs @@ -1,5 +1,5 @@ //! Messages related to the consensus protocol. -use super::{BlockNumber, Committee, LeaderSelectionMode, ViewNumber}; +use super::{BlockNumber, LeaderSelectionMode, ViewNumber}; use crate::{attester, validator}; use std::{fmt, hash::Hash}; use zksync_consensus_crypto::{keccak256::Keccak256, ByteFmt, Text, TextFmt}; @@ -17,7 +17,7 @@ pub struct GenesisRaw { /// First block of a fork. pub first_block: BlockNumber, /// Set of validators of the chain. - pub validators: Committee, + pub validators: validator::Committee, /// Set of attesters of the chain. pub attesters: Option, /// The mode used for selecting leader for a given view. @@ -33,7 +33,7 @@ impl GenesisRaw { } /// Hash of the genesis specification. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct GenesisHash(pub(crate) Keccak256); impl TextFmt for GenesisHash { diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index 6d4b98f4a..76f86606d 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -15,7 +15,7 @@ impl ReplicaCommit { pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaCommitVerifyError> { self.view .verify(genesis) - .map_err(ReplicaCommitVerifyError::View)?; + .map_err(ReplicaCommitVerifyError::BadView)?; if self.proposal.number < genesis.first_block { return Err(ReplicaCommitVerifyError::BadBlockNumber); @@ -30,7 +30,7 @@ impl ReplicaCommit { pub enum ReplicaCommitVerifyError { /// Invalid view. #[error("view: {0:#}")] - View(anyhow::Error), + BadView(anyhow::Error), /// Bad block number. #[error("block number < first block")] BadBlockNumber, diff --git a/node/libs/roles/src/validator/messages/tests.rs b/node/libs/roles/src/validator/messages/tests.rs deleted file mode 100644 index 7f58184c3..000000000 --- a/node/libs/roles/src/validator/messages/tests.rs +++ /dev/null @@ -1,375 +0,0 @@ -use crate::{ - attester::{self, WeightedAttester}, - validator::*, -}; -use anyhow::Context as _; -use rand::{prelude::StdRng, Rng, SeedableRng}; -use zksync_concurrency::ctx; -use zksync_consensus_crypto::Text; -use zksync_consensus_utils::enum_util::Variant as _; - -/// Hardcoded view numbers. -fn views() -> impl Iterator { - [8394532, 2297897, 9089304, 7203483, 9982111] - .into_iter() - .map(ViewNumber) -} - -/// Hardcoded validator secret keys. -fn validator_keys() -> Vec { - [ - "validator:secret:bls12_381:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", - "validator:secret:bls12_381:20132edc08a529e927f155e710ae7295a2a0d249f1b1f37726894d1d0d8f0d81", - "validator:secret:bls12_381:0946901f0a6650284726763b12de5da0f06df0016c8ec2144cf6b1903f1979a6", - "validator:secret:bls12_381:3143a64c079b2f50545288d7c9b282281e05c97ac043228830a9660ddd63fea3", - "validator:secret:bls12_381:5512f40d33844c1c8107aa630af764005ab6e13f6bf8edb59b4ca3683727e619", - ] - .iter() - .map(|raw| Text::new(raw).decode().unwrap()) - .collect() -} - -/// Hardcoded attester secret keys. -fn attester_keys() -> Vec { - [ - "attester:secret:secp256k1:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", - "attester:secret:secp256k1:20132edc08a529e927f155e710ae7295a2a0d249f1b1f37726894d1d0d8f0d81", - "attester:secret:secp256k1:0946901f0a6650284726763b12de5da0f06df0016c8ec2144cf6b1903f1979a6", - ] - .iter() - .map(|raw| Text::new(raw).decode().unwrap()) - .collect() -} - -/// Hardcoded validator committee. -fn validator_committee() -> Committee { - Committee::new( - validator_keys() - .iter() - .enumerate() - .map(|(i, key)| WeightedValidator { - key: key.public(), - weight: i as u64 + 10, - }), - ) - .unwrap() -} - -/// Hardcoded attester committee. -fn attester_committee() -> attester::Committee { - attester::Committee::new( - attester_keys() - .iter() - .enumerate() - .map(|(i, key)| WeightedAttester { - key: key.public(), - weight: i as u64 + 10, - }), - ) - .unwrap() -} - -/// Hardcoded payload. -fn payload() -> Payload { - Payload( - hex::decode("57b79660558f18d56b5196053f64007030a1cb7eeadb5c32d816b9439f77edf5f6bd9d") - .unwrap(), - ) -} - -/// Checks that the order of validators in a committee is stable. -#[test] -fn validator_committee_change_detector() { - let committee = validator_committee(); - let got: Vec = validator_keys() - .iter() - .map(|k| committee.index(&k.public()).unwrap()) - .collect(); - assert_eq!(vec![0, 1, 4, 3, 2], got); -} - -#[test] -fn payload_hash_change_detector() { - let want: PayloadHash = Text::new( - "payload:keccak256:ba8ffff2526cae27a9e8e014749014b08b80e01905c8b769159d02d6579d9b83", - ) - .decode() - .unwrap(); - assert_eq!(want, payload().hash()); -} - -#[test] -fn test_sticky() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - let committee = validator_committee(); - let want = committee - .get(rng.gen_range(0..committee.len())) - .unwrap() - .key - .clone(); - let sticky = LeaderSelectionMode::Sticky(want.clone()); - for _ in 0..100 { - assert_eq!(want, committee.view_leader(rng.gen(), &sticky)); - } -} - -#[test] -fn test_rota() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - let committee = validator_committee(); - let mut want = Vec::new(); - for _ in 0..3 { - want.push( - committee - .get(rng.gen_range(0..committee.len())) - .unwrap() - .key - .clone(), - ); - } - let rota = LeaderSelectionMode::Rota(want.clone()); - for _ in 0..100 { - let vn: ViewNumber = rng.gen(); - let pk = &want[vn.0 as usize % want.len()]; - assert_eq!(*pk, committee.view_leader(vn, &rota)); - } -} - -/// Checks that leader schedule is stable. -#[test] -fn roundrobin_change_detector() { - let committee = validator_committee(); - let mode = LeaderSelectionMode::RoundRobin; - let got: Vec<_> = views() - .map(|view| { - let got = committee.view_leader(view, &mode); - committee.index(&got).unwrap() - }) - .collect(); - assert_eq!(vec![2, 2, 4, 3, 1], got); -} - -/// Checks that leader schedule is stable. -#[test] -fn weighted_change_detector() { - let committee = validator_committee(); - let mode = LeaderSelectionMode::Weighted; - let got: Vec<_> = views() - .map(|view| { - let got = committee.view_leader(view, &mode); - committee.index(&got).unwrap() - }) - .collect(); - assert_eq!(vec![4, 2, 2, 2, 1], got); -} - -mod version1 { - use super::*; - - /// Hardcoded genesis with no attesters. - fn genesis_empty_attesters() -> Genesis { - GenesisRaw { - chain_id: ChainId(1337), - fork_number: ForkNumber(402598740274745173), - first_block: BlockNumber(8902834932452), - - protocol_version: ProtocolVersion(1), - validators: validator_committee(), - attesters: None, - leader_selection: LeaderSelectionMode::Weighted, - } - .with_hash() - } - - /// Hardcoded genesis with attesters. - fn genesis_with_attesters() -> Genesis { - GenesisRaw { - chain_id: ChainId(1337), - fork_number: ForkNumber(402598740274745173), - first_block: BlockNumber(8902834932452), - - protocol_version: ProtocolVersion(1), - validators: validator_committee(), - attesters: attester_committee().into(), - leader_selection: LeaderSelectionMode::Weighted, - } - .with_hash() - } - - /// Note that genesis is NOT versioned by ProtocolVersion. - /// Even if it was, ALL versions of genesis need to be supported FOREVER, - /// unless we introduce dynamic regenesis. - #[test] - fn genesis_hash_change_detector_empty_attesters() { - let want: GenesisHash = Text::new( - "genesis_hash:keccak256:13a16cfa758c6716b4c4d40a5fe71023a016c7507b7893c7dc775f4420fc5d61", - ) - .decode() - .unwrap(); - assert_eq!(want, genesis_empty_attesters().hash()); - } - - /// Note that genesis is NOT versioned by ProtocolVersion. - /// Even if it was, ALL versions of genesis need to be supported FOREVER, - /// unless we introduce dynamic regenesis. - #[test] - fn genesis_hash_change_detector_nonempty_attesters() { - let want: GenesisHash = Text::new( - "genesis_hash:keccak256:47a52a5491873fa4ceb369a334b4c09833a06bd34718fb22e530ab4d70b4daf7", - ) - .decode() - .unwrap(); - assert_eq!(want, genesis_with_attesters().hash()); - } - - #[test] - fn genesis_verify_leader_pubkey_not_in_committee() { - let mut rng = StdRng::seed_from_u64(29483920); - let mut genesis = rng.gen::(); - genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); - let genesis = genesis.with_hash(); - assert!(genesis.verify().is_err()) - } - - /// Hardcoded view. - fn view() -> View { - View { - genesis: genesis_empty_attesters().hash(), - number: ViewNumber(9136573498460759103), - } - } - - /// Hardcoded `BlockHeader`. - fn block_header() -> BlockHeader { - BlockHeader { - number: BlockNumber(772839452345), - payload: payload().hash(), - } - } - - /// Hardcoded `ReplicaCommit`. - fn replica_commit() -> ReplicaCommit { - ReplicaCommit { - view: view(), - proposal: block_header(), - } - } - - /// Hardcoded `CommitQC`. - fn commit_qc() -> CommitQC { - let genesis = genesis_empty_attesters(); - let replica_commit = replica_commit(); - let mut x = CommitQC::new(replica_commit.clone(), &genesis); - for k in validator_keys() { - x.add(&k.sign_msg(replica_commit.clone()), &genesis) - .unwrap(); - } - x - } - - /// Hardcoded `ReplicaNewView`. - fn replica_new_view() -> ReplicaNewView { - ReplicaNewView { - justification: ProposalJustification::Commit(commit_qc()), - } - } - - /// Hardcoded `ReplicaTimeout` - fn replica_timeout() -> ReplicaTimeout { - ReplicaTimeout { - view: view(), - high_vote: Some(replica_commit()), - high_qc: Some(commit_qc()), - } - } - - /// Hardcoded `TimeoutQC`. - fn timeout_qc() -> TimeoutQC { - let mut x = TimeoutQC::new(view()); - let genesis = genesis_empty_attesters(); - let replica_timeout = replica_timeout(); - for k in validator_keys() { - x.add(&k.sign_msg(replica_timeout.clone()), &genesis) - .unwrap(); - } - x - } - - /// Hardcoded `LeaderProposal`. - fn leader_proposal() -> LeaderProposal { - LeaderProposal { - proposal: block_header(), - proposal_payload: Some(payload()), - justification: ProposalJustification::Timeout(timeout_qc()), - } - } - - /// Asserts that msg.hash()==hash and that sig is a - /// valid signature of msg (signed by `keys()[0]`). - #[track_caller] - fn change_detector(msg: Msg, hash: &str, sig: &str) { - let key = validator_keys()[0].clone(); - - (|| { - // Decode hash and signature. - let hash: MsgHash = Text::new(hash).decode()?; - let sig: Signature = Text::new(sig).decode()?; - - // Check if msg.hash() is equal to hash. - if msg.hash() != hash { - return Err(anyhow::anyhow!("Hash mismatch")); - } - - // Check if sig is a valid signature of hash. - sig.verify_hash(&hash, &key.public())?; - - anyhow::Ok(()) - })() - .with_context(|| { - format!( - "\nIntended hash: {:?}\nIntended signature: {:?}", - msg.hash(), - key.sign_hash(&msg.hash()), - ) - }) - .unwrap(); - } - - #[test] - fn replica_commit_change_detector() { - change_detector( - replica_commit().insert(), - "validator_msg:keccak256:2ec798684e539d417fac1caba74ed1a27a033bc18058ba0a4632f6bb0ae4fe1c", - "validator:signature:bls12_381:8de9ad850d78eb4f918c8c3a02310be49fc9ac35f2b1fdd6489293db1d5128f0d4c8389674e6bc2eee4c6e16f58e0b51", - ); - } - - #[test] - fn replica_new_view_change_detector() { - change_detector( - replica_new_view().insert(), - "validator_msg:keccak256:be1d87c8fcabeb1dd6aebef8564994830f206997d3cf240ad19281a8ef84fbd1", - "validator:signature:bls12_381:92640683902cd5c092fe8732f556d2bb3e0e209665ba4fe8e6f9ce2572a43700a63e82c3fbfd803fd217b3027ca843ca", - ); - } - - #[test] - fn replica_timeout_change_detector() { - change_detector( - replica_timeout().insert(), - "validator_msg:keccak256:c3a2dbbb01fa4884effdbad3cdfea7c03f3fa0d1b8ae15b221ac83e1d7abb9df", - "validator:signature:bls12_381:acb18d66816a0e7e2c1fbaa2d5045ead1a41eba8692ef0cdbf3f3d3f5a5616fa77903cc45326090cca1468912419d824", - ); - } - - #[test] - fn leader_proposal_change_detector() { - change_detector( - leader_proposal().insert(), - "validator_msg:keccak256:ecf7cd55c9b44ae5c361d23067af4f3bf5aa0f4d170f4d33b8ce4214c9963c6b", - "validator:signature:bls12_381:b650efd18dd800363aafd8ca67e3b6f14963f99b094fb497a3edad56816108a4e835ffcec714320e6fe1b42c30057005", - ); - } -} diff --git a/node/libs/roles/src/validator/messages/tests/block.rs b/node/libs/roles/src/validator/messages/tests/block.rs new file mode 100644 index 000000000..e3a3ea520 --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/block.rs @@ -0,0 +1,71 @@ +use super::*; +use assert_matches::assert_matches; +use rand::Rng; +use validator::testonly::Setup; +use zksync_concurrency::ctx; +use zksync_consensus_crypto::{keccak256::Keccak256, Text}; + +#[test] +fn payload_hash_change_detector() { + let want: PayloadHash = Text::new( + "payload:keccak256:ba8ffff2526cae27a9e8e014749014b08b80e01905c8b769159d02d6579d9b83", + ) + .decode() + .unwrap(); + assert_eq!(want, payload().hash()); +} + +#[test] +fn test_payload_hash() { + let data = vec![1, 2, 3, 4]; + let payload = Payload(data.clone()); + let hash = payload.hash(); + assert_eq!(hash.0, Keccak256::new(&data)); +} + +#[test] +fn test_block_number_next() { + let block_number = BlockNumber(5); + assert_eq!(block_number.next(), BlockNumber(6)); +} + +#[test] +fn test_block_number_prev() { + let block_number = BlockNumber(5); + assert_eq!(block_number.prev(), Some(BlockNumber(4))); + + let block_number_zero = BlockNumber(0); + assert_eq!(block_number_zero.prev(), None); +} + +#[test] +fn test_block_number_add() { + let block_number = BlockNumber(5); + assert_eq!(block_number + 3, BlockNumber(8)); +} + +#[test] +fn test_final_block_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let setup = Setup::new(rng, 2); + + let payload: Payload = rng.gen(); + let view_number = rng.gen(); + let commit_qc = setup.make_commit_qc_with_payload(&payload, view_number); + let mut final_block = FinalBlock::new(payload.clone(), commit_qc.clone()); + + assert!(final_block.verify(&setup.genesis).is_ok()); + + final_block.payload = rng.gen(); + assert_matches!( + final_block.verify(&setup.genesis), + Err(BlockValidationError::HashMismatch { .. }) + ); + + final_block.justification.message.proposal.payload = final_block.payload.hash(); + assert_matches!( + final_block.verify(&setup.genesis), + Err(BlockValidationError::Justification(_)) + ); +} diff --git a/node/libs/roles/src/validator/messages/tests/committee.rs b/node/libs/roles/src/validator/messages/tests/committee.rs new file mode 100644 index 000000000..0b9968f9c --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/committee.rs @@ -0,0 +1,198 @@ +use super::*; +use rand::Rng; +use zksync_concurrency::ctx; + +/// Checks that the order of validators in a committee is stable. +#[test] +fn test_committee_order_change_detector() { + let committee = validator_committee(); + let got: Vec = validator_keys() + .iter() + .map(|k| committee.index(&k.public()).unwrap()) + .collect(); + assert_eq!(vec![0, 1, 4, 3, 2], got); +} + +fn create_validator(weight: u64) -> WeightedValidator { + WeightedValidator { + key: validator::SecretKey::generate().public(), + weight, + } +} + +#[test] +fn test_committee_new() { + let validators = vec![create_validator(10), create_validator(20)]; + let committee = Committee::new(validators).unwrap(); + assert_eq!(committee.len(), 2); + assert_eq!(committee.total_weight(), 30); +} + +#[test] +fn test_committee_new_duplicate_validator() { + let mut validators = vec![create_validator(10), create_validator(20)]; + validators[1].key = validators[0].key.clone(); + let result = Committee::new(validators); + assert!(result.is_err()); +} + +#[test] +fn test_committee_new_zero_weight() { + let validators = vec![create_validator(10), create_validator(0)]; + let result = Committee::new(validators); + assert!(result.is_err()); +} + +#[test] +fn test_committee_weights_overflow_check() { + let validators: Vec = [u64::MAX / 5; 6] + .iter() + .map(|w| create_validator(*w)) + .collect(); + let result = Committee::new(validators); + assert!(result.is_err()); +} + +#[test] +fn test_committee_new_empty() { + let validators = vec![]; + let result = Committee::new(validators); + assert!(result.is_err()); +} + +#[test] +fn test_committee_contains() { + let validators = vec![create_validator(10), create_validator(20)]; + let committee = Committee::new(validators.clone()).unwrap(); + assert!(committee.contains(&validators[0].key)); + assert!(!committee.contains(&validator::SecretKey::generate().public())); +} + +#[test] +fn test_committee_get() { + let validators = validator_keys() + .into_iter() + .map(|x| x.public()) + .collect::>(); + let committee = validator_committee(); + assert_eq!(committee.get(0).unwrap().key, validators[0]); + assert_eq!(committee.get(1).unwrap().key, validators[1]); + assert_eq!(committee.get(2).unwrap().key, validators[4]); + assert_eq!(committee.get(3).unwrap().key, validators[3]); + assert_eq!(committee.get(4).unwrap().key, validators[2]); + assert!(committee.get(5).is_none()); +} + +#[test] +fn test_committee_index() { + let validators = validator_keys() + .into_iter() + .map(|x| x.public()) + .collect::>(); + let committee = validator_committee(); + assert_eq!(committee.index(&validators[0]), Some(0)); + assert_eq!(committee.index(&validators[1]), Some(1)); + assert_eq!(committee.index(&validators[4]), Some(2)); + assert_eq!(committee.index(&validators[3]), Some(3)); + assert_eq!(committee.index(&validators[2]), Some(4)); + assert_eq!( + committee.index(&validator::SecretKey::generate().public()), + None + ); +} + +#[test] +fn test_committee_view_leader_round_robin() { + let committee = validator_committee(); + let mode = LeaderSelectionMode::RoundRobin; + let got: Vec<_> = views() + .map(|view| { + let got = committee.view_leader(view, &mode); + committee.index(&got).unwrap() + }) + .collect(); + assert_eq!(vec![2, 3, 4, 4, 1], got); +} + +#[test] +fn test_committee_view_leader_weighted() { + let committee = validator_committee(); + let mode = LeaderSelectionMode::Weighted; + let got: Vec<_> = views() + .map(|view| { + let got = committee.view_leader(view, &mode); + committee.index(&got).unwrap() + }) + .collect(); + assert_eq!(vec![2, 3, 2, 1, 3], got); +} + +#[test] +fn test_committee_view_leader_sticky() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let committee = validator_committee(); + let want = committee + .get(rng.gen_range(0..committee.len())) + .unwrap() + .key + .clone(); + let sticky = LeaderSelectionMode::Sticky(want.clone()); + for _ in 0..100 { + assert_eq!(want, committee.view_leader(rng.gen(), &sticky)); + } +} + +#[test] +fn test_committee_view_leader_rota() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let committee = validator_committee(); + let mut want = Vec::new(); + for _ in 0..3 { + want.push( + committee + .get(rng.gen_range(0..committee.len())) + .unwrap() + .key + .clone(), + ); + } + let rota = LeaderSelectionMode::Rota(want.clone()); + for _ in 0..100 { + let vn: ViewNumber = rng.gen(); + let pk = &want[vn.0 as usize % want.len()]; + assert_eq!(*pk, committee.view_leader(vn, &rota)); + } +} + +#[test] +fn test_committee_quorum_threshold() { + let validators = vec![create_validator(10), create_validator(20)]; + let committee = Committee::new(validators).unwrap(); + assert_eq!(committee.quorum_threshold(), 25); // 30 - (30 - 1) / 5 +} + +#[test] +fn test_committee_subquorum_threshold() { + let validators = vec![create_validator(10), create_validator(20)]; + let committee = Committee::new(validators).unwrap(); + assert_eq!(committee.subquorum_threshold(), 15); // 30 - 3 * (30 - 1) / 5 +} + +#[test] +fn test_committee_max_faulty_weight() { + let validators = vec![create_validator(10), create_validator(20)]; + let committee = Committee::new(validators).unwrap(); + assert_eq!(committee.max_faulty_weight(), 5); // (30 - 1) / 5 +} + +#[test] +fn test_committee_weight() { + let committee = validator_committee(); + let mut signers = Signers::new(5); + signers.0.set(1, true); + signers.0.set(2, true); + signers.0.set(4, true); + assert_eq!(committee.weight(&signers), 37); +} diff --git a/node/libs/roles/src/validator/messages/tests/consensus.rs b/node/libs/roles/src/validator/messages/tests/consensus.rs new file mode 100644 index 000000000..632f7508a --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/consensus.rs @@ -0,0 +1,92 @@ +use super::*; + +#[test] +fn test_view_next() { + let view = View { + genesis: GenesisHash::default(), + number: ViewNumber(1), + }; + let next_view = view.next(); + assert_eq!(next_view.number, ViewNumber(2)); +} + +#[test] +fn test_view_prev() { + let view = View { + genesis: GenesisHash::default(), + number: ViewNumber(1), + }; + let prev_view = view.prev(); + assert_eq!(prev_view.unwrap().number, ViewNumber(0)); + let view = View { + genesis: GenesisHash::default(), + number: ViewNumber(0), + }; + let prev_view = view.prev(); + assert!(prev_view.is_none()); +} + +#[test] +fn test_view_verify() { + let genesis = genesis_with_attesters(); + let view = View { + genesis: genesis.hash(), + number: ViewNumber(1), + }; + assert!(view.verify(&genesis).is_ok()); + assert!(view.verify(&genesis_empty_attesters()).is_err()); + let view = View { + genesis: GenesisHash::default(), + number: ViewNumber(1), + }; + assert!(view.verify(&genesis).is_err()); +} + +#[test] +fn test_signers_new() { + let signers = Signers::new(10); + assert_eq!(signers.len(), 10); + assert!(signers.is_empty()); +} + +#[test] +fn test_signers_count() { + let mut signers = Signers::new(10); + signers.0.set(0, true); + signers.0.set(1, true); + assert_eq!(signers.count(), 2); +} + +#[test] +fn test_signers_empty() { + let mut signers = Signers::new(10); + assert!(signers.is_empty()); + signers.0.set(1, true); + assert!(!signers.is_empty()); + signers.0.set(1, false); + assert!(signers.is_empty()); +} + +#[test] +fn test_signers_bitor_assign() { + let mut signers1 = Signers::new(10); + let mut signers2 = Signers::new(10); + signers1.0.set(0, true); + signers1.0.set(3, true); + signers2.0.set(1, true); + signers2.0.set(3, true); + signers1 |= &signers2; + assert_eq!(signers1.count(), 3); +} + +#[test] +fn test_signers_bitand_assign() { + let mut signers1 = Signers::new(10); + let mut signers2 = Signers::new(10); + signers1.0.set(0, true); + signers1.0.set(3, true); + signers2.0.set(1, true); + signers2.0.set(3, true); + signers1 &= &signers2; + assert_eq!(signers1.count(), 1); +} diff --git a/node/libs/roles/src/validator/messages/tests/genesis.rs b/node/libs/roles/src/validator/messages/tests/genesis.rs new file mode 100644 index 000000000..cdeac273d --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/genesis.rs @@ -0,0 +1,30 @@ +use super::*; +use rand::{prelude::StdRng, Rng, SeedableRng}; +use validator::testonly::Setup; +use zksync_concurrency::ctx; +use zksync_protobuf::ProtoFmt as _; + +#[test] +fn genesis_verify_leader_pubkey_not_in_committee() { + let mut rng = StdRng::seed_from_u64(29483920); + let mut genesis = rng.gen::(); + genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); + let genesis = genesis.with_hash(); + assert!(genesis.verify().is_err()) +} + +#[test] +fn test_genesis_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + let genesis = Setup::new(rng, 1).genesis.clone(); + assert!(genesis.verify().is_ok()); + assert!(Genesis::read(&genesis.build()).is_ok()); + + let mut genesis = (*genesis).clone(); + genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); + let genesis = genesis.with_hash(); + assert!(genesis.verify().is_err()); + assert!(Genesis::read(&genesis.build()).is_err()) +} diff --git a/node/libs/roles/src/validator/messages/tests/leader_proposal.rs b/node/libs/roles/src/validator/messages/tests/leader_proposal.rs new file mode 100644 index 000000000..3c1fdac37 --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/leader_proposal.rs @@ -0,0 +1,97 @@ +use super::*; +use assert_matches::assert_matches; +use zksync_concurrency::ctx; + +#[test] +fn test_leader_proposal_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + // This will create equally weighted validators + let mut setup = Setup::new(rng, 6); + setup.push_blocks(rng, 3); + + // Valid proposal + let payload: Payload = rng.gen(); + let block_header = BlockHeader { + number: setup.next(), + payload: payload.hash(), + }; + let commit_qc = match setup.blocks.last().unwrap() { + Block::Final(block) => block.justification.clone(), + _ => unreachable!(), + }; + let justification = ProposalJustification::Commit(commit_qc); + let proposal = LeaderProposal { + proposal: block_header, + proposal_payload: Some(payload.clone()), + justification, + }; + + assert!(proposal.verify(&setup.genesis).is_ok()); + + // Invalid justification + let mut wrong_proposal = proposal.clone(); + wrong_proposal.justification = ProposalJustification::Timeout(rng.gen()); + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::Justification(_)) + ); + + // Invalid block number + let mut wrong_proposal = proposal.clone(); + wrong_proposal.proposal.number = BlockNumber(1); + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::BadBlockNumber { .. }) + ); + + // Wrong reproposal + let mut wrong_proposal = proposal.clone(); + wrong_proposal.proposal_payload = None; + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::ReproposalWhenPreviousFinalized) + ); + + // Invalid payload + let mut wrong_proposal = proposal.clone(); + wrong_proposal.proposal.payload = rng.gen(); + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::MismatchedPayload { .. }) + ); + + // New leader proposal with a reproposal + let timeout_qc = setup.make_timeout_qc(rng, ViewNumber(7), Some(&payload)); + let justification = ProposalJustification::Timeout(timeout_qc); + let proposal = LeaderProposal { + proposal: block_header, + proposal_payload: None, + justification, + }; + + assert!(proposal.verify(&setup.genesis).is_ok()); + + // Invalid payload hash + let mut wrong_proposal = proposal.clone(); + wrong_proposal.proposal.payload = rng.gen(); + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::BadPayloadHash { .. }) + ); + + // Wrong new proposal + let mut wrong_proposal = proposal.clone(); + wrong_proposal.proposal_payload = Some(rng.gen()); + + assert_matches!( + wrong_proposal.verify(&setup.genesis), + Err(LeaderProposalVerifyError::NewProposalWhenPreviousNotFinalized) + ); +} diff --git a/node/libs/roles/src/validator/messages/tests/mod.rs b/node/libs/roles/src/validator/messages/tests/mod.rs new file mode 100644 index 000000000..b8b703b72 --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/mod.rs @@ -0,0 +1,192 @@ +use super::*; +use crate::{ + attester::{self, WeightedAttester}, + validator::{self, testonly::Setup}, +}; +use rand::Rng; +use zksync_consensus_crypto::Text; + +mod block; +mod committee; +mod consensus; +mod genesis; +mod leader_proposal; +mod replica_commit; +mod replica_timeout; +mod versions; + +/// Hardcoded view. +fn view() -> View { + View { + genesis: genesis_empty_attesters().hash(), + number: ViewNumber(9136), + } +} + +/// Hardcoded view numbers. +fn views() -> impl Iterator { + [2297, 7203, 8394, 9089, 99821].into_iter().map(ViewNumber) +} + +/// Hardcoded payload. +fn payload() -> Payload { + Payload( + hex::decode("57b79660558f18d56b5196053f64007030a1cb7eeadb5c32d816b9439f77edf5f6bd9d") + .unwrap(), + ) +} + +/// Hardcoded `BlockHeader`. +fn block_header() -> BlockHeader { + BlockHeader { + number: BlockNumber(7728), + payload: payload().hash(), + } +} + +/// Hardcoded validator secret keys. +fn validator_keys() -> Vec { + [ + "validator:secret:bls12_381:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", + "validator:secret:bls12_381:20132edc08a529e927f155e710ae7295a2a0d249f1b1f37726894d1d0d8f0d81", + "validator:secret:bls12_381:0946901f0a6650284726763b12de5da0f06df0016c8ec2144cf6b1903f1979a6", + "validator:secret:bls12_381:3143a64c079b2f50545288d7c9b282281e05c97ac043228830a9660ddd63fea3", + "validator:secret:bls12_381:5512f40d33844c1c8107aa630af764005ab6e13f6bf8edb59b4ca3683727e619", + ] + .iter() + .map(|raw| Text::new(raw).decode().unwrap()) + .collect() +} + +/// Hardcoded attester secret keys. +fn attester_keys() -> Vec { + [ + "attester:secret:secp256k1:27cb45b1670a1ae8d376a85821d51c7f91ebc6e32788027a84758441aaf0a987", + "attester:secret:secp256k1:20132edc08a529e927f155e710ae7295a2a0d249f1b1f37726894d1d0d8f0d81", + "attester:secret:secp256k1:0946901f0a6650284726763b12de5da0f06df0016c8ec2144cf6b1903f1979a6", + ] + .iter() + .map(|raw| Text::new(raw).decode().unwrap()) + .collect() +} + +/// Hardcoded validator committee. +fn validator_committee() -> Committee { + Committee::new( + validator_keys() + .iter() + .enumerate() + .map(|(i, key)| WeightedValidator { + key: key.public(), + weight: i as u64 + 10, + }), + ) + .unwrap() +} + +/// Hardcoded attester committee. +fn attester_committee() -> attester::Committee { + attester::Committee::new( + attester_keys() + .iter() + .enumerate() + .map(|(i, key)| WeightedAttester { + key: key.public(), + weight: i as u64 + 10, + }), + ) + .unwrap() +} + +/// Hardcoded genesis with no attesters. +fn genesis_empty_attesters() -> Genesis { + GenesisRaw { + chain_id: ChainId(1337), + fork_number: ForkNumber(42), + first_block: BlockNumber(2834), + + protocol_version: ProtocolVersion(1), + validators: validator_committee(), + attesters: None, + leader_selection: LeaderSelectionMode::Weighted, + } + .with_hash() +} + +/// Hardcoded genesis with attesters. +fn genesis_with_attesters() -> Genesis { + GenesisRaw { + chain_id: ChainId(1337), + fork_number: ForkNumber(42), + first_block: BlockNumber(2834), + + protocol_version: ProtocolVersion(1), + validators: validator_committee(), + attesters: attester_committee().into(), + leader_selection: LeaderSelectionMode::Weighted, + } + .with_hash() +} + +/// Hardcoded `LeaderProposal`. +fn leader_proposal() -> LeaderProposal { + LeaderProposal { + proposal: block_header(), + proposal_payload: Some(payload()), + justification: ProposalJustification::Timeout(timeout_qc()), + } +} + +/// Hardcoded `ReplicaCommit`. +fn replica_commit() -> ReplicaCommit { + ReplicaCommit { + view: view(), + proposal: block_header(), + } +} + +/// Hardcoded `CommitQC`. +fn commit_qc() -> CommitQC { + let genesis = genesis_empty_attesters(); + let replica_commit = replica_commit(); + let mut x = CommitQC::new(replica_commit.clone(), &genesis); + for k in validator_keys() { + x.add(&k.sign_msg(replica_commit.clone()), &genesis) + .unwrap(); + } + x +} + +/// Hardcoded `ReplicaTimeout` +fn replica_timeout() -> ReplicaTimeout { + ReplicaTimeout { + view: View { + genesis: genesis_empty_attesters().hash(), + number: ViewNumber(9169), + }, + high_vote: Some(replica_commit()), + high_qc: Some(commit_qc()), + } +} + +/// Hardcoded `TimeoutQC`. +fn timeout_qc() -> TimeoutQC { + let mut x = TimeoutQC::new(View { + genesis: genesis_empty_attesters().hash(), + number: ViewNumber(9169), + }); + let genesis = genesis_empty_attesters(); + let replica_timeout = replica_timeout(); + for k in validator_keys() { + x.add(&k.sign_msg(replica_timeout.clone()), &genesis) + .unwrap(); + } + x +} + +/// Hardcoded `ReplicaNewView`. +fn replica_new_view() -> ReplicaNewView { + ReplicaNewView { + justification: ProposalJustification::Commit(commit_qc()), + } +} diff --git a/node/libs/roles/src/validator/messages/tests/replica_commit.rs b/node/libs/roles/src/validator/messages/tests/replica_commit.rs new file mode 100644 index 000000000..06516ba57 --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/replica_commit.rs @@ -0,0 +1,95 @@ +use super::*; +use assert_matches::assert_matches; +use zksync_concurrency::ctx; + +#[test] +fn test_commit_qc() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + // This will create equally weighted validators + let setup1 = Setup::new(rng, 6); + let setup2 = Setup::new(rng, 6); + let mut genesis3 = (*setup1.genesis).clone(); + genesis3.validators = + Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); + let genesis3 = genesis3.with_hash(); + + for i in 0..setup1.validator_keys.len() + 1 { + let view = rng.gen(); + let mut qc = CommitQC::new(setup1.make_replica_commit(rng, view), &setup1.genesis); + for key in &setup1.validator_keys[0..i] { + qc.add(&key.sign_msg(qc.message.clone()), &setup1.genesis) + .unwrap(); + } + let expected_weight: u64 = setup1 + .genesis + .validators + .iter() + .take(i) + .map(|w| w.weight) + .sum(); + if expected_weight >= setup1.genesis.validators.quorum_threshold() { + qc.verify(&setup1.genesis).unwrap(); + } else { + assert_matches!( + qc.verify(&setup1.genesis), + Err(CommitQCVerifyError::NotEnoughSigners { .. }) + ); + } + + // Mismatching validator sets. + assert!(qc.verify(&setup2.genesis).is_err()); + assert!(qc.verify(&genesis3).is_err()); + } +} + +#[test] +fn test_commit_qc_add_errors() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let setup = Setup::new(rng, 2); + let view = rng.gen(); + let mut qc = CommitQC::new(setup.make_replica_commit(rng, view), &setup.genesis); + let msg = qc.message.clone(); + // Add the message + assert_matches!( + qc.add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ), + Ok(()) + ); + + // Try to add a message for a different view + let mut msg1 = msg.clone(); + msg1.view.number = view.next(); + assert_matches!( + qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), + Err(CommitQCAddError::InconsistentMessages { .. }) + ); + + // Try to add a message from a signer not in committee + assert_matches!( + qc.add( + &rng.gen::().sign_msg(msg.clone()), + &setup.genesis + ), + Err(CommitQCAddError::SignerNotInCommittee { .. }) + ); + + // Try to add the same message already added by same validator + assert_matches!( + qc.add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ), + Err(CommitQCAddError::DuplicateSigner { .. }) + ); + + // Add same message signed by another validator. + assert_matches!( + qc.add(&setup.validator_keys[1].sign_msg(msg), &setup.genesis), + Ok(()) + ); +} diff --git a/node/libs/roles/src/validator/messages/tests/replica_timeout.rs b/node/libs/roles/src/validator/messages/tests/replica_timeout.rs new file mode 100644 index 000000000..f69b08a68 --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/replica_timeout.rs @@ -0,0 +1,174 @@ +use super::*; +use assert_matches::assert_matches; +use rand::seq::SliceRandom as _; +use zksync_concurrency::ctx; + +#[test] +fn test_timeout_qc() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + // This will create equally weighted validators + let setup1 = Setup::new(rng, 6); + let setup2 = Setup::new(rng, 6); + let mut genesis3 = (*setup1.genesis).clone(); + genesis3.validators = + Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); + let genesis3 = genesis3.with_hash(); + + let view: ViewNumber = rng.gen(); + let msgs: Vec<_> = (0..3) + .map(|_| setup1.make_replica_timeout(rng, view)) + .collect(); + + for n in 0..setup1.validator_keys.len() + 1 { + let mut qc = TimeoutQC::new(msgs[0].view.clone()); + for key in &setup1.validator_keys[0..n] { + qc.add( + &key.sign_msg(msgs.choose(rng).unwrap().clone()), + &setup1.genesis, + ) + .unwrap(); + } + let expected_weight: u64 = setup1 + .genesis + .validators + .iter() + .take(n) + .map(|w| w.weight) + .sum(); + if expected_weight >= setup1.genesis.validators.quorum_threshold() { + qc.verify(&setup1.genesis).unwrap(); + } else { + assert_matches!( + qc.verify(&setup1.genesis), + Err(TimeoutQCVerifyError::NotEnoughSigners { .. }) + ); + } + + // Mismatching validator sets. + assert!(qc.verify(&setup2.genesis).is_err()); + assert!(qc.verify(&genesis3).is_err()); + } +} + +#[test] +fn test_timeout_qc_high_vote() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + + // This will create equally weighted validators + let setup = Setup::new(rng, 6); + + let view_num: ViewNumber = rng.gen(); + let msg_a = setup.make_replica_timeout(rng, view_num); + let msg_b = setup.make_replica_timeout(rng, view_num); + let msg_c = setup.make_replica_timeout(rng, view_num); + + // Case with 1 subquorum. + let mut qc = TimeoutQC::new(msg_a.view.clone()); + + for key in &setup.validator_keys { + qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) + .unwrap(); + } + + assert!(qc.high_vote(&setup.genesis).is_some()); + + // Case with 2 subquorums. + let mut qc = TimeoutQC::new(msg_a.view.clone()); + + for key in &setup.validator_keys[0..3] { + qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) + .unwrap(); + } + + for key in &setup.validator_keys[3..6] { + qc.add(&key.sign_msg(msg_b.clone()), &setup.genesis) + .unwrap(); + } + + assert!(qc.high_vote(&setup.genesis).is_none()); + + // Case with no subquorums. + let mut qc = TimeoutQC::new(msg_a.view.clone()); + + for key in &setup.validator_keys[0..2] { + qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) + .unwrap(); + } + + for key in &setup.validator_keys[2..4] { + qc.add(&key.sign_msg(msg_b.clone()), &setup.genesis) + .unwrap(); + } + + for key in &setup.validator_keys[4..6] { + qc.add(&key.sign_msg(msg_c.clone()), &setup.genesis) + .unwrap(); + } + + assert!(qc.high_vote(&setup.genesis).is_none()); +} + +#[test] +fn test_timeout_qc_add_errors() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let setup = Setup::new(rng, 2); + let view = rng.gen(); + let msg = setup.make_replica_timeout(rng, view); + let mut qc = TimeoutQC::new(msg.view.clone()); + let msg = setup.make_replica_timeout(rng, view); + + // Add the message + assert_matches!( + qc.add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ), + Ok(()) + ); + + // Try to add a message for a different view + let mut msg1 = msg.clone(); + msg1.view.number = view.next(); + assert_matches!( + qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), + Err(TimeoutQCAddError::InconsistentViews { .. }) + ); + + // Try to add a message from a signer not in committee + assert_matches!( + qc.add( + &rng.gen::().sign_msg(msg.clone()), + &setup.genesis + ), + Err(TimeoutQCAddError::SignerNotInCommittee { .. }) + ); + + // Try to add the same message already added by same validator + assert_matches!( + qc.add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ), + Err(TimeoutQCAddError::DuplicateSigner { .. }) + ); + + // Try to add a message for a validator that already added another message + let msg2 = setup.make_replica_timeout(rng, view); + assert_matches!( + qc.add(&setup.validator_keys[0].sign_msg(msg2), &setup.genesis), + Err(TimeoutQCAddError::DuplicateSigner { .. }) + ); + + // Add same message signed by another validator. + assert_matches!( + qc.add( + &setup.validator_keys[1].sign_msg(msg.clone()), + &setup.genesis + ), + Ok(()) + ); +} diff --git a/node/libs/roles/src/validator/messages/tests/versions.rs b/node/libs/roles/src/validator/messages/tests/versions.rs new file mode 100644 index 000000000..1fa2c46db --- /dev/null +++ b/node/libs/roles/src/validator/messages/tests/versions.rs @@ -0,0 +1,102 @@ +use super::*; +use anyhow::Context as _; +use zksync_consensus_crypto::Text; + +mod version1 { + use zksync_consensus_utils::enum_util::Variant as _; + + use super::*; + + /// Note that genesis is NOT versioned by ProtocolVersion. + /// Even if it was, ALL versions of genesis need to be supported FOREVER, + /// unless we introduce dynamic regenesis. + #[test] + fn genesis_hash_change_detector_empty_attesters() { + let want: GenesisHash = Text::new( + "genesis_hash:keccak256:75cfa582fcda9b5da37af8fb63a279f777bb17a97a50519e1a61aad6c77a522f", + ) + .decode() + .unwrap(); + assert_eq!(want, genesis_empty_attesters().hash()); + } + + /// Note that genesis is NOT versioned by ProtocolVersion. + /// Even if it was, ALL versions of genesis need to be supported FOREVER, + /// unless we introduce dynamic regenesis. + #[test] + fn genesis_hash_change_detector_nonempty_attesters() { + let want: GenesisHash = Text::new( + "genesis_hash:keccak256:586a4bc6167c084d7499cead9267b224ab04a4fdeff555630418bcd2df5d186d", + ) + .decode() + .unwrap(); + assert_eq!(want, genesis_with_attesters().hash()); + } + + /// Asserts that msg.hash()==hash and that sig is a + /// valid signature of msg (signed by `keys()[0]`). + #[track_caller] + fn msg_change_detector(msg: Msg, hash: &str, sig: &str) { + let key = validator_keys()[0].clone(); + + (|| { + // Decode hash and signature. + let hash: MsgHash = Text::new(hash).decode()?; + let sig: validator::Signature = Text::new(sig).decode()?; + + // Check if msg.hash() is equal to hash. + if msg.hash() != hash { + return Err(anyhow::anyhow!("Hash mismatch")); + } + + // Check if sig is a valid signature of hash. + sig.verify_hash(&hash, &key.public())?; + + anyhow::Ok(()) + })() + .with_context(|| { + format!( + "\nIntended hash: {:?}\nIntended signature: {:?}", + msg.hash(), + key.sign_hash(&msg.hash()), + ) + }) + .unwrap(); + } + + #[test] + fn replica_commit_change_detector() { + msg_change_detector( + replica_commit().insert(), + "validator_msg:keccak256:ccbb11a6b3f4e06840a2a06abc2a245a2b3de30bb951e759a9ec6920f74f0632", + "validator:signature:bls12_381:8e41b89c89c0de8f83102966596ab95f6bdfdc18fceaceb224753b3ff495e02d5479c709829bd6d0802c5a1f24fa96b5", + ); + } + + #[test] + fn replica_new_view_change_detector() { + msg_change_detector( + replica_new_view().insert(), + "validator_msg:keccak256:2be143114cd3442b96d5f6083713c4c338a1c18ef562ede4721ebf037689a6ad", + "validator:signature:bls12_381:9809b66d44509cf7847baaa03a35ae87062f9827cf1f90c8353f057eee45b79fde0f4c4c500980b69c59263b51b6d072", + ); + } + + #[test] + fn replica_timeout_change_detector() { + msg_change_detector( + replica_timeout().insert(), + "validator_msg:keccak256:615fa6d2960b48e30ab88fe195bbad161b8a6f9a59a45ca86b5e2f20593f76cd", + "validator:signature:bls12_381:ac9b6d340bf1b04421455676b8a28a8de079cd9b40f75f1009aa3da32981690bc520d4ec0284ae030fc8b036d86ca307", + ); + } + + #[test] + fn leader_proposal_change_detector() { + msg_change_detector( + leader_proposal().insert(), + "validator_msg:keccak256:7b079e4ca3021834fa35745cb042fea6dd5bb89a91ca5ba31ed6ba1765a1e113", + "validator:signature:bls12_381:98ca0f24d87f938b22ac9c2a2720466cd157a502b31ae5627ce5fdbda6de0ad6d2e9b159cf816cd1583644f2f69ecb84", + ); + } +} diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index c9800a4f1..b88768d9a 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/node/libs/roles/src/validator/testonly.rs @@ -38,10 +38,6 @@ pub struct SetupSpec { pub leader_selection: LeaderSelectionMode, } -/// Test setup. -#[derive(Debug, Clone)] -pub struct Setup(SetupInner); - impl SetupSpec { /// New `SetupSpec`. pub fn new(rng: &mut impl Rng, validators: usize) -> Self { @@ -68,6 +64,10 @@ impl SetupSpec { } } +/// Test setup. +#[derive(Debug, Clone)] +pub struct Setup(SetupInner); + impl Setup { /// New `Setup`. pub fn new(rng: &mut impl Rng, validators: usize) -> Self { @@ -81,6 +81,51 @@ impl Setup { Self::from_spec(rng, spec) } + /// Generates a new `Setup` from the given `SetupSpec`. + pub fn from_spec(rng: &mut impl Rng, spec: SetupSpec) -> Self { + let mut this = Self(SetupInner { + genesis: GenesisRaw { + chain_id: spec.chain_id, + fork_number: spec.fork_number, + first_block: spec.first_block, + + protocol_version: spec.protocol_version, + validators: Committee::new(spec.validator_weights.iter().map(|(k, w)| { + WeightedValidator { + key: k.public(), + weight: *w, + } + })) + .unwrap(), + attesters: attester::Committee::new(spec.attester_weights.iter().map(|(k, w)| { + attester::WeightedAttester { + key: k.public(), + weight: *w, + } + })) + .unwrap() + .into(), + leader_selection: spec.leader_selection, + } + .with_hash(), + validator_keys: spec.validator_weights.into_iter().map(|(k, _)| k).collect(), + attester_keys: spec.attester_weights.into_iter().map(|(k, _)| k).collect(), + blocks: vec![], + }); + // Populate pregenesis blocks. + for block in spec.first_pregenesis_block.0..spec.first_block.0 { + this.0.blocks.push( + PreGenesisBlock { + number: BlockNumber(block), + payload: rng.gen(), + justification: rng.gen(), + } + .into(), + ); + } + this + } + /// Next block to finalize. pub fn next(&self) -> BlockNumber { match self.0.blocks.last() { @@ -144,52 +189,107 @@ impl Setup { let first = self.0.blocks.first()?.number(); self.0.blocks.get(n.0.checked_sub(first.0)? as usize) } -} -impl Setup { - /// Generates a new `Setup` from the given `SetupSpec`. - pub fn from_spec(rng: &mut impl Rng, spec: SetupSpec) -> Self { - let mut this = Self(SetupInner { - genesis: GenesisRaw { - chain_id: spec.chain_id, - fork_number: spec.fork_number, - first_block: spec.first_block, + /// Creates a View with the given view number. + pub fn make_view(&self, number: ViewNumber) -> View { + View { + genesis: self.genesis.hash(), + number, + } + } - protocol_version: spec.protocol_version, - validators: Committee::new(spec.validator_weights.iter().map(|(k, w)| { - WeightedValidator { - key: k.public(), - weight: *w, - } - })) - .unwrap(), - attesters: attester::Committee::new(spec.attester_weights.iter().map(|(k, w)| { - attester::WeightedAttester { - key: k.public(), - weight: *w, - } - })) - .unwrap() - .into(), - leader_selection: spec.leader_selection, + /// Creates a ReplicaCommt with a random payload. + pub fn make_replica_commit(&self, rng: &mut impl Rng, view: ViewNumber) -> ReplicaCommit { + ReplicaCommit { + view: self.make_view(view), + proposal: BlockHeader { + number: self.next(), + payload: rng.gen(), + }, + } + } + + /// Creates a ReplicaCommt with the given payload. + pub fn make_replica_commit_with_payload( + &self, + payload: &Payload, + view: ViewNumber, + ) -> ReplicaCommit { + ReplicaCommit { + view: self.make_view(view), + proposal: BlockHeader { + number: self.next(), + payload: payload.hash(), + }, + } + } + + /// Creates a CommitQC with a random payload. + pub fn make_commit_qc(&self, rng: &mut impl Rng, view: ViewNumber) -> CommitQC { + let mut qc = CommitQC::new(self.make_replica_commit(rng, view), &self.genesis); + for key in &self.validator_keys { + qc.add(&key.sign_msg(qc.message.clone()), &self.genesis) + .unwrap(); + } + qc + } + + /// Creates a CommitQC with the given payload. + pub fn make_commit_qc_with_payload(&self, payload: &Payload, view: ViewNumber) -> CommitQC { + let mut qc = CommitQC::new( + self.make_replica_commit_with_payload(payload, view), + &self.genesis, + ); + for key in &self.validator_keys { + qc.add(&key.sign_msg(qc.message.clone()), &self.genesis) + .unwrap(); + } + qc + } + + /// Creates a ReplicaTimeout with a random payload. + pub fn make_replica_timeout(&self, rng: &mut impl Rng, view: ViewNumber) -> ReplicaTimeout { + let high_vote_view = ViewNumber(rng.gen_range(0..view.0)); + let high_qc_view = ViewNumber(rng.gen_range(0..high_vote_view.0)); + ReplicaTimeout { + view: self.make_view(view), + high_vote: Some(self.make_replica_commit(rng, high_vote_view)), + high_qc: Some(self.make_commit_qc(rng, high_qc_view)), + } + } + + /// Creates a TimeoutQC. If a payload is given, the QC will contain a + /// re-proposal for that payload + pub fn make_timeout_qc( + &self, + rng: &mut impl Rng, + view: ViewNumber, + payload_opt: Option<&Payload>, + ) -> TimeoutQC { + let mut vote = if let Some(payload) = payload_opt { + self.make_replica_commit_with_payload(payload, view.prev().unwrap()) + } else { + self.make_replica_commit(rng, view.prev().unwrap()) + }; + let commit_qc = match self.0.blocks.last().unwrap() { + Block::Final(block) => block.justification.clone(), + _ => unreachable!(), + }; + + let mut qc = TimeoutQC::new(self.make_view(view)); + for key in &self.validator_keys { + if payload_opt.is_none() { + vote.proposal.payload = rng.gen(); } - .with_hash(), - validator_keys: spec.validator_weights.into_iter().map(|(k, _)| k).collect(), - attester_keys: spec.attester_weights.into_iter().map(|(k, _)| k).collect(), - blocks: vec![], - }); - // Populate pregenesis blocks. - for block in spec.first_pregenesis_block.0..spec.first_block.0 { - this.0.blocks.push( - PreGenesisBlock { - number: BlockNumber(block), - payload: rng.gen(), - justification: rng.gen(), - } - .into(), - ); + let msg = ReplicaTimeout { + view: self.make_view(view), + high_vote: Some(vote.clone()), + high_qc: Some(commit_qc.clone()), + }; + qc.add(&key.sign_msg(msg), &self.genesis).unwrap(); } - this + + qc } } diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index f2616d499..95d0f1904 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -1,11 +1,8 @@ use super::*; -use crate::validator::testonly::Setup; -use assert_matches::assert_matches; -use rand::{seq::SliceRandom, Rng}; -use std::vec; +use rand::Rng; use zksync_concurrency::ctx; use zksync_consensus_crypto::{ByteFmt, Text, TextFmt}; -use zksync_protobuf::{testonly::test_encode_random, ProtoFmt}; +use zksync_protobuf::testonly::test_encode_random; #[test] fn test_byte_encoding() { @@ -102,432 +99,3 @@ fn test_schema_encoding() { test_encode_random::(rng); test_encode_random::(rng); } - -#[test] -fn test_genesis_verify() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - let genesis = Setup::new(rng, 1).genesis.clone(); - assert!(genesis.verify().is_ok()); - assert!(Genesis::read(&genesis.build()).is_ok()); - - let mut genesis = (*genesis).clone(); - genesis.leader_selection = LeaderSelectionMode::Sticky(rng.gen()); - let genesis = genesis.with_hash(); - assert!(genesis.verify().is_err()); - assert!(Genesis::read(&genesis.build()).is_err()) -} - -#[test] -fn test_signature_verify() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - let msg1: MsgHash = rng.gen(); - let msg2: MsgHash = rng.gen(); - - let key1: SecretKey = rng.gen(); - let key2: SecretKey = rng.gen(); - - let sig1 = key1.sign_hash(&msg1); - - // Matching key and message. - sig1.verify_hash(&msg1, &key1.public()).unwrap(); - - // Mismatching message. - assert!(sig1.verify_hash(&msg2, &key1.public()).is_err()); - - // Mismatching key. - assert!(sig1.verify_hash(&msg1, &key2.public()).is_err()); -} - -#[test] -fn test_agg_signature_verify() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - let msg1: MsgHash = rng.gen(); - let msg2: MsgHash = rng.gen(); - - let key1: SecretKey = rng.gen(); - let key2: SecretKey = rng.gen(); - - let sig1 = key1.sign_hash(&msg1); - let sig2 = key2.sign_hash(&msg2); - - let agg_sig = AggregateSignature::aggregate(vec![&sig1, &sig2]); - - // Matching key and message. - agg_sig - .verify_hash([(msg1, &key1.public()), (msg2, &key2.public())].into_iter()) - .unwrap(); - - // Mismatching message. - assert!(agg_sig - .verify_hash([(msg2, &key1.public()), (msg1, &key2.public())].into_iter()) - .is_err()); - - // Mismatching key. - assert!(agg_sig - .verify_hash([(msg1, &key2.public()), (msg2, &key1.public())].into_iter()) - .is_err()); -} - -fn make_view(number: ViewNumber, setup: &Setup) -> View { - View { - genesis: setup.genesis.hash(), - number, - } -} - -fn make_replica_commit(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> ReplicaCommit { - ReplicaCommit { - view: make_view(view, setup), - proposal: rng.gen(), - } -} - -fn make_commit_qc(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> CommitQC { - let mut qc = CommitQC::new(make_replica_commit(rng, view, setup), &setup.genesis); - for key in &setup.validator_keys { - qc.add(&key.sign_msg(qc.message.clone()), &setup.genesis) - .unwrap(); - } - qc -} - -fn make_replica_timeout(rng: &mut impl Rng, view: ViewNumber, setup: &Setup) -> ReplicaTimeout { - ReplicaTimeout { - view: make_view(view, setup), - high_vote: { - let view = ViewNumber(rng.gen_range(0..view.0)); - Some(make_replica_commit(rng, view, setup)) - }, - high_qc: { - let view = ViewNumber(rng.gen_range(0..view.0)); - Some(make_commit_qc(rng, view, setup)) - }, - } -} - -#[test] -fn test_commit_qc() { - use CommitQCVerifyError as Error; - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - // This will create equally weighted validators - let setup1 = Setup::new(rng, 6); - let setup2 = Setup::new(rng, 6); - let mut genesis3 = (*setup1.genesis).clone(); - genesis3.validators = - Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); - let genesis3 = genesis3.with_hash(); - - for i in 0..setup1.validator_keys.len() + 1 { - let view = rng.gen(); - let mut qc = CommitQC::new(make_replica_commit(rng, view, &setup1), &setup1.genesis); - for key in &setup1.validator_keys[0..i] { - qc.add(&key.sign_msg(qc.message.clone()), &setup1.genesis) - .unwrap(); - } - let expected_weight: u64 = setup1 - .genesis - .validators - .iter() - .take(i) - .map(|w| w.weight) - .sum(); - if expected_weight >= setup1.genesis.validators.quorum_threshold() { - qc.verify(&setup1.genesis).unwrap(); - } else { - assert_matches!( - qc.verify(&setup1.genesis), - Err(Error::NotEnoughSigners { .. }) - ); - } - - // Mismatching validator sets. - assert!(qc.verify(&setup2.genesis).is_err()); - assert!(qc.verify(&genesis3).is_err()); - } -} - -#[test] -fn test_commit_qc_add_errors() { - use CommitQCAddError as Error; - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - let setup = Setup::new(rng, 2); - let view = rng.gen(); - let mut qc = CommitQC::new(make_replica_commit(rng, view, &setup), &setup.genesis); - let msg = qc.message.clone(); - // Add the message - assert_matches!( - qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), - &setup.genesis - ), - Ok(()) - ); - - // Try to add a message for a different view - let mut msg1 = msg.clone(); - msg1.view.number = view.next(); - assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), - Err(Error::InconsistentMessages { .. }) - ); - - // Try to add a message from a signer not in committee - assert_matches!( - qc.add( - &rng.gen::().sign_msg(msg.clone()), - &setup.genesis - ), - Err(Error::SignerNotInCommittee { .. }) - ); - - // Try to add the same message already added by same validator - assert_matches!( - qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), - &setup.genesis - ), - Err(Error::DuplicateSigner { .. }) - ); - - // Add same message signed by another validator. - assert_matches!( - qc.add(&setup.validator_keys[1].sign_msg(msg), &setup.genesis), - Ok(()) - ); -} - -#[test] -fn test_timeout_qc() { - use TimeoutQCVerifyError as Error; - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - // This will create equally weighted validators - let setup1 = Setup::new(rng, 6); - let setup2 = Setup::new(rng, 6); - let mut genesis3 = (*setup1.genesis).clone(); - genesis3.validators = - Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); - let genesis3 = genesis3.with_hash(); - - let view: ViewNumber = rng.gen(); - let msgs: Vec<_> = (0..3) - .map(|_| make_replica_timeout(rng, view, &setup1)) - .collect(); - - for n in 0..setup1.validator_keys.len() + 1 { - let mut qc = TimeoutQC::new(msgs[0].view.clone()); - for key in &setup1.validator_keys[0..n] { - qc.add( - &key.sign_msg(msgs.choose(rng).unwrap().clone()), - &setup1.genesis, - ) - .unwrap(); - } - let expected_weight: u64 = setup1 - .genesis - .validators - .iter() - .take(n) - .map(|w| w.weight) - .sum(); - if expected_weight >= setup1.genesis.validators.quorum_threshold() { - qc.verify(&setup1.genesis).unwrap(); - } else { - assert_matches!( - qc.verify(&setup1.genesis), - Err(Error::NotEnoughSigners { .. }) - ); - } - - // Mismatching validator sets. - assert!(qc.verify(&setup2.genesis).is_err()); - assert!(qc.verify(&genesis3).is_err()); - } -} - -#[test] -fn test_prepare_qc_high_vote() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - // This will create equally weighted validators - let setup = Setup::new(rng, 6); - - let view_num: ViewNumber = rng.gen(); - let msg_a = make_replica_timeout(rng, view_num, &setup); - let msg_b = make_replica_timeout(rng, view_num, &setup); - let msg_c = make_replica_timeout(rng, view_num, &setup); - - // Case with 1 subquorum. - let mut qc = TimeoutQC::new(msg_a.view.clone()); - - for key in &setup.validator_keys { - qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) - .unwrap(); - } - - assert!(qc.high_vote(&setup.genesis).is_some()); - - // Case with 2 subquorums. - let mut qc = TimeoutQC::new(msg_a.view.clone()); - - for key in &setup.validator_keys[0..3] { - qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) - .unwrap(); - } - - for key in &setup.validator_keys[3..6] { - qc.add(&key.sign_msg(msg_b.clone()), &setup.genesis) - .unwrap(); - } - - assert!(qc.high_vote(&setup.genesis).is_none()); - - // Case with no subquorums. - let mut qc = TimeoutQC::new(msg_a.view.clone()); - - for key in &setup.validator_keys[0..2] { - qc.add(&key.sign_msg(msg_a.clone()), &setup.genesis) - .unwrap(); - } - - for key in &setup.validator_keys[2..4] { - qc.add(&key.sign_msg(msg_b.clone()), &setup.genesis) - .unwrap(); - } - - for key in &setup.validator_keys[4..6] { - qc.add(&key.sign_msg(msg_c.clone()), &setup.genesis) - .unwrap(); - } - - assert!(qc.high_vote(&setup.genesis).is_none()); -} - -#[test] -fn test_prepare_qc_add_errors() { - use TimeoutQCAddError as Error; - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - let setup = Setup::new(rng, 2); - let view = rng.gen(); - let msg = make_replica_timeout(rng, view, &setup); - let mut qc = TimeoutQC::new(msg.view.clone()); - let msg = make_replica_timeout(rng, view, &setup); - - // Add the message - assert_matches!( - qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), - &setup.genesis - ), - Ok(()) - ); - - // Try to add a message for a different view - let mut msg1 = msg.clone(); - msg1.view.number = view.next(); - assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), - Err(Error::InconsistentViews { .. }) - ); - - // Try to add a message from a signer not in committee - assert_matches!( - qc.add( - &rng.gen::().sign_msg(msg.clone()), - &setup.genesis - ), - Err(Error::SignerNotInCommittee { .. }) - ); - - // Try to add the same message already added by same validator - assert_matches!( - qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), - &setup.genesis - ), - Err(Error::DuplicateSigner { .. }) - ); - - // Try to add a message for a validator that already added another message - let msg2 = make_replica_timeout(rng, view, &setup); - assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg2), &setup.genesis), - Err(Error::DuplicateSigner { .. }) - ); - - // Add same message signed by another validator. - assert_matches!( - qc.add( - &setup.validator_keys[1].sign_msg(msg.clone()), - &setup.genesis - ), - Ok(()) - ); -} - -#[test] -fn test_validator_committee_weights() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - // Validators with non-uniform weights - let setup = Setup::new_with_weights(rng, vec![1000, 600, 800, 6000, 900, 700]); - // Expected sum of the validators weights - let sums = [1000, 1600, 2400, 8400, 9300, 10000]; - - let view: ViewNumber = rng.gen(); - let msg = make_replica_timeout(rng, view, &setup); - let mut qc = TimeoutQC::new(msg.view.clone()); - for (n, weight) in sums.iter().enumerate() { - let key = &setup.validator_keys[n]; - qc.add(&key.sign_msg(msg.clone()), &setup.genesis).unwrap(); - let signers = &qc.map[&msg]; - assert_eq!(setup.genesis.validators.weight(signers), *weight); - } -} - -#[test] -fn test_committee_weights_overflow_check() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - let validators: Vec = [u64::MAX / 5; 6] - .iter() - .map(|w| WeightedValidator { - key: rng.gen::().public(), - weight: *w, - }) - .collect(); - - // Creation should overflow - assert_matches!(Committee::new(validators), Err(_)); -} - -#[test] -fn test_committee_with_zero_weights() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); - - let validators: Vec = [1000, 0, 800, 6000, 0, 700] - .iter() - .map(|w| WeightedValidator { - key: rng.gen::().public(), - weight: *w, - }) - .collect(); - - // Committee creation should error on zero weight validators - assert_matches!(Committee::new(validators), Err(_)); -} From d5f85311a42789f585168a1a2703274b5ad00ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Mon, 21 Oct 2024 02:35:34 +0100 Subject: [PATCH 09/10] last tests --- .../roles/src/validator/messages/genesis.rs | 2 +- .../src/validator/messages/replica_commit.rs | 11 +- .../src/validator/messages/replica_timeout.rs | 72 ++--- .../messages/tests/leader_proposal.rs | 47 +++ .../messages/tests/replica_commit.rs | 152 +++++---- .../messages/tests/replica_timeout.rs | 298 ++++++++++++++---- node/libs/roles/src/validator/testonly.rs | 20 +- spec/informal-spec/types.rs | 6 +- 8 files changed, 420 insertions(+), 188 deletions(-) diff --git a/node/libs/roles/src/validator/messages/genesis.rs b/node/libs/roles/src/validator/messages/genesis.rs index 9af95411c..9f1264097 100644 --- a/node/libs/roles/src/validator/messages/genesis.rs +++ b/node/libs/roles/src/validator/messages/genesis.rs @@ -59,7 +59,7 @@ impl fmt::Debug for GenesisHash { /// Genesis with cached hash. #[derive(Clone)] -pub struct Genesis(GenesisRaw, GenesisHash); +pub struct Genesis(pub(crate) GenesisRaw, pub(crate) GenesisHash); impl std::ops::Deref for Genesis { type Target = GenesisRaw; diff --git a/node/libs/roles/src/validator/messages/replica_commit.rs b/node/libs/roles/src/validator/messages/replica_commit.rs index 76f86606d..13563a84c 100644 --- a/node/libs/roles/src/validator/messages/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/replica_commit.rs @@ -17,10 +17,6 @@ impl ReplicaCommit { .verify(genesis) .map_err(ReplicaCommitVerifyError::BadView)?; - if self.proposal.number < genesis.first_block { - return Err(ReplicaCommitVerifyError::BadBlockNumber); - } - Ok(()) } } @@ -31,9 +27,6 @@ pub enum ReplicaCommitVerifyError { /// Invalid view. #[error("view: {0:#}")] BadView(anyhow::Error), - /// Bad block number. - #[error("block number < first block")] - BadBlockNumber, } /// A Commit Quorum Certificate. It is an aggregate of signed ReplicaCommit messages. @@ -124,7 +117,7 @@ impl CommitQC { let weight = genesis.validators.weight(&self.signers); let threshold = genesis.validators.quorum_threshold(); if weight < threshold { - return Err(CommitQCVerifyError::NotEnoughSigners { + return Err(CommitQCVerifyError::NotEnoughWeight { got: weight, want: threshold, }); @@ -181,7 +174,7 @@ pub enum CommitQCVerifyError { BadSignersSet, /// Weight not reached. #[error("Signers have not reached threshold weight: got {got}, want {want}")] - NotEnoughSigners { + NotEnoughWeight { /// Got weight. got: u64, /// Want weight. diff --git a/node/libs/roles/src/validator/messages/replica_timeout.rs b/node/libs/roles/src/validator/messages/replica_timeout.rs index 02f4b4481..5851a0c0d 100644 --- a/node/libs/roles/src/validator/messages/replica_timeout.rs +++ b/node/libs/roles/src/validator/messages/replica_timeout.rs @@ -21,22 +21,16 @@ impl ReplicaTimeout { pub fn verify(&self, genesis: &Genesis) -> Result<(), ReplicaTimeoutVerifyError> { self.view .verify(genesis) - .map_err(ReplicaTimeoutVerifyError::View)?; + .map_err(ReplicaTimeoutVerifyError::BadView)?; if let Some(v) = &self.high_vote { - if self.view.number <= v.view.number { - return Err(ReplicaTimeoutVerifyError::HighVoteFutureView); - } v.verify(genesis) - .map_err(ReplicaTimeoutVerifyError::HighVote)?; + .map_err(ReplicaTimeoutVerifyError::InvalidHighVote)?; } if let Some(qc) = &self.high_qc { - if self.view.number <= qc.view().number { - return Err(ReplicaTimeoutVerifyError::HighQCFutureView); - } qc.verify(genesis) - .map_err(ReplicaTimeoutVerifyError::HighQC)?; + .map_err(ReplicaTimeoutVerifyError::InvalidHighQC)?; } Ok(()) @@ -48,19 +42,13 @@ impl ReplicaTimeout { pub enum ReplicaTimeoutVerifyError { /// View. #[error("view: {0:#}")] - View(anyhow::Error), - /// FutureHighVoteView. - #[error("high vote from the future")] - HighVoteFutureView, - /// FutureHighQCView. - #[error("high qc from the future")] - HighQCFutureView, - /// HighVote. - #[error("high_vote: {0:#}")] - HighVote(ReplicaCommitVerifyError), - /// HighQC. - #[error("high_qc: {0:#}")] - HighQC(CommitQCVerifyError), + BadView(anyhow::Error), + /// Invalid High Vote. + #[error("invalid high_vote: {0:#}")] + InvalidHighVote(ReplicaCommitVerifyError), + /// Invalid High QC. + #[error("invalid high_qc: {0:#}")] + InvalidHighQC(CommitQCVerifyError), } /// A quorum certificate of ReplicaTimeout messages. Since not all ReplicaTimeout messages are @@ -129,7 +117,7 @@ impl TimeoutQC { }); }; - // Check if already have a message from the same signer. + // Check if we already have a message from the same signer. if self.map.values().any(|s| s.0[i]) { return Err(TimeoutQCAddError::DuplicateSigner { signer: Box::new(msg.key.clone()), @@ -164,29 +152,23 @@ impl TimeoutQC { pub fn verify(&self, genesis: &Genesis) -> Result<(), TimeoutQCVerifyError> { self.view .verify(genesis) - .map_err(TimeoutQCVerifyError::View)?; + .map_err(TimeoutQCVerifyError::BadView)?; let mut sum = Signers::new(genesis.validators.len()); // Check the ReplicaTimeout messages. for (i, (msg, signers)) in self.map.iter().enumerate() { if msg.view != self.view { - return Err(TimeoutQCVerifyError::InconsistentViews); + return Err(TimeoutQCVerifyError::InconsistentView(i)); } if signers.len() != sum.len() { - return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( - "msg[{i}].signers has wrong length" - ))); + return Err(TimeoutQCVerifyError::WrongSignersLength(i)); } if signers.is_empty() { - return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( - "msg[{i}] has no signers assigned" - ))); + return Err(TimeoutQCVerifyError::NoSignersAssigned(i)); } if !(&sum & signers).is_empty() { - return Err(TimeoutQCVerifyError::BadFormat(anyhow::format_err!( - "overlapping signature sets for different messages" - ))); + return Err(TimeoutQCVerifyError::OverlappingSignatureSet(i)); } msg.verify(genesis) .map_err(|err| TimeoutQCVerifyError::InvalidMessage(i, err))?; @@ -198,7 +180,7 @@ impl TimeoutQC { let weight = genesis.validators.weight(&sum); let threshold = genesis.validators.quorum_threshold(); if weight < threshold { - return Err(TimeoutQCVerifyError::NotEnoughSigners { + return Err(TimeoutQCVerifyError::NotEnoughWeight { got: weight, want: threshold, }); @@ -261,19 +243,25 @@ pub enum TimeoutQCAddError { pub enum TimeoutQCVerifyError { /// Bad view. #[error("Bad view: {0:#}")] - View(anyhow::Error), + BadView(anyhow::Error), /// Inconsistent views. - #[error("Inconsistent views of signed messages")] - InconsistentViews, + #[error("Message with inconsistent view: number [{0}]")] + InconsistentView(usize), /// Invalid message. #[error("Invalid message: number [{0}], {1:#}")] InvalidMessage(usize, ReplicaTimeoutVerifyError), - /// Bad message format. - #[error(transparent)] - BadFormat(anyhow::Error), + /// Wrong signers length. + #[error("Message with wrong signers length: number [{0}]")] + WrongSignersLength(usize), + /// No signers assigned. + #[error("Message with no signers assigned: number [{0}]")] + NoSignersAssigned(usize), + /// Overlapping signature sets. + #[error("Message with overlapping signature set: number [{0}]")] + OverlappingSignatureSet(usize), /// Weight not reached. #[error("Signers have not reached threshold weight: got {got}, want {want}")] - NotEnoughSigners { + NotEnoughWeight { /// Got weight. got: u64, /// Want weight. diff --git a/node/libs/roles/src/validator/messages/tests/leader_proposal.rs b/node/libs/roles/src/validator/messages/tests/leader_proposal.rs index 3c1fdac37..11b66e635 100644 --- a/node/libs/roles/src/validator/messages/tests/leader_proposal.rs +++ b/node/libs/roles/src/validator/messages/tests/leader_proposal.rs @@ -95,3 +95,50 @@ fn test_leader_proposal_verify() { Err(LeaderProposalVerifyError::NewProposalWhenPreviousNotFinalized) ); } + +#[test] +fn test_justification_get_implied_block() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let mut setup = Setup::new(rng, 6); + setup.push_blocks(rng, 3); + + let payload: Payload = rng.gen(); + let block_header = BlockHeader { + number: setup.next(), + payload: payload.hash(), + }; + + // Justification with a commit QC + let commit_qc = match setup.blocks.last().unwrap() { + Block::Final(block) => block.justification.clone(), + _ => unreachable!(), + }; + let justification = ProposalJustification::Commit(commit_qc); + let proposal = LeaderProposal { + proposal: block_header, + proposal_payload: Some(payload.clone()), + justification, + }; + + let (implied_block_number, implied_payload) = + proposal.justification.get_implied_block(&setup.genesis); + + assert_eq!(implied_block_number, setup.next()); + assert!(implied_payload.is_none()); + + // Justification with a timeout QC + let timeout_qc = setup.make_timeout_qc(rng, ViewNumber(7), Some(&payload)); + let justification = ProposalJustification::Timeout(timeout_qc); + let proposal = LeaderProposal { + proposal: block_header, + proposal_payload: None, + justification, + }; + + let (implied_block_number, implied_payload) = + proposal.justification.get_implied_block(&setup.genesis); + + assert_eq!(implied_block_number, setup.next()); + assert_eq!(implied_payload, Some(payload.hash())); +} diff --git a/node/libs/roles/src/validator/messages/tests/replica_commit.rs b/node/libs/roles/src/validator/messages/tests/replica_commit.rs index 06516ba57..d7b9d3494 100644 --- a/node/libs/roles/src/validator/messages/tests/replica_commit.rs +++ b/node/libs/roles/src/validator/messages/tests/replica_commit.rs @@ -3,88 +3,86 @@ use assert_matches::assert_matches; use zksync_concurrency::ctx; #[test] -fn test_commit_qc() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); +fn test_replica_commit_verify() { + let genesis = genesis_empty_attesters(); + let commit = replica_commit(); + assert!(commit.verify(&genesis).is_ok()); - // This will create equally weighted validators - let setup1 = Setup::new(rng, 6); - let setup2 = Setup::new(rng, 6); - let mut genesis3 = (*setup1.genesis).clone(); - genesis3.validators = - Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); - let genesis3 = genesis3.with_hash(); - - for i in 0..setup1.validator_keys.len() + 1 { - let view = rng.gen(); - let mut qc = CommitQC::new(setup1.make_replica_commit(rng, view), &setup1.genesis); - for key in &setup1.validator_keys[0..i] { - qc.add(&key.sign_msg(qc.message.clone()), &setup1.genesis) - .unwrap(); - } - let expected_weight: u64 = setup1 - .genesis - .validators - .iter() - .take(i) - .map(|w| w.weight) - .sum(); - if expected_weight >= setup1.genesis.validators.quorum_threshold() { - qc.verify(&setup1.genesis).unwrap(); - } else { - assert_matches!( - qc.verify(&setup1.genesis), - Err(CommitQCVerifyError::NotEnoughSigners { .. }) - ); - } - - // Mismatching validator sets. - assert!(qc.verify(&setup2.genesis).is_err()); - assert!(qc.verify(&genesis3).is_err()); - } + // Wrong view + let wrong_genesis = genesis_with_attesters().clone(); + assert_matches!( + commit.verify(&wrong_genesis), + Err(ReplicaCommitVerifyError::BadView(_)) + ); } #[test] -fn test_commit_qc_add_errors() { +fn test_commit_qc_add() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); let setup = Setup::new(rng, 2); let view = rng.gen(); let mut qc = CommitQC::new(setup.make_replica_commit(rng, view), &setup.genesis); let msg = qc.message.clone(); - // Add the message + + // Add the first signature + assert_eq!(qc.signers.count(), 0); + assert!(qc + .add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ) + .is_ok()); + assert_eq!(qc.signers.count(), 1); + + // Try to add a signature from a signer not in committee assert_matches!( qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), + &rng.gen::().sign_msg(msg.clone()), &setup.genesis ), - Ok(()) + Err(CommitQCAddError::SignerNotInCommittee { .. }) ); - // Try to add a message for a different view - let mut msg1 = msg.clone(); - msg1.view.number = view.next(); + // Try to add a signature from the same validator assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), - Err(CommitQCAddError::InconsistentMessages { .. }) + qc.add( + &setup.validator_keys[0].sign_msg(msg.clone()), + &setup.genesis + ), + Err(CommitQCAddError::DuplicateSigner { .. }) ); - // Try to add a message from a signer not in committee + // Try to add an invalid signature assert_matches!( qc.add( - &rng.gen::().sign_msg(msg.clone()), + &Signed { + msg: msg.clone(), + key: setup.validator_keys[1].public(), + sig: rng.gen() + }, &setup.genesis ), - Err(CommitQCAddError::SignerNotInCommittee { .. }) + Err(CommitQCAddError::BadSignature(_)) + ); + + // Try to add a signature for a different message + let mut msg1 = msg.clone(); + msg1.view.number = view.next(); + assert_matches!( + qc.add(&setup.validator_keys[1].sign_msg(msg1), &setup.genesis), + Err(CommitQCAddError::InconsistentMessages) ); - // Try to add the same message already added by same validator + // Try to add an invalid message + let mut wrong_genesis = setup.genesis.clone().0; + wrong_genesis.chain_id = rng.gen(); assert_matches!( qc.add( - &setup.validator_keys[0].sign_msg(msg.clone()), - &setup.genesis + &setup.validator_keys[1].sign_msg(msg.clone()), + &wrong_genesis.with_hash() ), - Err(CommitQCAddError::DuplicateSigner { .. }) + Err(CommitQCAddError::InvalidMessage(_)) ); // Add same message signed by another validator. @@ -92,4 +90,50 @@ fn test_commit_qc_add_errors() { qc.add(&setup.validator_keys[1].sign_msg(msg), &setup.genesis), Ok(()) ); + assert_eq!(qc.signers.count(), 2); +} + +#[test] +fn test_commit_qc_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let setup = Setup::new(rng, 6); + let view = rng.gen(); + let qc = setup.make_commit_qc(rng, view); + + // Verify the QC + assert!(qc.verify(&setup.genesis).is_ok()); + + // QC with bad message + let mut qc1 = qc.clone(); + qc1.message.view.genesis = rng.gen(); + assert_matches!( + qc1.verify(&setup.genesis), + Err(CommitQCVerifyError::InvalidMessage(_)) + ); + + // QC with too many signers + let mut qc2 = qc.clone(); + qc2.signers = Signers::new(setup.genesis.validators.len() + 1); + assert_matches!( + qc2.verify(&setup.genesis), + Err(CommitQCVerifyError::BadSignersSet) + ); + + // QC with not enough weight + let mut qc3 = qc.clone(); + qc3.signers.0.set(0, false); + qc3.signers.0.set(4, false); + assert_matches!( + qc3.verify(&setup.genesis), + Err(CommitQCVerifyError::NotEnoughWeight { .. }) + ); + + // QC with bad signature + let mut qc4 = qc.clone(); + qc4.signature = rng.gen(); + assert_matches!( + qc4.verify(&setup.genesis), + Err(CommitQCVerifyError::BadSignature(_)) + ); } diff --git a/node/libs/roles/src/validator/messages/tests/replica_timeout.rs b/node/libs/roles/src/validator/messages/tests/replica_timeout.rs index f69b08a68..e07221cce 100644 --- a/node/libs/roles/src/validator/messages/tests/replica_timeout.rs +++ b/node/libs/roles/src/validator/messages/tests/replica_timeout.rs @@ -1,55 +1,35 @@ use super::*; use assert_matches::assert_matches; -use rand::seq::SliceRandom as _; use zksync_concurrency::ctx; #[test] -fn test_timeout_qc() { - let ctx = ctx::test_root(&ctx::RealClock); - let rng = &mut ctx.rng(); +fn test_replica_timeout_verify() { + let genesis = genesis_empty_attesters(); + let timeout = replica_timeout(); + assert!(timeout.verify(&genesis).is_ok()); - // This will create equally weighted validators - let setup1 = Setup::new(rng, 6); - let setup2 = Setup::new(rng, 6); - let mut genesis3 = (*setup1.genesis).clone(); - genesis3.validators = - Committee::new(setup1.genesis.validators.iter().take(3).cloned()).unwrap(); - let genesis3 = genesis3.with_hash(); - - let view: ViewNumber = rng.gen(); - let msgs: Vec<_> = (0..3) - .map(|_| setup1.make_replica_timeout(rng, view)) - .collect(); - - for n in 0..setup1.validator_keys.len() + 1 { - let mut qc = TimeoutQC::new(msgs[0].view.clone()); - for key in &setup1.validator_keys[0..n] { - qc.add( - &key.sign_msg(msgs.choose(rng).unwrap().clone()), - &setup1.genesis, - ) - .unwrap(); - } - let expected_weight: u64 = setup1 - .genesis - .validators - .iter() - .take(n) - .map(|w| w.weight) - .sum(); - if expected_weight >= setup1.genesis.validators.quorum_threshold() { - qc.verify(&setup1.genesis).unwrap(); - } else { - assert_matches!( - qc.verify(&setup1.genesis), - Err(TimeoutQCVerifyError::NotEnoughSigners { .. }) - ); - } - - // Mismatching validator sets. - assert!(qc.verify(&setup2.genesis).is_err()); - assert!(qc.verify(&genesis3).is_err()); - } + // Wrong view + let wrong_genesis = genesis_with_attesters().clone(); + assert_matches!( + timeout.verify(&wrong_genesis), + Err(ReplicaTimeoutVerifyError::BadView(_)) + ); + + // Invalid high vote + let mut timeout = replica_timeout(); + timeout.high_vote.as_mut().unwrap().view.genesis = genesis_with_attesters().hash(); + assert_matches!( + timeout.verify(&genesis), + Err(ReplicaTimeoutVerifyError::InvalidHighVote(_)) + ); + + // Invalid high QC + let mut timeout = replica_timeout(); + timeout.high_qc.as_mut().unwrap().message.view.genesis = genesis_with_attesters().hash(); + assert_matches!( + timeout.verify(&genesis), + Err(ReplicaTimeoutVerifyError::InvalidHighQC(_)) + ); } #[test] @@ -112,31 +92,57 @@ fn test_timeout_qc_high_vote() { } #[test] -fn test_timeout_qc_add_errors() { +fn test_timeout_qc_high_qc() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let setup = Setup::new(rng, 3); + let view = View { + genesis: setup.genesis.hash(), + number: ViewNumber(100), + }; + let mut qc = TimeoutQC::new(view); + + // No high QC + assert!(qc.high_qc().is_none()); + + // Add signatures with different high QC views + for i in 0..3 { + let high_vote_view = view.number; + let high_qc_view = ViewNumber(view.number.0 - i as u64); + let msg = ReplicaTimeout { + view: setup.make_view(view.number), + high_vote: Some(setup.make_replica_commit(rng, high_vote_view)), + high_qc: Some(setup.make_commit_qc(rng, high_qc_view)), + }; + qc.add( + &setup.validator_keys[i].sign_msg(msg.clone()), + &setup.genesis, + ) + .unwrap(); + } + + assert_eq!(qc.high_qc().unwrap().message.view.number, view.number); +} + +#[test] +fn test_timeout_qc_add() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); - let setup = Setup::new(rng, 2); + let setup = Setup::new(rng, 3); let view = rng.gen(); let msg = setup.make_replica_timeout(rng, view); let mut qc = TimeoutQC::new(msg.view.clone()); - let msg = setup.make_replica_timeout(rng, view); - // Add the message - assert_matches!( - qc.add( + // Add the first signature + assert!(qc.map.is_empty()); + assert!(qc + .add( &setup.validator_keys[0].sign_msg(msg.clone()), &setup.genesis - ), - Ok(()) - ); - - // Try to add a message for a different view - let mut msg1 = msg.clone(); - msg1.view.number = view.next(); - assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg1), &setup.genesis), - Err(TimeoutQCAddError::InconsistentViews { .. }) - ); + ) + .is_ok()); + assert_eq!(qc.map.len(), 1); + assert_eq!(qc.map.values().next().unwrap().count(), 1); // Try to add a message from a signer not in committee assert_matches!( @@ -156,19 +162,171 @@ fn test_timeout_qc_add_errors() { Err(TimeoutQCAddError::DuplicateSigner { .. }) ); - // Try to add a message for a validator that already added another message - let msg2 = setup.make_replica_timeout(rng, view); + // Try to add an invalid signature assert_matches!( - qc.add(&setup.validator_keys[0].sign_msg(msg2), &setup.genesis), - Err(TimeoutQCAddError::DuplicateSigner { .. }) + qc.add( + &Signed { + msg: msg.clone(), + key: setup.validator_keys[1].public(), + sig: rng.gen() + }, + &setup.genesis + ), + Err(TimeoutQCAddError::BadSignature(_)) ); - // Add same message signed by another validator. + // Try to add a message with a different view + let mut msg1 = msg.clone(); + msg1.view.number = view.next(); + assert_matches!( + qc.add(&setup.validator_keys[1].sign_msg(msg1), &setup.genesis), + Err(TimeoutQCAddError::InconsistentViews) + ); + + // Try to add an invalid message + let mut wrong_genesis = setup.genesis.clone().0; + wrong_genesis.chain_id = rng.gen(); assert_matches!( qc.add( &setup.validator_keys[1].sign_msg(msg.clone()), - &setup.genesis + &wrong_genesis.with_hash() ), - Ok(()) + Err(TimeoutQCAddError::InvalidMessage(_)) + ); + + // Add same message signed by another validator. + assert!(qc + .add( + &setup.validator_keys[1].sign_msg(msg.clone()), + &setup.genesis + ) + .is_ok()); + assert_eq!(qc.map.len(), 1); + assert_eq!(qc.map.values().next().unwrap().count(), 2); + + // Add a different message signed by another validator. + let msg2 = setup.make_replica_timeout(rng, view); + assert!(qc + .add( + &setup.validator_keys[2].sign_msg(msg2.clone()), + &setup.genesis + ) + .is_ok()); + assert_eq!(qc.map.len(), 2); + assert_eq!(qc.map.values().next().unwrap().count(), 2); + assert_eq!(qc.map.values().last().unwrap().count(), 1); +} + +#[test] +fn test_timeout_qc_verify() { + let ctx = ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let mut setup = Setup::new(rng, 6); + setup.push_blocks(rng, 2); + let view = rng.gen(); + let qc = setup.make_timeout_qc(rng, view, None); + + // Verify the QC + assert!(qc.verify(&setup.genesis).is_ok()); + + // QC with bad view + let mut qc1 = qc.clone(); + qc1.view = rng.gen(); + assert_matches!( + qc1.verify(&setup.genesis), + Err(TimeoutQCVerifyError::BadView(_)) + ); + + // QC with message with inconsistent view + let mut qc2 = qc.clone(); + qc2.map.insert( + ReplicaTimeout { + view: qc2.view.clone().next(), + high_vote: None, + high_qc: None, + }, + Signers::new(setup.genesis.validators.len()), + ); + assert_matches!( + qc2.verify(&setup.genesis), + Err(TimeoutQCVerifyError::InconsistentView(_)) + ); + + // QC with message with wrong signer length + let mut qc3 = qc.clone(); + qc3.map.insert( + ReplicaTimeout { + view: qc3.view.clone(), + high_vote: None, + high_qc: None, + }, + Signers::new(setup.genesis.validators.len() + 1), + ); + assert_matches!( + qc3.verify(&setup.genesis), + Err(TimeoutQCVerifyError::WrongSignersLength(_)) + ); + + // QC with message with no signers + let mut qc4 = qc.clone(); + qc4.map.insert( + ReplicaTimeout { + view: qc4.view.clone(), + high_vote: None, + high_qc: None, + }, + Signers::new(setup.genesis.validators.len()), + ); + assert_matches!( + qc4.verify(&setup.genesis), + Err(TimeoutQCVerifyError::NoSignersAssigned(_)) + ); + + // QC with overlapping signers + let mut qc5 = qc.clone(); + let mut signers = Signers::new(setup.genesis.validators.len()); + signers + .0 + .set(rng.gen_range(0..setup.genesis.validators.len()), true); + qc5.map.insert( + ReplicaTimeout { + view: qc5.view.clone(), + high_vote: None, + high_qc: None, + }, + signers, + ); + assert_matches!( + qc5.verify(&setup.genesis), + Err(TimeoutQCVerifyError::OverlappingSignatureSet(_)) + ); + + // QC with invalid message + let mut qc6 = qc.clone(); + let (mut timeout, signers) = qc6.map.pop_first().unwrap(); + timeout.high_qc = Some(rng.gen()); + qc6.map.insert(timeout, signers); + assert_matches!( + qc6.verify(&setup.genesis), + Err(TimeoutQCVerifyError::InvalidMessage(_, _)) + ); + + // QC with not enough weight + let mut qc7 = qc.clone(); + let (timeout, mut signers) = qc7.map.pop_first().unwrap(); + signers.0.set(0, false); + signers.0.set(4, false); + qc7.map.insert(timeout, signers); + assert_matches!( + qc7.verify(&setup.genesis), + Err(TimeoutQCVerifyError::NotEnoughWeight { .. }) + ); + + // QC with bad signature + let mut qc8 = qc.clone(); + qc8.signature = rng.gen(); + assert_matches!( + qc8.verify(&setup.genesis), + Err(TimeoutQCVerifyError::BadSignature(_)) ); } diff --git a/node/libs/roles/src/validator/testonly.rs b/node/libs/roles/src/validator/testonly.rs index b88768d9a..eff17eda5 100644 --- a/node/libs/roles/src/validator/testonly.rs +++ b/node/libs/roles/src/validator/testonly.rs @@ -249,7 +249,7 @@ impl Setup { /// Creates a ReplicaTimeout with a random payload. pub fn make_replica_timeout(&self, rng: &mut impl Rng, view: ViewNumber) -> ReplicaTimeout { - let high_vote_view = ViewNumber(rng.gen_range(0..view.0)); + let high_vote_view = ViewNumber(rng.gen_range(0..=view.0)); let high_qc_view = ViewNumber(rng.gen_range(0..high_vote_view.0)); ReplicaTimeout { view: self.make_view(view), @@ -277,16 +277,16 @@ impl Setup { }; let mut qc = TimeoutQC::new(self.make_view(view)); + if payload_opt.is_none() { + vote.proposal.payload = rng.gen(); + } + let msg = ReplicaTimeout { + view: self.make_view(view), + high_vote: Some(vote.clone()), + high_qc: Some(commit_qc.clone()), + }; for key in &self.validator_keys { - if payload_opt.is_none() { - vote.proposal.payload = rng.gen(); - } - let msg = ReplicaTimeout { - view: self.make_view(view), - high_vote: Some(vote.clone()), - high_qc: Some(commit_qc.clone()), - }; - qc.add(&key.sign_msg(msg), &self.genesis).unwrap(); + qc.add(&key.sign_msg(msg.clone()), &self.genesis).unwrap(); } qc diff --git a/spec/informal-spec/types.rs b/spec/informal-spec/types.rs index f5789cff4..a2a517c69 100644 --- a/spec/informal-spec/types.rs +++ b/spec/informal-spec/types.rs @@ -183,8 +183,10 @@ impl SignedTimeoutVote { } fn verify(self) -> bool { - // If we wish, there are two invariants that are easy to check but aren't required for correctness: - // self.view() >= self.high_vote.view() && self.high_vote.view() >= self.high_commit_qc_view + // If we wish, there are three invariants that are easy to check but don't need to be stricly enforced for correctness: + // 1. self.view() >= self.high_vote.view() + // 2. self.high_vote.view() >= self.high_commit_qc_view + // 3. self.view() > self.high_commit_qc_view self.vote.high_commit_qc_view == self.high_commit_qc.map(|x| x.view()) && self.verify_sig() && self.high_commit_qc.map(|qc| qc.verify()) } From fcf36aa4b37e22754f35fd9a53d74784216e4419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Fran=C3=A7a?= Date: Mon, 21 Oct 2024 02:42:03 +0100 Subject: [PATCH 10/10] nit --- node/libs/roles/src/validator/messages/tests/versions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/libs/roles/src/validator/messages/tests/versions.rs b/node/libs/roles/src/validator/messages/tests/versions.rs index 1fa2c46db..3832c523d 100644 --- a/node/libs/roles/src/validator/messages/tests/versions.rs +++ b/node/libs/roles/src/validator/messages/tests/versions.rs @@ -46,7 +46,7 @@ mod version1 { // Check if msg.hash() is equal to hash. if msg.hash() != hash { - return Err(anyhow::anyhow!("Hash mismatch")); + anyhow::bail!("Hash mismatch"); } // Check if sig is a valid signature of hash.