diff --git a/node/actors/bft/src/leader/replica_commit.rs b/node/actors/bft/src/leader/replica_commit.rs index e142e3a2..54fdef2b 100644 --- a/node/actors/bft/src/leader/replica_commit.rs +++ b/node/actors/bft/src/leader/replica_commit.rs @@ -127,11 +127,20 @@ impl StateMachine { .or_insert_with(|| CommitQC::new(message.clone(), self.config.genesis())) .add(&signed_message, self.config.genesis()); - // We store the message in our cache. let cache_entry = self .commit_message_cache .entry(message.view.number) .or_default(); + + // We check validators weight from current messages + // TODO: this is wrong, we have to calculate previous weights by proposal + let previous_weight = self + .config + .genesis() + .validators + .weight_from_msgs(&cache_entry.values().collect()); + + // We store the message in our cache. cache_entry.insert(author.clone(), signed_message.clone()); // Now we check if we have enough messages to continue. @@ -146,7 +155,9 @@ impl StateMachine { else { return Ok(()); }; - debug_assert_eq!(replica_messages.len(), threshold); + // Check that previous weight did not reach threshold + // to ensure this is the first time the threshold has been reached + debug_assert!(previous_weight < threshold); // ----------- Update the state machine -------------- diff --git a/node/actors/bft/src/leader/replica_prepare.rs b/node/actors/bft/src/leader/replica_prepare.rs index 6e074302..e2a75835 100644 --- a/node/actors/bft/src/leader/replica_prepare.rs +++ b/node/actors/bft/src/leader/replica_prepare.rs @@ -137,21 +137,28 @@ impl StateMachine { .or_insert_with(|| validator::PrepareQC::new(message.view.clone())) .add(&signed_message, self.config.genesis()); - // We store the message in our cache. - self.prepare_message_cache + // Work on current view messages + let entry = self + .prepare_message_cache .entry(message.view.number) - .or_default() - .insert(author.clone(), signed_message); + .or_default(); - // Now we check if we have enough messages to continue. - let messages: Vec<&validator::Signed> = self - .prepare_message_cache - .get(&message.view.number) - .unwrap() - .values() - .collect(); + // We check validators weight from current messages + let previous_weight = self + .config + .genesis() + .validators + .weight_from_msgs(&entry.values().collect()); + + // We store the message in our cache. + entry.insert(author.clone(), signed_message); - let weight = self.config.genesis().validators.weight_from_msgs(&messages); + // Now we check if we have enough weight to continue. + let weight = self + .config + .genesis() + .validators + .weight_from_msgs(&entry.values().collect()); let threshold = self.config.genesis().validators.threshold(); if weight < threshold { return Ok(()); @@ -161,7 +168,9 @@ impl StateMachine { // for this same view if we receive another replica prepare message after this. self.prepare_message_cache.remove(&message.view.number); - debug_assert!(weight >= threshold); + // Check that previous weight did not reach threshold + // to ensure this is the first time the threshold has been reached + debug_assert!(previous_weight < threshold); // ----------- Update the state machine -------------- diff --git a/node/actors/bft/src/testonly/ut_harness.rs b/node/actors/bft/src/testonly/ut_harness.rs index 29590cce..9b26535e 100644 --- a/node/actors/bft/src/testonly/ut_harness.rs +++ b/node/actors/bft/src/testonly/ut_harness.rs @@ -7,12 +7,12 @@ use crate::{ testonly, Config, PayloadManager, }; use assert_matches::assert_matches; -use std::{cmp::Ordering, sync::Arc}; +use std::sync::Arc; use zksync_concurrency::ctx; use zksync_consensus_network as network; use zksync_consensus_roles::validator::{ self, CommitQC, LeaderCommit, LeaderPrepare, Phase, PrepareQC, ReplicaCommit, ReplicaPrepare, - SecretKey, Signed, ViewNumber, + SecretKey, Signed, ValidatorCommittee, ViewNumber, }; use zksync_consensus_storage::{ testonly::{in_memory, new_store}, @@ -224,15 +224,23 @@ impl UTHarness { ctx: &ctx::Ctx, msg: ReplicaPrepare, ) -> Signed { + let expected_validator_weight = ValidatorCommittee::MAX_WEIGHT / self.keys.len(); let want_threshold = self.genesis().validators.threshold(); let mut leader_prepare = None; let msgs: Vec<_> = self.keys.iter().map(|k| k.sign_msg(msg.clone())).collect(); + let mut first_match = true; for (i, msg) in msgs.into_iter().enumerate() { let res = self.process_replica_prepare(ctx, msg).await; - match (i + 1).cmp(&want_threshold) { - Ordering::Equal => leader_prepare = res.unwrap(), - Ordering::Less => assert!(res.unwrap().is_none()), - Ordering::Greater => assert_matches!(res, Err(replica_prepare::Error::Old { .. })), + match ( + (i + 1) * expected_validator_weight < want_threshold, + first_match, + ) { + (true, _) => assert!(res.unwrap().is_none()), + (false, true) => { + first_match = false; + leader_prepare = res.unwrap() + } + (false, false) => assert_matches!(res, Err(replica_prepare::Error::Old { .. })), } } leader_prepare.unwrap() @@ -252,15 +260,23 @@ impl UTHarness { ctx: &ctx::Ctx, msg: ReplicaCommit, ) -> Signed { + let expected_validator_weight = ValidatorCommittee::MAX_WEIGHT / self.keys.len(); + let want_threshold = self.genesis().validators.threshold(); + let mut first_match = true; for (i, key) in self.keys.iter().enumerate() { let res = self .leader .process_replica_commit(ctx, key.sign_msg(msg.clone())); - let want_threshold = self.genesis().validators.threshold(); - match (i + 1).cmp(&want_threshold) { - Ordering::Equal => res.unwrap(), - Ordering::Less => res.unwrap(), - Ordering::Greater => assert_matches!(res, Err(replica_commit::Error::Old { .. })), + match ( + (i + 1) * expected_validator_weight < want_threshold, + first_match, + ) { + (true, _) => res.unwrap(), + (false, true) => { + first_match = false; + res.unwrap() + } + (false, false) => assert_matches!(res, Err(replica_commit::Error::Old { .. })), } } self.try_recv().unwrap() diff --git a/node/libs/roles/src/validator/messages/consensus.rs b/node/libs/roles/src/validator/messages/consensus.rs index f08d68cd..07e16a6e 100644 --- a/node/libs/roles/src/validator/messages/consensus.rs +++ b/node/libs/roles/src/validator/messages/consensus.rs @@ -83,8 +83,13 @@ pub struct ValidatorCommittee { } impl ValidatorCommittee { + /// Maximum validator weight + /// 100.00% + pub const MAX_WEIGHT: usize = 10000; + /// Required weight to verify signatures. - const THRESHOLD: usize = 100_00; + /// Currently 80.00% + const THRESHOLD: usize = 8000; /// Creates a new ValidatorCommittee from a list of validator public keys. pub fn new(validators: impl IntoIterator) -> anyhow::Result { @@ -135,17 +140,17 @@ impl ValidatorCommittee { self.vec.len() } - /// Returns true if the given validator is in the validator set. + /// 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 set. + /// Get validator by its index in the committee. pub fn get(&self, index: usize) -> Option<&validator::PublicKey> { self.vec.get(index) } - /// Get the index of a validator in the set. + /// Get the index of a validator in the committee. pub fn index(&self, validator: &validator::PublicKey) -> Option { self.indexes.get(validator).copied() } @@ -156,12 +161,12 @@ impl ValidatorCommittee { self.get(index).unwrap().clone() } - /// Signature threshold for this validator set. + /// Signature threshold for this validator committee. pub fn threshold(&self) -> usize { Self::THRESHOLD } - /// Maximal number of faulty replicas allowed in this validator set. + /// Maximal number of faulty replicas allowed in this validator committee. pub fn faulty_replicas(&self) -> usize { faulty_replicas(self.len()) } diff --git a/node/libs/roles/src/validator/tests.rs b/node/libs/roles/src/validator/tests.rs index 0fbd9399..34649cde 100644 --- a/node/libs/roles/src/validator/tests.rs +++ b/node/libs/roles/src/validator/tests.rs @@ -200,6 +200,7 @@ fn test_commit_qc() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); + // This will ceate equally weighted validators let setup1 = Setup::new(rng, 6); let setup2 = Setup::new(rng, 6); let genesis3 = Genesis { @@ -216,7 +217,8 @@ fn test_commit_qc() { for key in &setup1.keys[0..i] { qc.add(&key.sign_msg(qc.message.clone()), &setup1.genesis); } - if i >= setup1.genesis.validators.threshold() { + let expected_weight = i * ValidatorCommittee::MAX_WEIGHT / 6; + if expected_weight >= setup1.genesis.validators.threshold() { qc.verify(&setup1.genesis).unwrap(); } else { assert_matches!( @@ -237,6 +239,7 @@ fn test_prepare_qc() { let ctx = ctx::test_root(&ctx::RealClock); let rng = &mut ctx.rng(); + // This will ceate equally weighted validators let setup1 = Setup::new(rng, 6); let setup2 = Setup::new(rng, 6); let genesis3 = Genesis { @@ -260,7 +263,8 @@ fn test_prepare_qc() { &setup1.genesis, ); } - if n >= setup1.genesis.validators.threshold() { + let expected_weight = n * ValidatorCommittee::MAX_WEIGHT / 6; + if expected_weight >= setup1.genesis.validators.threshold() { qc.verify(&setup1.genesis).unwrap(); } else { assert_matches!(