BFT-457: Simplify batch signature

node/actors/network/src/gossip/mod.rs

@@ -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")?;
 
@@ -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()
                 {
@@ -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() {

node/libs/crypto/src/secp256k1/mod.rs

@@ -175,96 +175,6 @@ impl Ord for Signature {
     }
 }
 
-/// Seclp256k1 multi-sig
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Default)]
-pub struct AggregateSignature(Vec<Signature>);
-
-impl AggregateSignature {
-    /// Add a signature to the aggregate.
-    pub fn add(&mut self, sig: Signature) {
-        self.0.push(sig)
-    }
-
-    /// Verifies an aggregated signature for multiple messages against the provided list of public keys.
-    ///
-    /// Verification fails if the total number of messages and signatures do not match.
-    ///
-    /// This method was added to mimic the bn254 version, which used BLS signature aggregation and did
-    /// not rely on message and key ordering; it supported different messages from each signatory as well.
-    ///
-    /// Here we have a simple list of signatures, and we cannot rely on them having been added in the same
-    /// order in which they are going to be verified, therefore we have to try every message against every
-    /// signature.
-    ///
-    /// The method assumes that there are no repeated pairs in the input, ie. that every signature is used exactly once.
-    ///
-    /// The method inherited its non-generic never-inlined nature from the bn254 version, which was marked as such
-    /// to allow the crate level optimisations to take place, which in turn were required for tests to run fast.
-    #[inline(never)]
-    pub fn verify_hash(&self, hashes_and_pks: &[(&[u8], &PublicKey)]) -> anyhow::Result<()> {
-        if hashes_and_pks.len() != self.0.len() {
-            bail!(
-                "wrong number of inputs vs signatures: expected {}, got {}",
-                self.0.len(),
-                hashes_and_pks.len()
-            );
-        }
-        // Keep track of which signatures have been verified.
-        let mut verified = vec![false; self.0.len()];
-
-        'inputs: for (i, (hash, pk)) in hashes_and_pks.iter().enumerate() {
-            'signatures: for (j, sig) in self.0.iter().enumerate() {
-                if verified[j] {
-                    continue 'signatures;
-                }
-                if sig.verify_hash(hash, pk).is_ok() {
-                    verified[j] = true;
-                    continue 'inputs;
-                }
-            }
-            bail!("failed to verify message {i} against any of the signatures in the aggregate");
-        }
-        Ok(())
-    }
-
-    /// Verify messages after hashing them with Keccak256
-    pub fn verify<'a>(
-        &self,
-        msgs_and_pks: impl Iterator<Item = (&'a [u8], &'a PublicKey)>,
-    ) -> anyhow::Result<()> {
-        let hashes_and_pks = msgs_and_pks
-            .map(|(msg, pk)| (Keccak256::new(msg), pk))
-            .collect::<Vec<_>>();
-
-        let hashes_and_pks = hashes_and_pks
-            .iter()
-            .map(|(hash, pk)| (hash.as_bytes().as_slice(), *pk))
-            .collect::<Vec<_>>();
-
-        self.verify_hash(&hashes_and_pks)
-    }
-}
-
-impl ByteFmt for AggregateSignature {
-    fn decode(bytes: &[u8]) -> anyhow::Result<Self> {
-        anyhow::ensure!(
-            bytes.len() % SIGNATURE_LENGTH == 0,
-            "unexpected aggregate signature length"
-        );
-
-        let sigs = bytes
-            .chunks_exact(SIGNATURE_LENGTH)
-            .map(Signature::decode)
-            .collect::<Result<Vec<_>, _>>()?;
-
-        Ok(Self(sigs))
-    }
-
-    fn encode(&self) -> Vec<u8> {
-        self.0.iter().flat_map(|s| s.encode()).collect()
-    }
-}
-
 /// Normalize the V in signatures from Ethereum tooling.
 ///
 /// Based on <https://github.com/gakonst/ethers-rs/blob/51fe937f6515689b17a3a83b74a05984ad3a7f11/ethers-core/src/types/signature.rs#L202>

node/libs/crypto/src/secp256k1/testonly.rs

@@ -1,4 +1,4 @@
-use crate::secp256k1::{AggregateSignature, PublicKey, SecretKey, Signature};
+use crate::secp256k1::{PublicKey, SecretKey, Signature};
 use elliptic_curve::ScalarPrimitive;
 use k256::ecdsa::RecoveryId;
 use rand::{
@@ -50,13 +50,3 @@ impl Distribution<Signature> for Standard {
         Signature { sig, recid }
     }
 }
-
-impl Distribution<AggregateSignature> for Standard {
-    fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> AggregateSignature {
-        let mut agg = AggregateSignature::default();
-        for _ in 0..rng.gen_range(1..=5) {
-            agg.add(rng.gen());
-        }
-        agg
-    }
-}

node/libs/crypto/src/secp256k1/tests.rs

@@ -3,7 +3,6 @@ use std::fmt::Debug;
 use rand::{
     distributions::{Distribution, Standard},
     rngs::StdRng,
-    seq::SliceRandom,
     Rng, SeedableRng,
 };
 
@@ -59,11 +58,6 @@ fn prop_sig_format() {
     prop_byte_format::<Signature>();
 }
 
