Skip to content

Commit

Permalink
BFT-457: Remove attester::AggregateSignature
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Jun 12, 2024
1 parent f418e3b commit ce1f480
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 234 deletions.
30 changes: 11 additions & 19 deletions node/actors/network/src/gossip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,20 +137,14 @@ impl Network {
.last_viewed_qc
.clone()
.map(|qc| {
attester::BatchQC::new(
attester::Batch {
number: qc.message.number.next(),
},
self.genesis(),
)
attester::BatchQC::new(attester::Batch {
number: qc.message.number.next(),
})
})
.unwrap_or_else(|| {
attester::BatchQC::new(
attester::Batch {
number: attester::BatchNumber(0),
},
self.genesis(),
)
attester::BatchQC::new(attester::Batch {
number: attester::BatchNumber(0),
})
})
.context("new qc")?;

Expand All @@ -160,9 +154,7 @@ impl Network {
.batch_qc
.clone()
.entry(new_qc.message.number.clone())
.or_insert_with(|| {
attester::BatchQC::new(new_qc.message.clone(), self.genesis()).expect("qc")
})
.or_insert_with(|| attester::BatchQC::new(new_qc.message.clone()).expect("qc"))
.add(&sig, self.genesis())
.is_err()
{
Expand All @@ -171,12 +163,12 @@ impl Network {
}
}

let weight = attesters.weight(
&self
.batch_qc
let weight = attesters.weight_of_keys(
self.batch_qc
.get(&new_qc.message.number)
.context("last qc")?
.signers,
.signatures
.keys(),
);

if weight < attesters.threshold() {
Expand Down
45 changes: 24 additions & 21 deletions node/libs/roles/src/attester/conv.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use super::{
AggregateSignature, Batch, BatchNumber, BatchQC, Msg, MsgHash, PublicKey, Signature, Signed,
Signers, WeightedAttester,
Batch, BatchNumber, BatchQC, Msg, MsgHash, PublicKey, Signature, Signed, Signers,
WeightedAttester,
};
use crate::proto::attester::{self as proto};
use crate::proto::attester::{self as proto, Attestation};
use anyhow::Context as _;
use zksync_consensus_crypto::ByteFmt;
use zksync_consensus_utils::enum_util::Variant;
Expand Down Expand Up @@ -114,20 +114,6 @@ impl ProtoFmt for Signers {
}
}

impl ProtoFmt for AggregateSignature {
type Proto = proto::AggregateSignature;

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self(ByteFmt::decode(required(&r.secp256k1)?)?))
}

fn build(&self) -> Self::Proto {
Self::Proto {
secp256k1: Some(self.0.encode()),
}
}
}

impl ProtoFmt for MsgHash {
type Proto = proto::MsgHash;

Expand All @@ -146,18 +132,35 @@ impl ProtoFmt for BatchQC {
type Proto = proto::BatchQc;

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
let signatures: Vec<(PublicKey, Signature)> = r
.signatures
.iter()
.map(|s| {
let key = read_required(&s.key).context("key")?;
let sig = read_required(&s.sig).context("sig")?;
Ok((key, sig))
})
.collect::<anyhow::Result<Vec<_>>>()?;

let signatures = signatures.into_iter().collect();

Ok(Self {
message: read_required(&r.msg).context("message")?,
signers: read_required(&r.signers).context("signers")?,
signature: read_required(&r.sig).context("signature")?,
signatures,
})
}

fn build(&self) -> Self::Proto {
Self::Proto {
msg: Some(self.message.build()),
signers: Some(self.signers.build()),
sig: Some(self.signature.build()),
signatures: self
.signatures
.iter()
.map(|(pk, sig)| Attestation {
key: Some(pk.build()),
sig: Some(sig.build()),
})
.collect(),
}
}
}
74 changes: 0 additions & 74 deletions node/libs/roles/src/attester/keys/aggregate_signature.rs

This file was deleted.

2 changes: 0 additions & 2 deletions node/libs/roles/src/attester/keys/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
//! Keys and signatures used by the attester.
mod aggregate_signature;
mod public_key;
mod secret_key;
mod signature;

pub use aggregate_signature::AggregateSignature;
pub use public_key::PublicKey;
pub use secret_key::SecretKey;
pub use signature::Signature;
70 changes: 32 additions & 38 deletions node/libs/roles/src/attester/messages/batch.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use super::{Signed, Signers};
use std::collections::BTreeMap;

use super::Signed;
use crate::{attester, validator::Genesis};
use anyhow::{ensure, Context as _};
use zksync_consensus_utils::enum_util::Variant;

#[derive(Clone, Debug, PartialEq, Eq, Hash, Default, PartialOrd)]
/// A batch number.
Expand All @@ -26,10 +29,8 @@ pub struct Batch {
/// It contains the signatures of the attesters that signed the batch.
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct BatchQC {
/// The aggregate signature of the signed L1 batches.
pub signature: attester::AggregateSignature,
/// The attesters that signed this message.
pub signers: Signers,
/// The signatures of the signed L1 batches from all the attesters who signed it.
pub signatures: BTreeMap<attester::PublicKey, attester::Signature>,
/// The message that was signed.
pub message: Batch,
}
Expand Down Expand Up @@ -75,36 +76,34 @@ pub enum BatchQCAddError {

impl BatchQC {
/// Create a new empty instance for a given `Batch` message.
pub fn new(message: Batch, genesis: &Genesis) -> anyhow::Result<Self> {
pub fn new(message: Batch) -> anyhow::Result<Self> {
Ok(Self {
message,
signers: Signers::new(
genesis
.attesters
.as_ref()
.context("no attester committee in genesis")?
.len(),
),
signature: attester::AggregateSignature::default(),
signatures: BTreeMap::default(),
})
}

/// Add a attester's signature.
/// Signature is assumed to be already verified.
pub fn add(&mut self, msg: &Signed<Batch>, genesis: &Genesis) -> anyhow::Result<()> {
use BatchQCAddError as Error;
ensure!(self.message == msg.msg, Error::InconsistentMessages);
let i = genesis

let committee = genesis
.attesters
.as_ref()
.context("no attester committee in genesis")?
.index(&msg.key)
.ok_or(Error::SignerNotInCommittee {
.context("no attester committee in genesis")?;

ensure!(self.message == msg.msg, Error::InconsistentMessages);
ensure!(!self.signatures.contains_key(&msg.key), Error::Exists);
ensure!(
committee.contains(&msg.key),
Error::SignerNotInCommittee {
signer: Box::new(msg.key.clone()),
})?;
ensure!(!self.signers.0[i], Error::Exists);
self.signers.0.set(i, true);
self.signature.add(&msg.sig);
}
);

self.signatures.insert(msg.key.clone(), msg.sig.clone());

Ok(())
}

Expand All @@ -115,28 +114,23 @@ impl BatchQC {
.attesters
.as_ref()
.ok_or(Error::AttestersNotInGenesis)?;
if self.signers.len() != attesters.len() {
return Err(Error::BadSignersSet);
}

// Verify that the signer's weight is sufficient.
let weight = attesters.weight(&self.signers);
let weight = attesters.weight_of_keys(self.signatures.keys());
let threshold = attesters.threshold();
if weight < threshold {
return Err(Error::NotEnoughSigners {
got: weight,
want: threshold,
});
}
// The enumeration here goes by order of public key, which assumes
// that the aggregate signature does not care about ordering.
let messages_and_keys = attesters
.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)

let hash = self.message.clone().insert().hash();

for (pk, sig) in &self.signatures {
sig.verify_hash(&hash, pk).map_err(Error::BadSignature)?;
}

Ok(())
}
}
8 changes: 7 additions & 1 deletion node/libs/roles/src/attester/messages/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl Committee {
}

/// Compute the sum of signers weights.
pub fn weight(&self, signers: &Signers) -> u64 {
pub fn weight_of_signers(&self, signers: &Signers) -> u64 {
assert_eq!(self.vec.len(), signers.len());
self.vec
.iter()
Expand All @@ -162,6 +162,12 @@ impl Committee {
.sum()
}

/// Compute the sum of signers weights.
pub fn weight_of_keys<'a>(&self, keys: impl Iterator<Item = &'a attester::PublicKey>) -> u64 {
keys.filter_map(|pk| self.index(pk).map(|i| self.vec[i].weight))
.sum()
}

/// Sum of all attesters weight in the committee
pub fn total_weight(&self) -> u64 {
self.total_weight
Expand Down
Loading

0 comments on commit ce1f480

Please sign in to comment.