Add a batch verifier to multiexp, along with constant time variants

Saves ~8% during FROST key gen, even with dropping a vartime for a 
constant time (as needed to be secure), as the new batch verifier is 
used where batch verification previously wasn't. The new multiexp API 
itself also offered a very slight performance boost, which may solely be 
a measurement error.

Handles most of https://github.com/serai-dex/serai/issues/10. The blame 
function isn't binary searched nor randomly sorted yet.

coins/monero/src/frost.rs

@@ -7,7 +7,6 @@ use blake2::{digest::Update, Digest, Blake2b512};
 
 use curve25519_dalek::{
   constants::ED25519_BASEPOINT_TABLE as DTable,
-  traits::VartimeMultiscalarMul,
   scalar::Scalar as DScalar,
   edwards::EdwardsPoint as DPoint
 };
@@ -56,8 +55,8 @@ impl Curve for Ed25519 {
     &dfg::ED25519_BASEPOINT_TABLE
   }
 
-  fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G {
-    dfg::EdwardsPoint(DPoint::vartime_multiscalar_mul(scalars, points))
+  fn little_endian() -> bool {
+    true
   }
 
   // This, as used by CLSAG, will already be a keccak256 hash

coins/monero/tests/frost.rs

@@ -62,7 +62,7 @@ pub fn generate_keys() -> (HashMap<u16, MultisigKeys<Ed25519>>, Scalar) {
       }
       our_secret_shares.insert(*l, shares[&i].clone());
     }
-    keys.insert(*i, machine.complete(our_secret_shares).unwrap().clone());
+    keys.insert(*i, machine.complete(&mut OsRng, our_secret_shares).unwrap().clone());
   }
 
   let mut group_private = Scalar::zero();

crypto/frost/Cargo.toml

@@ -17,7 +17,7 @@ group = "0.11"
 blake2 = "0.10"
 transcript = { path = "../transcript" }
 
-multiexp = { path = "../multiexp" }
+multiexp = { path = "../multiexp", features = ["batch"] }
 
 [dev-dependencies]
 rand = "0.8"

crypto/frost/src/key_gen.rs

@@ -5,6 +5,8 @@ use rand_core::{RngCore, CryptoRng};
 
 use ff::{Field, PrimeField};
 
+use multiexp::{multiexp_vartime, BatchVerifier};
+
 use crate::{
   Curve, MultisigParams, MultisigKeys, FrostError,
   schnorr::{self, SchnorrSignature},
@@ -122,13 +124,7 @@ fn verify_r1<R: RngCore + CryptoRng, C: Curve>(
     commitments.insert(l, these_commitments);
   }
 
-  schnorr::batch_verify(rng, &signatures).map_err(
-    |l| if l == 0 {
-      FrostError::InternalError("batch validation is broken".to_string())
-    } else {
-      FrostError::InvalidProofOfKnowledge(l)
-    }
-  )?;
+  schnorr::batch_verify(rng, &signatures).map_err(|l| FrostError::InvalidProofOfKnowledge(l))?;
 
   Ok(commitments)
 }