-#[test]
-fn prop_aggsig_format() {
-    prop_byte_format::<AggregateSignature>();
-}
-
 #[test]
 fn prop_sign_verify() {
     let rng = &mut make_rng();
@@ -103,56 +97,6 @@ fn prop_sign_verify_wrong_msg_fail() {
     }
 }
 
-#[test]
-fn prop_sign_verify_agg() {
-    let rng = &mut make_rng();
-
-    for _ in 0..10 {
-        let mut agg = AggregateSignature::default();
-        let mut inputs = Vec::new();
-        for _ in 0..rng.gen_range(0..5) {
-            let sk = rng.gen::<SecretKey>();
-            let msg = gen_msg(rng);
-            let sig = sk.sign(&msg).unwrap();
-            agg.add(sig);
-            inputs.push((msg, sk.public()));
-        }
-
-        // Verification should work for any order of messages and signatures.
-        agg.0.shuffle(rng);
-        inputs.shuffle(rng);
-
-        agg.verify(inputs.iter().map(|(msg, pk)| (msg.as_slice(), pk)))
-            .unwrap();
-    }
-}
-
-#[test]
-fn prop_sign_verify_agg_fail() {
-    let rng = &mut make_rng();
-
-    for _ in 0..10 {
-        let mut agg = AggregateSignature::default();
-        let mut inputs = Vec::new();
-        // Minimum two signatures so that we can pop something.
-        for _ in 0..=rng.gen_range(2..5) {
-            let sk = rng.gen::<SecretKey>();
-            let msg = gen_msg(rng);
-            let sig = sk.sign(&msg).unwrap();
-            agg.add(sig);
-            inputs.push((msg, sk.public()));
-        }
-        // Do something to mess it up.
-        if rng.gen_bool(0.5) {
-            inputs.pop();
-        } else {
-            agg.0.pop();
-        }
-        agg.verify(inputs.iter().map(|(msg, pk)| (msg.as_slice(), pk)))
-            .unwrap_err();
-    }
-}
-
 /// Test vectors from <https://web3js.readthedocs.io/en/v1.2.0/web3-eth-accounts.html#eth-accounts-signtransaction>
 ///
 /// The same example has been used in <https://github.com/gakonst/ethers-rs/blob/ba00f549/ethers-signers/src/wallet/private_key.rs#L197>

node/libs/protobuf/src/proto_fmt.rs

@@ -285,6 +285,33 @@ pub fn read_optional<T: ProtoFmt>(field: &Option<T::Proto>) -> anyhow::Result<Op
     field.as_ref().map(ProtoFmt::read).transpose()
 }
 
+/// Parses a repeated proto struct into a map.
+///
+/// The method assumes that we have a `BTreeMap<K, V>` field that we serialized
+/// as `Vec<T>` into protobuf, where `T` consists of a `K::Proto` and `V::Proto`
+/// field. The `k` and `v` function are getters to project `T` into the key and
+/// value protobuf components, which are individually decoded into `K` and `V`.
+pub fn read_map<T, K, V>(
+    items: &[T],
+    k: impl Fn(&T) -> &Option<K::Proto>,
+    v: impl Fn(&T) -> &Option<V::Proto>,
+) -> anyhow::Result<BTreeMap<K, V>>
+where
+    K: ProtoFmt + Ord,
+    V: ProtoFmt,
+{
+    let items: Vec<(K, V)> = items
+        .iter()
+        .map(|item| {
+            let k = read_required(k(item)).context("key")?;
+            let v = read_required(v(item)).context("value")?;
+            Ok((k, v))
+        })
+        .collect::<anyhow::Result<Vec<_>>>()?;
+
+    Ok(items.into_iter().collect())
+}
+
 /// Extracts a required field.
 pub fn required<T>(field: &Option<T>) -> anyhow::Result<&T> {
     field.as_ref().context("missing")

node/libs/roles/src/attester/conv.rs

@@ -1,12 +1,12 @@
 use super::{
-    AggregateSignature, Batch, BatchNumber, BatchQC, Msg, MsgHash, PublicKey, Signature, Signed,
-    Signers, WeightedAttester,
+    AggregateSignature, Batch, BatchNumber, BatchQC, Msg, MsgHash, MultiSig, 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;
-use zksync_protobuf::{read_required, required, ProtoFmt};
+use zksync_protobuf::{read_map, read_required, required, ProtoFmt};
 
 impl ProtoFmt for Batch {
     type Proto = proto::Batch;
@@ -118,12 +118,12 @@ impl ProtoFmt for AggregateSignature {
     type Proto = proto::AggregateSignature;
 
     fn read(r: &Self::Proto) -> anyhow::Result<Self> {
-        Ok(Self(ByteFmt::decode(required(&r.secp256k1)?)?))
+        Ok(Self(ByteFmt::decode(required(&r.bls12_381)?)?))
     }
 
     fn build(&self) -> Self::Proto {
         Self::Proto {
-            secp256k1: Some(self.0.encode()),
+            bls12_381: Some(self.0.encode()),
         }
     }
 }
@@ -148,16 +148,23 @@ impl ProtoFmt for BatchQC {
     fn read(r: &Self::Proto) -> anyhow::Result<Self> {
         Ok(Self {
             message: read_required(&r.msg).context("message")?,
-            signers: read_required(&r.signers).context("signers")?,
-            signature: read_required(&r.sig).context("signature")?,
+            signatures: MultiSig(
+                read_map(&r.signatures, |s| &s.key, |s| &s.sig).context("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(),
         }
     }
 }