Move common fields into ModelParameters (#365)

Co-authored-by: Pratyush Mishra <pratyushmishra@berkeley.edu>
arkworks-rs · Dec 22, 2021 · 1b44dbb · 1b44dbb
1 parent dabf462
commit 1b44dbb
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 135 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,10 @@
 - [\#345](https://github.com/arkworks-rs/algebra/pull/345) (arc-ec, ark-serialize) Change the serialization format for Twisted Edwards Curves. We now encode the Y coordinate and take the sign bit of the X co-ordinate, the default flag is also now the Positive X value. The old methods for backwards compatibility are located [here](https://github.com/arkworks-rs/algebra/pull/345/files#diff-3621a48bb33f469144044d8d5fc663f767e103132a764812cda6be6c25877494R860)
 - [\#348](https://github.com/arkworks-rs/algebra/pull/348) (arc-ec) Rename `msm:{Fixed,Variable}BaseMSM:multi_scalar_mul` to `msm:{Fixed,Variable}:msm` to avoid redundancy.
 - [\#359](https://github.com/arkworks-rs/algebra/pull/359) (ark-test-templates) Simplify the field and curve test macros.
+- [\#365](https://github.com/arkworks-rs/algebra/pull/365) (arc-ec)
+    - Move `COFACTOR`, `COFACTOR_INV`, and `is_in_correct_subgroup_assuming_on_curve()` from `{SW,TE}ModelParameters` to `ModelParameters`.
+    - Add `mul_bits()` to `AffineCurve` and provide a default implementation of `mul()` using this.
+    - Remove duplicate function `scale_by_cofactor()` from `short_weierstrass_jacobian::GroupAffine` and `twisted_edwards_extended::GroupAffine`
 
 ### Features
 

diff --git a/ec/src/lib.rs b/ec/src/lib.rs
@@ -19,10 +19,9 @@ extern crate derivative;
 #[macro_use]
 extern crate ark_std;
 
-use crate::group::Group;
 use ark_ff::{
     bytes::{FromBytes, ToBytes},
-    fields::{Field, PrimeField, SquareRootField},
+    fields::{BitIteratorBE, Field, PrimeField, SquareRootField},
     UniformRand,
 };
 use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
@@ -156,11 +155,19 @@ pub trait ProjectiveCurve:
     + for<'a> core::iter::Sum<&'a Self>
     + From<<Self as ProjectiveCurve>::Affine>
 {
-    const COFACTOR: &'static [u64];
+    type Parameters: ModelParameters<
+        ScalarField = Self::ScalarField,
+        BaseField = Self::BaseField,
+        Affine = Self::Affine,
+    >;
     type ScalarField: PrimeField + SquareRootField;
     type BaseField: Field;
-    type Affine: AffineCurve<Projective = Self, ScalarField = Self::ScalarField, BaseField = Self::BaseField>
-        + From<Self>
+    type Affine: AffineCurve<
+            Parameters = Self::Parameters,
+            Projective = Self,
+            ScalarField = Self::ScalarField,
+            BaseField = Self::BaseField,
+        > + From<Self>
         + Into<Self>;
 
     /// Returns a fixed generator of unknown exponent.
@@ -252,11 +259,19 @@ pub trait AffineCurve:
     + for<'a> core::iter::Sum<&'a Self>
     + From<<Self as AffineCurve>::Projective>
 {
-    const COFACTOR: &'static [u64];
+    type Parameters: ModelParameters<
+        ScalarField = Self::ScalarField,
+        BaseField = Self::BaseField,
+        Affine = Self,
+    >;
     type ScalarField: PrimeField + SquareRootField + Into<<Self::ScalarField as PrimeField>::BigInt>;
     type BaseField: Field;
-    type Projective: ProjectiveCurve<Affine = Self, ScalarField = Self::ScalarField, BaseField = Self::BaseField>
-        + From<Self>
+    type Projective: ProjectiveCurve<
+            Parameters = Self::Parameters,
+            Affine = Self,
+            ScalarField = Self::ScalarField,
+            BaseField = Self::BaseField,
+        > + From<Self>
         + Into<Self>
         + MulAssign<Self::ScalarField>; // needed due to https://github.com/rust-lang/rust/issues/69640
 
@@ -274,15 +289,34 @@ pub trait AffineCurve:
     /// random group elements from a hash-function or RNG output.
     fn from_random_bytes(bytes: &[u8]) -> Option<Self>;
 
+    /// Multiplies `self` by the scalar represented by `bits`. `bits` must be a
+    /// big-endian bit-wise decomposition of the scalar.
+    fn mul_bits(&self, bits: impl Iterator<Item = bool>) -> Self::Projective {
+        let mut res = Self::Projective::zero();
+        // Skip leading zeros.
+        for i in bits.skip_while(|b| !b) {
+            res.double_in_place();
+            if i {
+                res.add_assign_mixed(&self)
+            }
+        }
+        res
+    }
+
     /// Performs scalar multiplication of this element with mixed addition.
     #[must_use]
-    fn mul<S: Into<<Self::ScalarField as PrimeField>::BigInt>>(&self, other: S)
-        -> Self::Projective;
+    #[inline]
+    fn mul<S: Into<<Self::ScalarField as PrimeField>::BigInt>>(&self, by: S) -> Self::Projective {
+        self.mul_bits(BitIteratorBE::without_leading_zeros(by.into()))
+    }
 
     /// Multiplies this element by the cofactor and output the
     /// resulting projective element.
     #[must_use]
-    fn mul_by_cofactor_to_projective(&self) -> Self::Projective;
+    #[inline]
+    fn mul_by_cofactor_to_projective(&self) -> Self::Projective {
+        self.mul_bits(BitIteratorBE::new(Self::Parameters::COFACTOR))
+    }
 
     /// Multiplies this element by the cofactor.
     #[must_use]
@@ -293,10 +327,12 @@ pub trait AffineCurve:
     /// Multiplies this element by the inverse of the cofactor in
     /// `Self::ScalarField`.
     #[must_use]
-    fn mul_by_cofactor_inv(&self) -> Self;
+    fn mul_by_cofactor_inv(&self) -> Self {
+        self.mul(Self::Parameters::COFACTOR_INV).into()
+    }
 }
 
-impl<C: ProjectiveCurve> Group for C {
+impl<C: ProjectiveCurve> crate::group::Group for C {
     type ScalarField = C::ScalarField;
 
     #[inline]

diff --git a/ec/src/models/mod.rs b/ec/src/models/mod.rs
@@ -1,4 +1,4 @@
-use crate::models::short_weierstrass_jacobian::GroupAffine;
+use crate::AffineCurve;
 use ark_ff::{fields::BitIteratorBE, Field, PrimeField, SquareRootField, Zero};
 
 pub mod bls12;
@@ -9,16 +9,29 @@ pub mod mnt6;
 pub mod short_weierstrass_jacobian;
 pub mod twisted_edwards_extended;
 
-pub trait ModelParameters: Send + Sync + 'static {
+/// Model parameters for an elliptic curve.
+pub trait ModelParameters: Send + Sync + Sized + 'static {
     type BaseField: Field + SquareRootField;
     type ScalarField: PrimeField + SquareRootField + Into<<Self::ScalarField as PrimeField>::BigInt>;
+    type Affine: AffineCurve<BaseField = Self::BaseField, ScalarField = Self::ScalarField>;
+
+    const COFACTOR: &'static [u64];
+    const COFACTOR_INV: Self::ScalarField;
+
+    /// Checks that the current point is in the prime order subgroup given
+    /// the point on the curve.
+    fn is_in_correct_subgroup_assuming_on_curve(item: &Self::Affine) -> bool {
+        item.mul_bits(BitIteratorBE::new(Self::ScalarField::characteristic()))
+            .is_zero()
+    }
 }
 
-pub trait SWModelParameters: ModelParameters {
+/// Model parameters for a Short Weierstrass curve.
+pub trait SWModelParameters:
+    ModelParameters<Affine = short_weierstrass_jacobian::GroupAffine<Self>>
+{
     const COEFF_A: Self::BaseField;
     const COEFF_B: Self::BaseField;
-    const COFACTOR: &'static [u64];
-    const COFACTOR_INV: Self::ScalarField;
     const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);
 
     #[inline(always)]
@@ -37,21 +50,14 @@ pub trait SWModelParameters: ModelParameters {
         }
         *elem
     }
-
-    fn is_in_correct_subgroup_assuming_on_curve(item: &GroupAffine<Self>) -> bool
-    where
-        Self: Sized,
-    {
-        item.mul_bits(BitIteratorBE::new(Self::ScalarField::characteristic()))
-            .is_zero()
-    }
 }
 
-pub trait TEModelParameters: ModelParameters {
+/// Model parameters for a Twisted Edwards curve.
+pub trait TEModelParameters:
+    ModelParameters<Affine = twisted_edwards_extended::GroupAffine<Self>>
+{
     const COEFF_A: Self::BaseField;
     const COEFF_D: Self::BaseField;
-    const COFACTOR: &'static [u64];
-    const COFACTOR_INV: Self::ScalarField;
     const AFFINE_GENERATOR_COEFFS: (Self::BaseField, Self::BaseField);
 
     type MontgomeryModelParameters: MontgomeryModelParameters<BaseField = Self::BaseField>;
@@ -64,6 +70,7 @@ pub trait TEModelParameters: ModelParameters {
     }
 }
 
+/// Model parameters for a Montgomery curve.
 pub trait MontgomeryModelParameters: ModelParameters {
     const COEFF_A: Self::BaseField;
     const COEFF_B: Self::BaseField;

diff --git a/ec/src/models/short_weierstrass_jacobian.rs b/ec/src/models/short_weierstrass_jacobian.rs
@@ -12,7 +12,7 @@ use ark_std::{
 
 use ark_ff::{
     bytes::{FromBytes, ToBytes},
-    fields::{BitIteratorBE, Field, PrimeField, SquareRootField},
+    fields::{Field, PrimeField, SquareRootField},
     ToConstraintField, UniformRand,
 };
 
@@ -74,26 +74,6 @@ impl<P: Parameters> GroupAffine<P> {
         Self { x, y, infinity }
     }
 
-    /// Multiply `self` by the cofactor of the curve, `P::COFACTOR`.
-    pub fn scale_by_cofactor(&self) -> GroupProjective<P> {
-        let cofactor = BitIteratorBE::new(P::COFACTOR);
-        self.mul_bits(cofactor)
-    }
-
-    /// Multiplies `self` by the scalar represented by `bits`. `bits` must be a
-    /// big-endian bit-wise decomposition of the scalar.
-    pub(crate) fn mul_bits(&self, bits: impl Iterator<Item = bool>) -> GroupProjective<P> {
-        let mut res = GroupProjective::zero();
-        // Skip leading zeros.
-        for i in bits.skip_while(|b| !b) {
-            res.double_in_place();
-            if i {
-                res.add_assign_mixed(&self)
-            }
-        }
-        res
-    }
-
     /// Attempts to construct an affine point given an x-coordinate. The
     /// point is not guaranteed to be in the prime order subgroup.
     ///
@@ -133,9 +113,11 @@ impl<P: Parameters> GroupAffine<P> {
             y2 == x3b
         }
     }
+}
 
-    /// Checks if `self` is in the subgroup having order that equaling that of
-    /// `P::ScalarField`.
+impl<P: Parameters> GroupAffine<P> {
+    /// Checks if `self` is in the subgroup having order equaling that of
+    /// `P::ScalarField` given it is on the curve.
     pub fn is_in_correct_subgroup_assuming_on_curve(&self) -> bool {
         P::is_in_correct_subgroup_assuming_on_curve(self)
     }
@@ -192,14 +174,14 @@ impl<P: Parameters> Distribution<GroupAffine<P>> for Standard {
             let greatest = rng.gen();
 
             if let Some(p) = GroupAffine::get_point_from_x(x, greatest) {
-                return p.scale_by_cofactor().into();
+                return p.mul_by_cofactor();
             }
         }
     }
 }
 
 impl<P: Parameters> AffineCurve for GroupAffine<P> {
-    const COFACTOR: &'static [u64] = P::COFACTOR;
+    type Parameters = P;
     type BaseField = P::BaseField;
     type ScalarField = P::ScalarField;
     type Projective = GroupProjective<P>;
@@ -227,21 +209,6 @@ impl<P: Parameters> AffineCurve for GroupAffine<P> {
             }
         })
     }
-
-    #[inline]
-    fn mul<S: Into<<Self::ScalarField as PrimeField>::BigInt>>(&self, by: S) -> GroupProjective<P> {
-        let bits = BitIteratorBE::new(by.into());
-        self.mul_bits(bits)
-    }
-
-    #[inline]
-    fn mul_by_cofactor_to_projective(&self) -> Self::Projective {
-        self.scale_by_cofactor()
-    }
-
-    fn mul_by_cofactor_inv(&self) -> Self {
-        self.mul(P::COFACTOR_INV).into()
-    }
 }
 
 impl<P: Parameters> Neg for GroupAffine<P> {
@@ -360,7 +327,7 @@ impl<P: Parameters> Distribution<GroupProjective<P>> for Standard {
             let greatest = rng.gen();
 
             if let Some(p) = GroupAffine::get_point_from_x(x, greatest) {
-                return p.scale_by_cofactor().into();
+                return p.mul_by_cofactor_to_projective();
             }
         }
     }
@@ -425,7 +392,7 @@ impl<P: Parameters> Zero for GroupProjective<P> {
 }
 
 impl<P: Parameters> ProjectiveCurve for GroupProjective<P> {
-    const COFACTOR: &'static [u64] = P::COFACTOR;
+    type Parameters = P;
     type BaseField = P::BaseField;
     type ScalarField = P::ScalarField;
     type Affine = GroupAffine<P>;