@@ -192,7 +188,8 @@ fn generate_key_r2<R: RngCore + CryptoRng, C: Curve>(
 /// issue, yet simply confirming protocol completion without issue is enough to confirm the same
 /// key was generated as long as a lack of duplicated commitments was also confirmed when they were
 /// broadcasted initially
-fn complete_r2<C: Curve>(
+fn complete_r2<R: RngCore + CryptoRng, C: Curve>(
+  rng: &mut R,
   params: MultisigParams,
   share: C::F,
   commitments: HashMap<u16, Vec<C::G>>,
@@ -211,6 +208,7 @@ fn complete_r2<C: Curve>(
     shares.insert(l, C::F_from_slice(&share).map_err(|_| FrostError::InvalidShare(params.i()))?);
   }
 
+  let mut batch = BatchVerifier::new(shares.len(), C::little_endian());
   for (l, share) in &shares {
     if *l == params.i() {
       continue;
@@ -218,16 +216,18 @@ fn complete_r2<C: Curve>(
 
     let i_scalar = C::F::from(params.i.into());
     let mut exp = C::F::one();
-    let mut exps = Vec::with_capacity(usize::from(params.t()));
-    for _ in 0 .. params.t() {
-      exps.push(exp);
+    let mut values = Vec::with_capacity(usize::from(params.t()) + 1);
+    for lt in 0 .. params.t() {
+      values.push((exp, commitments[&l][usize::from(lt)]));
       exp *= i_scalar;
     }
+    values.push((-*share, C::generator()));
 
-    // Doesn't use multiexp_vartime with -shares[l] due to not being able to push to commitments
-    if C::multiexp_vartime(&exps, &commitments[&l]) != (C::generator_table() * *share) {
-      Err(FrostError::InvalidCommitment(*l))?;
-    }
+    batch.queue(rng, *l, values);
+  }
+
+  if !batch.verify() {
+    Err(FrostError::InvalidCommitment(batch.blame_vartime().unwrap()))?;
   }
 
   // TODO: Clear the original share
@@ -239,19 +239,18 @@ fn complete_r2<C: Curve>(
 
   let mut verification_shares = HashMap::new();
   for l in 1 ..= params.n() {
-    let mut exps = vec![];
-    let mut cs = vec![];
+    let mut values = vec![];
     for i in 1 ..= params.n() {
       for j in 0 .. params.t() {
         let mut exp = C::F::one();
         for _ in 0 .. j {
           exp *= C::F::from(u64::try_from(l).unwrap());
         }
-        exps.push(exp);
-        cs.push(commitments[&i][usize::from(j)]);
+        values.push((exp, commitments[&i][usize::from(j)]));
       }
     }
-    verification_shares.insert(l, C::multiexp_vartime(&exps, &cs));
+    // Doesn't do a unified multiexp due to needing individual verification shares
+    verification_shares.insert(l, multiexp_vartime(values, C::little_endian()));
   }
   debug_assert_eq!(C::generator_table() * secret_share, verification_shares[&params.i()]);
 
@@ -362,8 +361,9 @@ impl<C: Curve> StateMachine<C> {
   /// group's public key, while setting a valid secret share inside the machine. > t participants
   /// must report completion without issue before this key can be considered usable, yet you should
   /// wait for all participants to report as such
-  pub fn complete(
+  pub fn complete<R: RngCore + CryptoRng>(
     &mut self,
+    rng: &mut R,
     shares: HashMap<u16, Vec<u8>>,
   ) -> Result<MultisigKeys<C>, FrostError> {
     if self.state != State::GeneratedSecretShares {
@@ -371,6 +371,7 @@ impl<C: Curve> StateMachine<C> {
     }
 
     let keys = complete_r2(
+      rng,
       self.params,
       self.secret.take().unwrap(),
       self.commitments.take().unwrap(),

crypto/frost/src/lib.rs

@@ -4,9 +4,7 @@ use std::collections::HashMap;
 use thiserror::Error;
 
 use ff::{Field, PrimeField};
-use group::{Group, GroupOps, ScalarMul};
-
-pub use multiexp::multiexp_vartime;
+use group::{Group, GroupOps};
 
 mod schnorr;
 
@@ -38,7 +36,7 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug {
   // This is available via G::Scalar yet `C::G::Scalar` is ambiguous, forcing horrific accesses
   type F: PrimeField;
   /// Group element type
-  type G: Group + GroupOps + ScalarMul<Self::F>;
+  type G: Group<Scalar = Self::F> + GroupOps;
   /// Precomputed table type
   type T: Mul<Self::F, Output = Self::G>;
 
@@ -57,12 +55,8 @@ pub trait Curve: Clone + Copy + PartialEq + Eq + Debug {
   /// If there isn't a precomputed table available, the generator itself should be used
   fn generator_table() -> Self::T;
 
-  /// Multiexponentation function, presumably Straus or Pippenger
-  /// This library does forward an implementation of Straus which should increase key generation
-  /// performance by around 4x, also named multiexp_vartime, with a similar API. However, if a more
-  /// performant implementation is available, that should be used instead
-  // This could also be written as -> Option<C::G> with None for not implemented
-  fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G;
+  /// If little endian is used for the scalar field's Repr
+  fn little_endian() -> bool;
 
   /// Hash the message as needed to calculate the binding factor
   /// H3 from the IETF draft

crypto/frost/src/schnorr.rs

@@ -3,6 +3,8 @@ use rand_core::{RngCore, CryptoRng};
 use ff::Field;
 use group::Group;
 
+use multiexp::BatchVerifier;
+
 use crate::Curve;
 
 #[allow(non_snake_case)]
@@ -44,39 +46,25 @@ pub(crate) fn batch_verify<C: Curve, R: RngCore + CryptoRng>(
   rng: &mut R,
   triplets: &[(u16, C::G, C::F, SchnorrSignature<C>)]
 ) -> Result<(), u16> {
-  let mut first = true;
-  let mut scalars = Vec::with_capacity(triplets.len() * 3);
-  let mut points = Vec::with_capacity(triplets.len() * 3);
+  let mut values = [(C::F::one(), C::G::generator()); 3];
+  let mut batch = BatchVerifier::new(triplets.len() * 3, C::little_endian());
   for triple in triplets {
-    let mut u = C::F::one();
-    if !first {
-      u = C::F::random(&mut *rng);
-    }
+    // R
+    values[0].1 = triple.3.R;
+    // cA
+    values[1] = (triple.2, triple.1);
+    // -sG
+    values[2].0 = -triple.3.s;
 
-    // uR
-    scalars.push(u);
-    points.push(triple.3.R);
-
-    // -usG
-    scalars.push(-triple.3.s * u);
-    points.push(C::generator());
-
-    // ucA
-    scalars.push(if first { first = false; triple.2 } else { triple.2 * u});
-    points.push(triple.1);
+    batch.queue(rng, triple.0, values);
   }
 
   // s = r + ca
   // sG == R + cA
   // R + cA - sG == 0
-  if C::multiexp_vartime(&scalars, &points) == C::G::identity() {
+  if batch.verify_vartime() {
     Ok(())
   } else {
-    for triple in triplets {
-      if !verify::<C>(triple.1, triple.2, &triple.3) {
-        Err(triple.0)?;
-      }
-    }
-    Err(0)
+    Err(batch.blame_vartime().unwrap())
   }
 }

crypto/frost/src/tests/literal/secp256k1.rs

@@ -13,7 +13,7 @@ use k256::{
   ProjectivePoint
 };
 
-use crate::{CurveError, Curve, multiexp_vartime, algorithm::Hram, tests::curve::test_curve};
+use crate::{CurveError, Curve, algorithm::Hram, tests::curve::test_curve};
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub struct Secp256k1;
@@ -38,8 +38,8 @@ impl Curve for Secp256k1 {
     Self::G::GENERATOR
   }
 
-  fn multiexp_vartime(scalars: &[Self::F], points: &[Self::G]) -> Self::G {
-    multiexp_vartime(scalars, points, false)
+  fn little_endian() -> bool {
+    false
   }
 
   // The IETF draft doesn't specify a secp256k1 ciphersuite

crypto/frost/src/tests/mod.rs

@@ -82,7 +82,7 @@ pub fn key_gen<R: RngCore + CryptoRng, C: Curve>(
       }
       our_secret_shares.insert(*l, shares[&i].clone());
     }
-    let these_keys = machine.complete(our_secret_shares).unwrap();
+    let these_keys = machine.complete(rng, our_secret_shares).unwrap();
 
     // Verify the verification_shares are agreed upon
     if verification_shares.is_none() {

crypto/multiexp/Cargo.toml

@@ -7,5 +7,9 @@ authors = ["Luke Parker <lukeparker5132@gmail.com>"]
 edition = "2021"
 
 [dependencies]
-ff = "0.11"
 group = "0.11"
+
+rand_core = { version = "0.6", optional = true }
+
+[features]
+batch = ["rand_core"]

crypto/multiexp/src/lib.rs

@@ -1,43 +1,52 @@
-use ff::PrimeField;
-use group::{Group, GroupEncoding, ScalarMul};
+use group::{ff::PrimeField, Group};
 
-// An implementation of Straus, with a extremely minimal API that lets us add other algorithms in
-// the future. Takes in a list of scalars and points with a boolean for if the scalars are little
-// endian encoded or not
-pub fn multiexp_vartime<F: PrimeField, G: Group + GroupEncoding + ScalarMul<F>>(
-  scalars: &[F],
-  points: &[G],
-  little: bool
-) -> G {
+#[cfg(feature = "batch")]
+use group::ff::Field;
+#[cfg(feature = "batch")]
+use rand_core::{RngCore, CryptoRng};
+
+fn prep<
+  G: Group,
+  I: IntoIterator<Item = (G::Scalar, G)>
+>(pairs: I, little: bool) -> (Vec<Vec<u8>>, Vec<[G; 16]>) {
+  let mut nibbles = vec![];
   let mut tables = vec![];
-  // dalek uses 8 in their impl, along with a carry scheme where values are [-8, 8)
-  // Moving to a similar system here did save a marginal amount, yet not one significant enough for
-  // its pain (as some fields do have scalars which can have their top bit set, a scenario dalek
-  // assumes is never true)
-  tables.resize(points.len(), [G::identity(); 16]);
-  for p in 0 .. points.len() {
+  for pair in pairs.into_iter() {
+    let p = nibbles.len();
+    nibbles.push(vec![]);
+    {
+      let mut repr = pair.0.to_repr();
+      let bytes = repr.as_mut();
+      if !little {
+        bytes.reverse();
+      }
+
+      nibbles[p].resize(bytes.len() * 2, 0);
+      for i in 0 .. bytes.len() {
+        nibbles[p][i * 2] = bytes[i] & 0b1111;
+        nibbles[p][(i * 2) + 1] = (bytes[i] >> 4) & 0b1111;
+      }
+    }
+
+    tables.push([G::identity(); 16]);
     let mut accum = G::identity();
     for i in 1 .. 16 {
-      accum += points[p];
+      accum += pair.1;
       tables[p][i] = accum;
     }
   }
 
-  let mut nibbles = vec![];
-  nibbles.resize(scalars.len(), vec![]);
-  for s in 0 .. scalars.len() {
-    let mut repr = scalars[s].to_repr();
-    let bytes = repr.as_mut();
-    if !little {
-      bytes.reverse();
-    }
+  (nibbles, tables)
+}
 
-    nibbles[s].resize(bytes.len() * 2, 0);
-    for i in 0 .. bytes.len() {
-      nibbles[s][i * 2] = bytes[i] & 0b1111;
-      nibbles[s][(i * 2) + 1] = (bytes[i] >> 4) & 0b1111;
-    }
-  }
+// An implementation of Straus, with a extremely minimal API that lets us add other algorithms in
+// the future. Takes in an iterator of scalars and points with a boolean for if the scalars are
+// little endian encoded in their Reprs or not
+pub fn multiexp<
+  G: Group,
+  I: IntoIterator<Item = (G::Scalar, G)>
+>(pairs: I, little: bool) -> G {
+  let (nibbles, tables) = prep(pairs, little);
 
   let mut res = G::identity();
   for b in (0 .. nibbles[0].len()).rev() {
@@ -45,7 +54,26 @@ pub fn multiexp_vartime<F: PrimeField, G: Group + GroupEncoding + ScalarMul<F>>(
       res = res.double();
     }
 
-    for s in 0 .. scalars.len() {
+    for s in 0 .. tables.len() {
+      res += tables[s][nibbles[s][b] as usize];
+    }
+  }
+  res
+}
+
+pub fn multiexp_vartime<
+  G: Group,
+  I: IntoIterator<Item = (G::Scalar, G)>
+>(pairs: I, little: bool) -> G {
+  let (nibbles, tables) = prep(pairs, little);
+
+  let mut res = G::identity();
+  for b in (0 .. nibbles[0].len()).rev() {
+    for _ in 0 .. 4 {
+      res = res.double();
+    }
+
+    for s in 0 .. tables.len() {
       if nibbles[s][b] != 0 {
         res += tables[s][nibbles[s][b] as usize];
       }
@@ -53,3 +81,52 @@ pub fn multiexp_vartime<F: PrimeField, G: Group + GroupEncoding + ScalarMul<F>>(
   }
   res
 }
+
+#[cfg(feature = "batch")]
+pub struct BatchVerifier<Id: Copy, G: Group>(Vec<(Id, Vec<(G::Scalar, G)>)>, bool);
+
+#[cfg(feature = "batch")]
+impl<Id: Copy, G: Group> BatchVerifier<Id, G> {
+  pub fn new(capacity: usize, endian: bool) -> BatchVerifier<Id, G> {
+    BatchVerifier(Vec::with_capacity(capacity), endian)
+  }
+
+  pub fn queue<
+    R: RngCore + CryptoRng,
+    I: IntoIterator<Item = (G::Scalar, G)>
+  >(&mut self, rng: &mut R, id: Id, pairs: I) {
+    // Define a unique scalar factor for this set of variables so individual items can't overlap
+    let u = if self.0.len() == 0 {
+      G::Scalar::one()
+    } else {
+      G::Scalar::random(rng)
+    };
+    self.0.push((id, pairs.into_iter().map(|(scalar, point)| (scalar * u, point)).collect()));
+  }
+
+  pub fn verify(&self) -> bool {
+    multiexp(
+      self.0.iter().flat_map(|sets| sets.1.iter()).cloned(),
+      self.1
+    ).is_identity().into()
+  }
+
+  pub fn verify_vartime(&self) -> bool {
+    multiexp_vartime(
+      self.0.iter().flat_map(|sets| sets.1.iter()).cloned(),
+      self.1
+    ).is_identity().into()
+  }
+
+  // Solely has a vartime variant as there shouldn't be any reason for this to not be vartime, yet
+  // we should explicitly label vartime software as vartime
+  // TODO: Binary search, or at least randomly sort
+  pub fn blame_vartime(&self) -> Option<Id> {
+    for value in &self.0 {
+      if !bool::from(multiexp_vartime(value.1.clone(), self.1).is_identity()) {
+        return Some(value.0);
+      }
+    }
+    None
+  }
+}