From ec8877825883a7b28dfaa13dbd31da2c69d1a99d Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 14 May 2020 23:16:07 +1200
Subject: [PATCH] group: Add scalar multiplication bounds to Group

The Scalar associated type is moved from CurveProjective to Group.
---
 bellman/src/domain.rs                     |  2 +-
 bellman/src/groth16/prover.rs             |  4 +-
 bellman/src/groth16/tests/dummy_engine.rs |  8 +--
 group/src/lib.rs                          | 25 +++++++--
 group/src/wnaf.rs                         |  9 +--
 pairing/benches/bls12_381/ec.rs           |  8 +--
 pairing/src/bls12_381/ec.rs               | 68 ++++++++++++++++-------
 pairing/src/lib.rs                        | 10 +++-
 pairing/src/tests/engine.rs               | 12 ++--
 9 files changed, 90 insertions(+), 56 deletions(-)

diff --git a/bellman/src/domain.rs b/bellman/src/domain.rs
index 748a15a75..4e24f18b0 100644
--- a/bellman/src/domain.rs
+++ b/bellman/src/domain.rs
@@ -221,7 +221,7 @@ impl<G: CurveProjective, E: ScalarEngine<Fr = G::Scalar>> Group<E> for Point<G>
         Point(G::identity())
     }
     fn group_mul_assign(&mut self, by: &G::Scalar) {
-        self.0.mul_assign(by.to_repr());
+        self.0.mul_assign(by);
     }
     fn group_add_assign(&mut self, other: &Self) {
         self.0.add_assign(&other.0);
diff --git a/bellman/src/groth16/prover.rs b/bellman/src/groth16/prover.rs
index 97707fb09..036dda800 100644
--- a/bellman/src/groth16/prover.rs
+++ b/bellman/src/groth16/prover.rs
@@ -317,7 +317,7 @@ where
     let mut a_answer = a_inputs.wait()?;
     AddAssign::<&E::G1>::add_assign(&mut a_answer, &a_aux.wait()?);
     AddAssign::<&E::G1>::add_assign(&mut g_a, &a_answer);
-    a_answer.mul_assign(s);
+    MulAssign::<E::Fr>::mul_assign(&mut a_answer, s);
     AddAssign::<&E::G1>::add_assign(&mut g_c, &a_answer);
 
     let mut b1_answer: E::G1 = b_g1_inputs.wait()?;
@@ -326,7 +326,7 @@ where
     AddAssign::<&E::G2>::add_assign(&mut b2_answer, &b_g2_aux.wait()?);
 
     AddAssign::<&E::G2>::add_assign(&mut g_b, &b2_answer);
-    b1_answer.mul_assign(r);
+    MulAssign::<E::Fr>::mul_assign(&mut b1_answer, r);
     AddAssign::<&E::G1>::add_assign(&mut g_c, &b1_answer);
     AddAssign::<&E::G1>::add_assign(&mut g_c, &h.wait()?);
     AddAssign::<&E::G1>::add_assign(&mut g_c, &l.wait()?);
diff --git a/bellman/src/groth16/tests/dummy_engine.rs b/bellman/src/groth16/tests/dummy_engine.rs
index e7b6b17da..982377227 100644
--- a/bellman/src/groth16/tests/dummy_engine.rs
+++ b/bellman/src/groth16/tests/dummy_engine.rs
@@ -367,6 +367,7 @@ impl Engine for DummyEngine {
 
 impl Group for Fr {
     type Subgroup = Fr;
+    type Scalar = Fr;
 
     fn random<R: RngCore + ?Sized>(rng: &mut R) -> Self {
         <Fr as Field>::random(rng)
@@ -394,7 +395,6 @@ impl PrimeGroup for Fr {}
 impl CurveProjective for Fr {
     type Affine = Fr;
     type Base = Fr;
-    type Scalar = Fr;
 
     fn batch_normalization(_: &mut [Self]) {}
 
@@ -402,12 +402,6 @@ impl CurveProjective for Fr {
         true
     }
 
-    fn mul_assign<S: Into<<Self::Scalar as PrimeField>::Repr>>(&mut self, other: S) {
-        let tmp = Fr::from_repr(other.into()).unwrap();
-
-        MulAssign::mul_assign(self, &tmp);
-    }
-
     fn into_affine(&self) -> Fr {
         *self
     }
diff --git a/group/src/lib.rs b/group/src/lib.rs
index 8f711c44c..b31cdedeb 100644
--- a/group/src/lib.rs
+++ b/group/src/lib.rs
@@ -6,7 +6,7 @@ use rand::RngCore;
 use std::error::Error;
 use std::fmt;
 use std::iter::Sum;
-use std::ops::{Add, AddAssign, Neg, Sub, SubAssign};
+use std::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign};
 
 pub mod tests;
 
@@ -28,6 +28,20 @@ impl<T, Rhs, Output> GroupOps<Rhs, Output> for T where
 pub trait GroupOpsOwned<Rhs = Self, Output = Self>: for<'r> GroupOps<&'r Rhs, Output> {}
 impl<T, Rhs, Output> GroupOpsOwned<Rhs, Output> for T where T: for<'r> GroupOps<&'r Rhs, Output> {}
 
+/// A helper trait for types implementing group scalar multiplication.
+pub trait ScalarMul<Rhs, Output = Self>: Mul<Rhs, Output = Output> + MulAssign<Rhs> {}
+
+impl<T, Rhs, Output> ScalarMul<Rhs, Output> for T where T: Mul<Rhs, Output = Output> + MulAssign<Rhs>
+{}
+
+/// A helper trait for references implementing group scalar multiplication.
+///
+/// This trait, in combination with `ScalarMul`, is necessary to address type constraint
+/// issues in `pairing::Engine` (specifically, to ensure that [`ff::ScalarEngine::Fr`] is
+/// correctly constrained to implement these traits required by [`Group::Scalar`]).
+pub trait ScalarMulOwned<Rhs, Output = Self>: for<'r> ScalarMul<&'r Rhs, Output> {}
+impl<T, Rhs, Output> ScalarMulOwned<Rhs, Output> for T where T: for<'r> ScalarMul<&'r Rhs, Output> {}
+
 /// This trait represents an element of a cryptographic group.
 pub trait Group:
     Clone
@@ -46,11 +60,16 @@ pub trait Group:
     + GroupOpsOwned
     + GroupOps<<Self as Group>::Subgroup>
     + GroupOpsOwned<<Self as Group>::Subgroup>
+    + ScalarMul<<Self as Group>::Scalar>
+    + ScalarMulOwned<<Self as Group>::Scalar>
 {
     /// The large prime-order subgroup in which cryptographic operations are performed.
     /// If `Self` implements `PrimeGroup`, then `Self::Subgroup` may be `Self`.
     type Subgroup: PrimeGroup;
 
+    /// Scalars modulo the order of [`Group::Subgroup`].
+    type Scalar: PrimeField;
+
     /// Returns an element chosen uniformly at random using a user-provided RNG.
     fn random<R: RngCore + ?Sized>(rng: &mut R) -> Self;
 
@@ -78,7 +97,6 @@ pub trait CurveProjective:
     + GroupOps<<Self as CurveProjective>::Affine>
     + GroupOpsOwned<<Self as CurveProjective>::Affine>
 {
-    type Scalar: PrimeField;
     type Base: Field;
     type Affine: CurveAffine<Projective = Self, Scalar = Self::Scalar>;
 
@@ -90,9 +108,6 @@ pub trait CurveProjective:
     /// cheap affine conversion is possible.
     fn is_normalized(&self) -> bool;
 
-    /// Performs scalar multiplication of this element.
-    fn mul_assign<S: Into<<Self::Scalar as PrimeField>::Repr>>(&mut self, other: S);
-
     /// Converts this element into its affine representation.
     fn into_affine(&self) -> Self::Affine;
 
diff --git a/group/src/wnaf.rs b/group/src/wnaf.rs
index 79efd021a..0ee46cafc 100644
--- a/group/src/wnaf.rs
+++ b/group/src/wnaf.rs
@@ -2,7 +2,7 @@ use byteorder::{ByteOrder, LittleEndian};
 use ff::PrimeField;
 use std::iter;
 
-use super::CurveProjective;
+use super::{CurveProjective, Group};
 
 /// Replaces the contents of `table` with a w-NAF window table for the given window size.
 pub(crate) fn wnaf_table<G: CurveProjective>(table: &mut Vec<G>, mut base: G, window: usize) {
@@ -140,10 +140,7 @@ impl<G: CurveProjective> Wnaf<(), Vec<G>, Vec<i64>> {
 
     /// Given a scalar, compute its wNAF representation and return a `Wnaf` object that can perform
     /// exponentiations with `.base(..)`.
-    pub fn scalar(
-        &mut self,
-        scalar: &<G as CurveProjective>::Scalar,
-    ) -> Wnaf<usize, &mut Vec<G>, &[i64]> {
+    pub fn scalar(&mut self, scalar: &<G as Group>::Scalar) -> Wnaf<usize, &mut Vec<G>, &[i64]> {
         // Compute the appropriate window size for the scalar.
         let window_size = G::recommended_wnaf_for_scalar(&scalar);
 
@@ -198,7 +195,7 @@ impl<B, S: AsRef<[i64]>> Wnaf<usize, B, S> {
 
 impl<B, S: AsMut<Vec<i64>>> Wnaf<usize, B, S> {
     /// Performs exponentiation given a scalar.
-    pub fn scalar<G: CurveProjective>(&mut self, scalar: &<G as CurveProjective>::Scalar) -> G
+    pub fn scalar<G: CurveProjective>(&mut self, scalar: &<G as Group>::Scalar) -> G
     where
         B: AsRef<[G]>,
     {
diff --git a/pairing/benches/bls12_381/ec.rs b/pairing/benches/bls12_381/ec.rs
index d5ece1273..41c2b4cb1 100644
--- a/pairing/benches/bls12_381/ec.rs
+++ b/pairing/benches/bls12_381/ec.rs
@@ -5,7 +5,7 @@ pub(crate) mod g1 {
     use std::ops::AddAssign;
 
     use ff::Field;
-    use group::{CurveProjective, Group};
+    use group::Group;
     use pairing::bls12_381::*;
 
     fn bench_g1_mul_assign(c: &mut Criterion) {
@@ -24,7 +24,7 @@ pub(crate) mod g1 {
         c.bench_function("G1::mul_assign", |b| {
             b.iter(|| {
                 let mut tmp = v[count].0;
-                tmp.mul_assign(v[count].1);
+                tmp *= v[count].1;
                 count = (count + 1) % SAMPLES;
                 tmp
             })
@@ -92,7 +92,7 @@ pub(crate) mod g2 {
     use std::ops::AddAssign;
 
     use ff::Field;
-    use group::{CurveProjective, Group};
+    use group::Group;
     use pairing::bls12_381::*;
 
     fn bench_g2_mul_assign(c: &mut Criterion) {
@@ -111,7 +111,7 @@ pub(crate) mod g2 {
         c.bench_function("G2::mul_assign", |b| {
             b.iter(|| {
                 let mut tmp = v[count].0;
-                tmp.mul_assign(v[count].1);
+                tmp *= v[count].1;
                 count = (count + 1) % SAMPLES;
                 tmp
             })
diff --git a/pairing/src/bls12_381/ec.rs b/pairing/src/bls12_381/ec.rs
index bb236832a..9b88c36df 100644
--- a/pairing/src/bls12_381/ec.rs
+++ b/pairing/src/bls12_381/ec.rs
@@ -520,8 +520,55 @@ macro_rules! curve_impl {
             }
         }
 
+        impl ::std::ops::Mul<<$projective as Group>::Scalar> for $projective {
+            type Output = Self;
+
+            fn mul(mut self, other: <$projective as Group>::Scalar) -> Self {
+                self.mul_assign(&other);
+                self
+            }
+        }
+
+        impl<'r> ::std::ops::Mul<&'r <$projective as Group>::Scalar> for $projective {
+            type Output = Self;
+
+            fn mul(mut self, other: &'r <$projective as Group>::Scalar) -> Self {
+                self.mul_assign(other);
+                self
+            }
+        }
+
+        impl ::std::ops::MulAssign<<$projective as Group>::Scalar> for $projective {
+            fn mul_assign(&mut self, other: <$projective as Group>::Scalar) {
+                self.mul_assign(&other);
+            }
+        }
+
+        impl<'r> ::std::ops::MulAssign<&'r <$projective as Group>::Scalar> for $projective {
+            fn mul_assign(&mut self, other: &'r <$projective as Group>::Scalar) {
+                let mut res = Self::identity();
+
+                let mut found_one = false;
+
+                for i in BitIterator::<u8, _>::new(other.to_repr()) {
+                    if found_one {
+                        res = res.double();
+                    } else {
+                        found_one = i;
+                    }
+
+                    if i {
+                        res.add_assign(&*self);
+                    }
+                }
+
+                *self = res;
+            }
+        }
+
         impl Group for $projective {
             type Subgroup = Self;
+            type Scalar = $scalarfield;
 
             fn random<R: RngCore + ?Sized>(rng: &mut R) -> Self {
                 loop {
@@ -611,7 +658,6 @@ macro_rules! curve_impl {
         impl PrimeGroup for $projective {}
 
         impl CurveProjective for $projective {
-            type Scalar = $scalarfield;
             type Base = $basefield;
             type Affine = $affine;
 
@@ -672,26 +718,6 @@ macro_rules! curve_impl {
                 }
             }
 
-            fn mul_assign<S: Into<<Self::Scalar as PrimeField>::Repr>>(&mut self, other: S) {
-                let mut res = Self::identity();
-
-                let mut found_one = false;
-
-                for i in BitIterator::<u8, _>::new(other.into()) {
-                    if found_one {
-                        res = res.double();
-                    } else {
-                        found_one = i;
-                    }
-
-                    if i {
-                        res.add_assign(&*self);
-                    }
-                }
-
-                *self = res;
-            }
-
             fn into_affine(&self) -> $affine {
                 (*self).into()
             }
diff --git a/pairing/src/lib.rs b/pairing/src/lib.rs
index 341b0d0b7..a89cc6a73 100644
--- a/pairing/src/lib.rs
+++ b/pairing/src/lib.rs
@@ -21,7 +21,7 @@ pub mod tests;
 pub mod bls12_381;
 
 use ff::{Field, PrimeField, ScalarEngine};
-use group::{CurveAffine, CurveProjective, GroupOps, GroupOpsOwned};
+use group::{CurveAffine, CurveProjective, GroupOps, GroupOpsOwned, ScalarMul, ScalarMulOwned};
 use subtle::CtOption;
 
 /// An "engine" is a collection of types (fields, elliptic curve groups, etc.)
@@ -32,7 +32,9 @@ pub trait Engine: ScalarEngine {
     type G1: CurveProjective<Base = Self::Fq, Scalar = Self::Fr, Affine = Self::G1Affine>
         + From<Self::G1Affine>
         + GroupOps<Self::G1Affine>
-        + GroupOpsOwned<Self::G1Affine>;
+        + GroupOpsOwned<Self::G1Affine>
+        + ScalarMul<Self::Fr>
+        + ScalarMulOwned<Self::Fr>;
 
     /// The affine representation of an element in G1.
     type G1Affine: PairingCurveAffine<
@@ -47,7 +49,9 @@ pub trait Engine: ScalarEngine {
     type G2: CurveProjective<Base = Self::Fqe, Scalar = Self::Fr, Affine = Self::G2Affine>
         + From<Self::G2Affine>
         + GroupOps<Self::G2Affine>
-        + GroupOpsOwned<Self::G2Affine>;
+        + GroupOpsOwned<Self::G2Affine>
+        + ScalarMul<Self::Fr>
+        + ScalarMulOwned<Self::Fr>;
 
     /// The affine representation of an element in G2.
     type G2Affine: PairingCurveAffine<
diff --git a/pairing/src/tests/engine.rs b/pairing/src/tests/engine.rs
index 576954389..0a366e06b 100644
--- a/pairing/src/tests/engine.rs
+++ b/pairing/src/tests/engine.rs
@@ -114,23 +114,21 @@ fn random_bilinearity_tests<E: Engine>() {
         let d = E::Fr::random(&mut rng);
 
         let mut ac = a;
-        ac.mul_assign(c);
+        MulAssign::<&E::Fr>::mul_assign(&mut ac, &c);
 
         let mut ad = a;
-        ad.mul_assign(d);
+        MulAssign::<&E::Fr>::mul_assign(&mut ad, &d);
 
         let mut bc = b;
-        bc.mul_assign(c);
+        MulAssign::<&E::Fr>::mul_assign(&mut bc, &c);
 
         let mut bd = b;
-        bd.mul_assign(d);
+        MulAssign::<&E::Fr>::mul_assign(&mut bd, &d);
 
         let acbd = E::pairing(ac, bd);
         let adbc = E::pairing(ad, bc);
 
-        let mut cd = c;
-        cd.mul_assign(&d);
-        let mut cd = cd.to_repr();
+        let mut cd = (c * &d).to_repr();
         <E::Fr as PrimeField>::ReprEndianness::toggle_little_endian(&mut cd);
 
         use byteorder::ByteOrder;