Merge pull request #266 from str4d/group-trait-updates

Updates to group traits

bellman/src/groth16/tests/dummy_engine.rs

@@ -399,7 +399,7 @@ impl PrimeGroup for Fr {}
 impl CofactorGroup for Fr {
     type Subgroup = Fr;
 
-    fn mul_by_cofactor(&self) -> Self::Subgroup {
+    fn clear_cofactor(&self) -> Self::Subgroup {
         *self
     }
 
@@ -417,10 +417,6 @@ impl Curve for Fr {
 }
 
 impl WnafGroup for Fr {
-    fn recommended_wnaf_for_scalar(_: &Self::Scalar) -> usize {
-        3
-    }
-
     fn recommended_wnaf_for_num_scalars(_: usize) -> usize {
         3
     }

group/src/cofactor.rs

@@ -17,10 +17,16 @@ pub trait CofactorGroup:
     /// If `Self` implements `PrimeGroup`, then `Self::Subgroup` may be `Self`.
     type Subgroup: PrimeGroup<Scalar = Self::Scalar> + Into<Self>;
 
-    /// Returns `[h] self`, where `h` is the cofactor of the group.
+    /// Maps `self` to the prime-order subgroup by multiplying this element by some
+    /// `k`-multiple of the cofactor.
+    ///
+    /// The value `k` does not vary between inputs for a given implementation, but may
+    /// vary between different implementations of `CofactorGroup` because some groups have
+    /// more efficient methods of clearing the cofactor when `k` is allowed to be
+    /// different than `1`.
     ///
     /// If `Self` implements [`PrimeGroup`], this returns `self`.
-    fn mul_by_cofactor(&self) -> Self::Subgroup;
+    fn clear_cofactor(&self) -> Self::Subgroup;
 
     /// Returns `self` if it is contained in the prime-order subgroup.
     ///
@@ -33,7 +39,7 @@ pub trait CofactorGroup:
     /// - `true` if `self` is in the torsion subgroup.
     /// - `false` if `self` is not in the torsion subgroup.
     fn is_small_order(&self) -> Choice {
-        self.mul_by_cofactor().is_identity()
+        self.clear_cofactor().is_identity()
     }
 
     /// Determines if this element is "torsion free", i.e., is contained in the

group/src/lib.rs

@@ -62,7 +62,9 @@ pub trait Group:
     /// Scalars modulo the order of this group's scalar field.
     type Scalar: PrimeField;
 
-    /// Returns an element chosen uniformly at random using a user-provided RNG.
+    /// Returns an element chosen uniformly at random from this group.
+    ///
+    /// This function is non-deterministic, and samples from the user-provided RNG.
     fn random<R: RngCore + ?Sized>(rng: &mut R) -> Self;
 
     /// Returns the additive identity, also known as the "neutral element".

group/src/wnaf.rs

@@ -6,10 +6,6 @@ use super::Group;
 
 /// Extension trait on a [`Group`] that provides helpers used by [`Wnaf`].
 pub trait WnafGroup: Group {
-    /// Recommends a wNAF window table size given a scalar. Always returns a number
-    /// between 2 and 22, inclusive.
-    fn recommended_wnaf_for_scalar(scalar: &Self::Scalar) -> usize;
-
     /// Recommends a wNAF window size given the number of scalars you intend to multiply
     /// a base by. Always returns a number between 2 and 22, inclusive.
     fn recommended_wnaf_for_num_scalars(num_scalars: usize) -> usize;
@@ -154,8 +150,8 @@ impl<G: WnafGroup> 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 Group>::Scalar) -> Wnaf<usize, &mut Vec<G>, &[i64]> {
-        // Compute the appropriate window size for the scalar.
+        // We hard-code a window size of 4.
-        let window_size = G::recommended_wnaf_for_scalar(&scalar);
+        let window_size = 4;
 
         // Compute the wNAF form of the scalar.
         wnaf_form(&mut self.scalar, scalar.to_repr(), window_size);

pairing/src/bls12_381/ec.rs

@@ -1024,7 +1024,9 @@ macro_rules! curve_impl {
         impl CofactorGroup for $projective {
             type Subgroup = $subgroup;
 
-            fn mul_by_cofactor(&self) -> Self::Subgroup {
+            fn clear_cofactor(&self) -> Self::Subgroup {
+                // This implementation uses the cofactor directly, and differs from the
+                // bls12_381 crate which uses a multiple of the cofactor.
                 $subgroup($affine::from(*self).scale_by_cofactor().into())
             }
 
@@ -1095,12 +1097,6 @@ macro_rules! curve_impl {
         }
 
         impl WnafGroup for $projective {
-            fn recommended_wnaf_for_scalar(_: &Self::Scalar) -> usize {
-                Self::empirical_recommended_wnaf_for_scalar(
-                    <Self::Scalar as PrimeField>::NUM_BITS as usize,
-                )
-            }
-
             fn recommended_wnaf_for_num_scalars(num_scalars: usize) -> usize {
                 Self::empirical_recommended_wnaf_for_num_scalars(num_scalars)
             }
@@ -1452,16 +1448,6 @@ pub mod g1 {
     }
 
     impl G1 {
-        fn empirical_recommended_wnaf_for_scalar(num_bits: usize) -> usize {
-            if num_bits >= 130 {
-                4
-            } else if num_bits >= 34 {
-                3
-            } else {
-                2
-            }
-        }
-
         fn empirical_recommended_wnaf_for_num_scalars(num_scalars: usize) -> usize {
             const RECOMMENDATIONS: [usize; 12] =
                 [1, 3, 7, 20, 43, 120, 273, 563, 1630, 3128, 7933, 62569];
@@ -2084,16 +2070,6 @@ pub mod g2 {
     }
 
     impl G2 {
-        fn empirical_recommended_wnaf_for_scalar(num_bits: usize) -> usize {
-            if num_bits >= 103 {
-                4
-            } else if num_bits >= 37 {
-                3
-            } else {
-                2
-            }
-        }
-
         fn empirical_recommended_wnaf_for_num_scalars(num_scalars: usize) -> usize {
             const RECOMMENDATIONS: [usize; 11] =
                 [1, 3, 8, 20, 47, 126, 260, 826, 1501, 4555, 84071];