Skip to content

Commit

Permalink
Simplified ValidatorCommittee internal structure and methods
Browse files Browse the repository at this point in the history
  • Loading branch information
ElFantasma committed Mar 26, 2024
1 parent 94e4ef4 commit fc5a921
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 56 deletions.
17 changes: 1 addition & 16 deletions node/actors/bft/src/leader/replica_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,6 @@ impl StateMachine {
.entry(message.view.number)
.or_default();

// We check validators weight from current messages, stored by proposal
let weight_before = self.config.genesis().validators.weight_from_msgs(
cache_entry
.values()
.filter(|m| m.msg.proposal == message.proposal)
.collect::<Vec<_>>()
.as_slice(),
);

// We store the message in our cache.
cache_entry.insert(author.clone(), signed_message.clone());

Expand All @@ -151,17 +142,11 @@ impl StateMachine {
.collect::<Vec<_>>()
.as_slice(),
);
let threshold = self.config.genesis().validators.threshold();
if weight < threshold {
if weight < self.config.genesis().validators.threshold() {
return Ok(());
};

// Check that previous weight did not reach threshold
// to ensure this is the first time the threshold has been reached
debug_assert!(weight_before < threshold);

// ----------- Update the state machine --------------

let now = ctx.now();
metrics::METRICS
.leader_commit_phase_latency
Expand Down
2 changes: 1 addition & 1 deletion node/actors/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl Executor {
.block_store
.genesis()
.validators
.iter()
.iter_keys()
.any(|key| key == &validator.key.public())
{
anyhow::bail!("this validator doesn't belong to the consensus");
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Network {
/// Constructs a new consensus network state.
pub(crate) fn new(ctx: &ctx::Ctx, gossip: Arc<gossip::Network>) -> Option<Arc<Self>> {
let key = gossip.cfg.validator_key.clone()?;
let validators: HashSet<_> = gossip.genesis().validators.iter().cloned().collect();
let validators: HashSet<_> = gossip.genesis().validators.iter_keys().cloned().collect();
Some(Arc::new(Self {
key,
inbound: PoolWatch::new(validators.clone(), 0),
Expand Down
2 changes: 1 addition & 1 deletion node/actors/network/src/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl Instance {
pub async fn wait_for_consensus_connections(&self) {
let consensus_state = self.net.consensus.as_ref().unwrap();

let want: HashSet<_> = self.genesis().validators.iter().cloned().collect();
let want: HashSet<_> = self.genesis().validators.iter_keys().cloned().collect();
consensus_state
.inbound
.subscribe()
Expand Down
6 changes: 1 addition & 5 deletions node/libs/roles/src/validator/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,7 @@ impl ProtoFmt for Genesis {
fn build(&self) -> Self::Proto {
Self::Proto {
fork: Some(self.fork.build()),
validators: self
.validators
.weighted_validators_iter()
.map(|v| v.build())
.collect(),
validators: self.validators.iter().map(|v| v.build()).collect(),
}
}
}
Expand Down
42 changes: 20 additions & 22 deletions node/libs/roles/src/validator/messages/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use bit_vec::BitVec;
use std::{
collections::{BTreeMap, BTreeSet},
fmt,
iter::zip,
};
use zksync_consensus_crypto::{keccak256::Keccak256, ByteFmt, Text, TextFmt};
use zksync_consensus_utils::enum_util::{BadVariantError, Variant};
Expand Down Expand Up @@ -76,9 +75,8 @@ impl Default for Fork {
/// We represent each validator by its validator public key.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ValidatorCommittee {
vec: Vec<validator::PublicKey>,
vec: Vec<WeightedValidator>,
indexes: BTreeMap<validator::PublicKey, usize>,
weights: Vec<u64>,
}

impl ValidatorCommittee {
Expand All @@ -89,41 +87,40 @@ impl ValidatorCommittee {
/// Creates a new ValidatorCommittee from a list of validator public keys.
pub fn new(validators: impl IntoIterator<Item = WeightedValidator>) -> anyhow::Result<Self> {
let mut set = BTreeSet::new();
let mut weights = BTreeMap::new();
let mut weighted_validators = BTreeMap::new();
for validator in validators {
anyhow::ensure!(
set.insert(validator.key.clone()),
"Duplicate validator in ValidatorCommittee"
);
weights.insert(validator.key, validator.weight);
weighted_validators.insert(validator.key.clone(), validator);
}
anyhow::ensure!(
!set.is_empty(),
"ValidatorCommittee must contain at least one validator"
);
Ok(Self {
vec: set.iter().cloned().collect(),
vec: set
.iter()
.map(|key| weighted_validators[key].clone())
.collect(),
indexes: set
.clone()
.into_iter()
.enumerate()
.map(|(i, pk)| (pk, i))
.collect(),
weights: set.iter().map(|pk| weights[pk]).collect(),
})
}

/// Iterates over validators.
pub fn iter(&self) -> impl Iterator<Item = &validator::PublicKey> {
/// Iterates over weighted validators.
pub fn iter(&self) -> impl Iterator<Item = &WeightedValidator> {
self.vec.iter()
}

/// Iterates over validators.
pub fn weighted_validators_iter(&self) -> impl Iterator<Item = WeightedValidator> + '_ {
zip(&self.vec, &self.weights).map(|(key, weight)| WeightedValidator {
key: key.clone(),
weight: *weight,
})
/// Iterates over validator keys.
pub fn iter_keys(&self) -> impl Iterator<Item = &validator::PublicKey> {
self.vec.iter().map(|v| &v.key)
}

/// Returns the number of validators.
Expand All @@ -138,7 +135,7 @@ impl ValidatorCommittee {
}

/// Get validator by its index in the committee.
pub fn get(&self, index: usize) -> Option<&validator::PublicKey> {
pub fn get(&self, index: usize) -> Option<&WeightedValidator> {
self.vec.get(index)
}

Expand All @@ -150,7 +147,7 @@ impl ValidatorCommittee {
/// Computes the validator for the given view.
pub fn view_leader(&self, view_number: ViewNumber) -> validator::PublicKey {
let index = view_number.0 as usize % self.len();
self.get(index).unwrap().clone()
self.get(index).unwrap().key.clone()
}

/// Signature threshold for this validator committee.
Expand All @@ -165,11 +162,11 @@ impl ValidatorCommittee {

/// Compute the sum of signers weights.
pub fn weight_from_signers(&self, signers: Signers) -> u64 {
self.weights
self.vec
.iter()
.enumerate()
.filter(|(i, _)| signers.0[*i])
.map(|(_, weight)| weight)
.map(|(_, v)| v.weight)
.sum()
}

Expand All @@ -178,16 +175,17 @@ impl ValidatorCommittee {
signed
.iter()
.map(|s| {
self.weights[self
self.vec[self
.index(&s.key)
.expect("Signer is not in validator committee")]
.weight
})
.sum()
}

/// Sum of all validators' weight in the committee
pub fn max_weight(&self) -> u64 {
self.weights.iter().sum()
self.vec.iter().map(|v| v.weight).sum()
}
}

Expand Down Expand Up @@ -423,7 +421,7 @@ pub enum Phase {
}

/// Validator representation inside a ValidatorCommittee.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct WeightedValidator {
/// Validator key
pub key: validator::PublicKey,
Expand Down
2 changes: 1 addition & 1 deletion node/libs/roles/src/validator/messages/leader_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl CommitQC {
// Now we can verify the signature.
let messages_and_keys = genesis
.validators
.iter()
.iter_keys()
.enumerate()
.filter(|(i, _)| self.signers.0[*i])
.map(|(_, pk)| (self.message.clone(), pk));
Expand Down
2 changes: 1 addition & 1 deletion node/libs/roles/src/validator/messages/leader_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl PrepareQC {
let messages_and_keys = self.map.clone().into_iter().flat_map(|(msg, signers)| {
genesis
.validators
.iter()
.iter_keys()
.enumerate()
.filter(|(i, _)| signers.0[*i])
.map(|(_, pk)| (msg.clone(), pk))
Expand Down
12 changes: 4 additions & 8 deletions node/libs/roles/src/validator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,8 @@ fn test_commit_qc() {
let setup1 = Setup::new(rng, 6);
let setup2 = Setup::new(rng, 6);
let genesis3 = Genesis {
validators: ValidatorCommittee::new(
setup1.genesis.validators.weighted_validators_iter().take(3),
)
.unwrap(),
validators: ValidatorCommittee::new(setup1.genesis.validators.iter().take(3).cloned())
.unwrap(),
fork: setup1.genesis.fork.clone(),
};
let validator_weight = setup1.genesis.validators.max_weight();
Expand Down Expand Up @@ -244,10 +242,8 @@ fn test_prepare_qc() {
let setup1 = Setup::new(rng, 6);
let setup2 = Setup::new(rng, 6);
let genesis3 = Genesis {
validators: ValidatorCommittee::new(
setup1.genesis.validators.weighted_validators_iter().take(3),
)
.unwrap(),
validators: ValidatorCommittee::new(setup1.genesis.validators.iter().take(3).cloned())
.unwrap(),
fork: setup1.genesis.fork.clone(),
};

Expand Down

0 comments on commit fc5a921

Please sign in to comment.