Migrate to group::coordinates

CHANGELOG.md

@@ -8,6 +8,14 @@ and this project adheres to Rust's notion of
 ## [Unreleased]
 ### Changed
 - MSRV is now 1.60.0.
+- `pasta_curves::arithmetic::CurveAffine` has been refactored to have a bound
+  of `group::coordinates::ShortWeierstrassPoint`:
+  - `CurveAffine::Base` has been renamed to `CurveAffine::BaseExt`.
+  - All trait methods have been removed (use `ShortWeierstrassPoint` instead).
+
+### Removed
+- `pasta_curves::arithmetic::Coordinates` (use
+  `group::coordinates::ShortWeierstrassCoordinates` instead).
 
 ## [0.5.1] - 2023-03-02
 ### Fixed

Cargo.toml

@@ -72,3 +72,6 @@ sqrt-table = ["alloc", "lazy_static"]
 repr-c = []
 uninline-portable = []
 serde = ["hex", "serde_crate"]
+
+[patch.crates-io]
+group = { git = "https://github.com/zkcrypto/group.git", rev = "8d104f18dbca5b0ab66f052130cc6456ff465e08" }

benches/point.rs

@@ -33,7 +33,7 @@ fn point_bench<C: CurveExt>(c: &mut Criterion, name: &str) {
 
     for &n in [100, 1000, 10000].iter() {
         let input = vec![a; n];
-        let mut output = vec![C::AffineRepr::default(); n];
+        let mut output = vec![C::Affine::default(); n];
         group.bench_function(format!("point batch_normalize/{}", n), |bencher| {
             bencher.iter(|| C::batch_normalize(input.as_slice(), output.as_mut_slice()));
         });

src/arithmetic/curves.rs

@@ -2,7 +2,10 @@
 //! write code that generalizes over a pair of groups.
 
 #[cfg(feature = "alloc")]
-use group::prime::{PrimeCurve, PrimeCurveAffine};
+use group::{
+    coordinates::ShortWeierstrassPoint,
+    prime::{PrimeCurve, PrimeCurveAffine},
+};
 #[cfg(feature = "alloc")]
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
 
@@ -24,7 +27,7 @@ pub trait CurveExt:
     + Default
     + ConditionallySelectable
     + ConstantTimeEq
-    + From<<Self as PrimeCurve>::Affine>
+    + From<Self::Affine>
 {
     /// The scalar field of this elliptic curve.
     type ScalarExt: ff::WithSmallOrderMulGroup<3>;
@@ -88,94 +91,17 @@ pub trait CurveExt:
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
 pub trait CurveAffine:
-    PrimeCurveAffine<
-        Scalar = <Self as CurveAffine>::ScalarExt,
-        Curve = <Self as CurveAffine>::CurveExt,
-    > + Default
-    + Add<Output = <Self as PrimeCurveAffine>::Curve>
-    + Sub<Output = <Self as PrimeCurveAffine>::Curve>
-    + ConditionallySelectable
+    PrimeCurveAffine<Curve = Self::CurveExt, Scalar = Self::ScalarExt>
+    + ShortWeierstrassPoint<Base = Self::BaseExt>
+    + Add<Output = Self::Curve>
+    + Sub<Output = Self::Curve>
     + ConstantTimeEq
-    + From<<Self as PrimeCurveAffine>::Curve>
+    + From<Self::Curve>
 {
     /// The scalar field of this elliptic curve.
     type ScalarExt: ff::WithSmallOrderMulGroup<3> + Ord;
     /// The base field over which this elliptic curve is constructed.
-    type Base: ff::WithSmallOrderMulGroup<3> + Ord;
+    type BaseExt: ff::WithSmallOrderMulGroup<3> + Ord;
     /// The projective form of the curve
     type CurveExt: CurveExt<AffineExt = Self, ScalarExt = <Self as CurveAffine>::ScalarExt>;
-
-    /// Gets the coordinates of this point.
-    ///
-    /// Returns None if this is the identity.
-    fn coordinates(&self) -> CtOption<Coordinates<Self>>;
-
-    /// Obtains a point given $(x, y)$, failing if it is not on the
-    /// curve.
-    fn from_xy(x: Self::Base, y: Self::Base) -> CtOption<Self>;
-
-    /// Returns whether or not this element is on the curve; should
-    /// always be true unless an "unchecked" API was used.
-    fn is_on_curve(&self) -> Choice;
-
-    /// Returns the curve constant $a$.
-    fn a() -> Self::Base;
-
-    /// Returns the curve constant $b$.
-    fn b() -> Self::Base;
-}
-
-/// The affine coordinates of a point on an elliptic curve.
-#[cfg(feature = "alloc")]
-#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
-#[derive(Clone, Copy, Debug, Default)]
-pub struct Coordinates<C: CurveAffine> {
-    pub(crate) x: C::Base,
-    pub(crate) y: C::Base,
-}
-
-#[cfg(feature = "alloc")]
-impl<C: CurveAffine> Coordinates<C> {
-    /// Obtains a `Coordinates` value given $(x, y)$, failing if it is not on the curve.
-    pub fn from_xy(x: C::Base, y: C::Base) -> CtOption<Self> {
-        // We use CurveAffine::from_xy to validate the coordinates.
-        C::from_xy(x, y).map(|_| Coordinates { x, y })
-    }
-    /// Returns the x-coordinate.
-    ///
-    /// Equivalent to `Coordinates::u`.
-    pub fn x(&self) -> &C::Base {
-        &self.x
-    }
-
-    /// Returns the y-coordinate.
-    ///
-    /// Equivalent to `Coordinates::v`.
-    pub fn y(&self) -> &C::Base {
-        &self.y
-    }
-
-    /// Returns the u-coordinate.
-    ///
-    /// Equivalent to `Coordinates::x`.
-    pub fn u(&self) -> &C::Base {
-        &self.x
-    }
-
-    /// Returns the v-coordinate.
-    ///
-    /// Equivalent to `Coordinates::y`.
-    pub fn v(&self) -> &C::Base {
-        &self.y
-    }
-}
-
-#[cfg(feature = "alloc")]
-impl<C: CurveAffine> ConditionallySelectable for Coordinates<C> {
-    fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self {
-        Coordinates {
-            x: C::Base::conditional_select(&a.x, &b.x, choice),
-            y: C::Base::conditional_select(&a.y, &b.y, choice),
-        }
-    }
 }

src/curves.rs

@@ -12,8 +12,8 @@ use alloc::boxed::Box;
 use ff::{Field, PrimeField};
 use group::{
     cofactor::{CofactorCurve, CofactorGroup},
-    prime::{PrimeCurve, PrimeCurveAffine, PrimeGroup},
-    Curve as _, Group as _, GroupEncoding,
+    prime::{PrimeCurve, PrimeGroup},
+    Curve as _, CurveAffine as _, Group as _, GroupEncoding,
 };
 use rand::RngCore;
 use subtle::{Choice, ConditionallySelectable, ConstantTimeEq, CtOption};
@@ -24,7 +24,7 @@ use ff::WithSmallOrderMulGroup;
 use super::{Fp, Fq};
 
 #[cfg(feature = "alloc")]
-use crate::arithmetic::{Coordinates, CurveAffine, CurveExt};
+use crate::arithmetic::{CurveAffine, CurveExt};
 
 macro_rules! new_curve_impl {
     (($($privacy:tt)*), $name:ident, $name_affine:ident, $iso:ident, $base:ident, $scalar:ident,
@@ -168,9 +168,9 @@ macro_rules! new_curve_impl {
         }
 
         impl group::Curve for $name {
-            type AffineRepr = $name_affine;
+            type Affine = $name_affine;
 
-            fn batch_normalize(p: &[Self], q: &mut [Self::AffineRepr]) {
+            fn batch_normalize(p: &[Self], q: &mut [Self::Affine]) {
                 assert_eq!(p.len(), q.len());
 
                 let mut acc = $base::one();
@@ -207,7 +207,7 @@ macro_rules! new_curve_impl {
                 }
             }
 
-            fn to_affine(&self) -> Self::AffineRepr {
+            fn to_affine(&self) -> Self::Affine {
                 let zinv = self.z.invert().unwrap_or($base::zero());
                 let zinv2 = zinv.square();
                 let x = self.x * zinv2;
@@ -244,13 +244,9 @@ macro_rules! new_curve_impl {
             }
         }
 
-        impl PrimeCurve for $name {
-            type Affine = $name_affine;
-        }
+        impl PrimeCurve for $name {}
 
-        impl CofactorCurve for $name {
-            type Affine = $name_affine;
-        }
+        impl CofactorCurve for $name {}
 
         impl GroupEncoding for $name {
             type Repr = [u8; 32];
@@ -610,7 +606,7 @@ macro_rules! new_curve_impl {
             }
         }
 
-        impl PrimeCurveAffine for $name_affine {
+        impl group::CurveAffine for $name_affine {
             type Curve = $name;
             type Scalar = $scalar;
 
@@ -636,27 +632,6 @@ macro_rules! new_curve_impl {
             }
         }
 
-        impl group::cofactor::CofactorCurveAffine for $name_affine {
-            type Curve = $name;
-            type Scalar = $scalar;
-
-            fn identity() -> Self {
-                <Self as PrimeCurveAffine>::identity()
-            }
-
-            fn generator() -> Self {
-                <Self as PrimeCurveAffine>::generator()
-            }
-
-            fn is_identity(&self) -> Choice {
-                <Self as PrimeCurveAffine>::is_identity(self)
-            }
-
-            fn to_curve(&self) -> Self::Curve {
-                <Self as PrimeCurveAffine>::to_curve(self)
-            }
-        }
-
         impl GroupEncoding for $name_affine {
             type Repr = [u8; 32];
 
@@ -708,32 +683,43 @@ macro_rules! new_curve_impl {
         #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
         impl CurveAffine for $name_affine {
             type ScalarExt = $scalar;
-            type Base = $base;
+            type BaseExt = $base;
             type CurveExt = $name;
+        }
 
+        impl $name_affine {
             fn is_on_curve(&self) -> Choice {
                 // y^2 - x^3 - ax ?= b
                 (self.y.square() - (self.x.square() + &$name::curve_constant_a()) * self.x).ct_eq(&$name::curve_constant_b())
                     | self.is_identity()
             }
+        }
 
-            fn coordinates(&self) -> CtOption<Coordinates<Self>> {
-                CtOption::new(Coordinates { x: self.x, y: self.y }, !self.is_identity())
-            }
+        #[cfg(feature = "alloc")]
+        #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
+        impl group::coordinates::ShortWeierstrassPoint for $name_affine {
+            type Base = $base;
+
+            const A: Self::Base = $name::curve_constant_a();
+            const B: Self::Base = $name::curve_constant_b();
 
-            fn from_xy(x: Self::Base, y: Self::Base) -> CtOption<Self> {
+            fn from_bare_coordinates(x: Self::Base, y: Self::Base) -> CtOption<Self> {
                 let p = $name_affine {
                     x, y,
                 };
                 CtOption::new(p, p.is_on_curve())
             }
 
-            fn a() -> Self::Base {
-                $name::curve_constant_a()
+            fn from_coordinates(coords: group::coordinates::ShortWeierstrassCoordinates<Self>) -> Self {
+                $name_affine {
+                    x: coords.x(),
+                    y: coords.y(),
+                }
             }
 
-            fn b() -> Self::Base {
-                $name::curve_constant_b()
+            fn coordinates(&self) -> CtOption<group::coordinates::ShortWeierstrassCoordinates<Self>> {
+                // TODO: Avoid a second on-curve check by storing this directly.
+                group::coordinates::ShortWeierstrassCoordinates::from_coordinates(self.x, self.y)
             }
         }
 

src/serde_impl.rs

@@ -137,7 +137,7 @@ mod tests {
     use core::fmt::Debug;
 
     use ff::Field;
-    use group::{prime::PrimeCurveAffine, Curve, Group};
+    use group::{Curve, CurveAffine, Group};
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;