Change serialisation format for Twisted Edwards Curves

CHANGELOG.md

@@ -9,6 +9,7 @@
 - [\#333](https://github.com/arkworks-rs/algebra/pull/333) (ark-poly) Expose more properties of `EvaluationDomain`s.
 - [\#338](https://github.com/arkworks-rs/algebra/pull/338) (ark-ec) Add missing `UniformRand` trait bound to `GroupAffine`.
 - [\#338](https://github.com/arkworks-rs/algebra/pull/338) (workspace) Change to Rust 2021 edition.
+- [\#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)
 
 ### Features
 

ec/src/models/twisted_edwards_extended.rs

@@ -73,23 +73,32 @@ impl<P: Parameters> GroupAffine<P> {
         res
     }
 
-    /// Attempts to construct an affine point given an x-coordinate. The
+    /// Attempts to construct an affine point given an y-coordinate. The
     /// point is not guaranteed to be in the prime order subgroup.
     ///
     /// If and only if `greatest` is set will the lexicographically
-    /// largest y-coordinate be selected.
+    /// largest x-coordinate be selected.
+    ///
+    /// a * X^2 + Y^2 = 1 + d * X^2 * Y^2
+    /// a * X^2 - d * X^2 * Y^2 = 1 - Y^2
+    /// X^2 * (a - d * Y^2) = 1 - Y^2
+    /// X^2 = (1 - Y^2) / (a - d * Y^2)
     #[allow(dead_code)]
-    pub fn get_point_from_x(x: P::BaseField, greatest: bool) -> Option<Self> {
-        let x2 = x.square();
-        let one = P::BaseField::one();
-        let numerator = P::mul_by_a(&x2) - &one;
-        let denominator = P::COEFF_D * &x2 - &one;
-        let y2 = denominator.inverse().map(|denom| denom * &numerator);
-        y2.and_then(|y2| y2.sqrt()).map(|y| {
-            let negy = -y;
-            let y = if (y < negy) ^ greatest { y } else { negy };
-            Self::new(x, y)
-        })
+    pub fn get_point_from_y(y: P::BaseField, greatest: bool) -> Option<Self> {
+        let y2 = y.square();
+
+        let numerator = P::BaseField::one() - y2;
+        let denominator = P::COEFF_A - (y2 * P::COEFF_D);
+
+        denominator
+            .inverse()
+            .map(|denom| denom * &numerator)
+            .and_then(|x2| x2.sqrt())
+            .map(|x| {
+                let negx = -x;
+                let x = if (x < negx) ^ greatest { x } else { negx };
+                Self::new(x, y)
+            })
     }
 
     /// Checks that the current point is on the elliptic curve.
@@ -136,13 +145,13 @@ impl<P: Parameters> AffineCurve for GroupAffine<P> {
     }
 
     fn from_random_bytes(bytes: &[u8]) -> Option<Self> {
-        P::BaseField::from_random_bytes_with_flags::<EdwardsFlags>(bytes).and_then(|(x, flags)| {
-            // if x is valid and is zero, then parse this
+        P::BaseField::from_random_bytes_with_flags::<EdwardsFlags>(bytes).and_then(|(y, flags)| {
+            // if y is valid and is zero, then parse this
             // point as infinity.
-            if x.is_zero() {
+            if y.is_zero() {
                 Some(Self::zero())
             } else {
-                Self::get_point_from_x(x, flags.is_positive())
+                Self::get_point_from_y(y, flags.is_positive())
             }
         })
     }
@@ -251,10 +260,10 @@ impl<P: Parameters> Distribution<GroupAffine<P>> for Standard {
     #[inline]
     fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> GroupAffine<P> {
         loop {
-            let x = P::BaseField::rand(rng);
+            let y = P::BaseField::rand(rng);
             let greatest = rng.gen();
 
-            if let Some(p) = GroupAffine::get_point_from_x(x, greatest) {
+            if let Some(p) = GroupAffine::get_point_from_y(y, greatest) {
                 return p.scale_by_cofactor().into();
             }
         }
@@ -350,10 +359,10 @@ impl<P: Parameters> Distribution<GroupProjective<P>> for Standard {
     #[inline]
     fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> GroupProjective<P> {
         loop {
-            let x = P::BaseField::rand(rng);
+            let y = P::BaseField::rand(rng);
             let greatest = rng.gen();
 
-            if let Some(p) = GroupAffine::get_point_from_x(x, greatest) {
+            if let Some(p) = GroupAffine::get_point_from_y(y, greatest) {
                 return p.scale_by_cofactor();
             }
         }
@@ -704,8 +713,8 @@ impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
             // Serialize 0.
             P::BaseField::zero().serialize_with_flags(writer, flags)
         } else {
-            let flags = EdwardsFlags::from_y_sign(self.y > -self.y);
-            self.x.serialize_with_flags(writer, flags)
+            let flags = EdwardsFlags::from_x_sign(self.x > -self.x);
+            self.y.serialize_with_flags(writer, flags)
         }
     }
 
@@ -760,12 +769,12 @@ impl<P: Parameters> CanonicalSerialize for GroupProjective<P> {
 impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
     #[allow(unused_qualifications)]
     fn deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
-        let (x, flags): (P::BaseField, EdwardsFlags) =
+        let (y, flags): (P::BaseField, EdwardsFlags) =
             CanonicalDeserializeWithFlags::deserialize_with_flags(&mut reader)?;
-        if x == P::BaseField::zero() {
+        if y == P::BaseField::zero() {
             Ok(Self::zero())
         } else {
-            let p = GroupAffine::<P>::get_point_from_x(x, flags.is_positive())
+            let p = GroupAffine::<P>::get_point_from_y(y, flags.is_positive())
                 .ok_or(SerializationError::InvalidData)?;
             if !p.is_in_correct_subgroup_assuming_on_curve() {
                 return Err(SerializationError::InvalidData);
@@ -836,3 +845,120 @@ where
         GroupAffine::from(*self).to_field_elements()
     }
 }
+
+// This impl block and the one following are being used to encapsulate all of
+// the methods that are needed for backwards compatibility with the old
+// serialization format
+// See Issue #330
+impl<P: Parameters> GroupAffine<P> {
+    /// Attempts to construct an affine point given an x-coordinate. The
+    /// point is not guaranteed to be in the prime order subgroup.
+    ///
+    /// If and only if `greatest` is set will the lexicographically
+    /// largest y-coordinate be selected.
+    ///
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    #[allow(dead_code)]
+    pub fn get_point_from_x_old(x: P::BaseField, greatest: bool) -> Option<Self> {
+        let x2 = x.square();
+        let one = P::BaseField::one();
+        let numerator = P::mul_by_a(&x2) - &one;
+        let denominator = P::COEFF_D * &x2 - &one;
+        let y2 = denominator.inverse().map(|denom| denom * &numerator);
+        y2.and_then(|y2| y2.sqrt()).map(|y| {
+            let negy = -y;
+            let y = if (y < negy) ^ greatest { y } else { negy };
+            Self::new(x, y)
+        })
+    }
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn serialize_old<W: Write>(&self, writer: W) -> Result<(), SerializationError> {
+        if self.is_zero() {
+            let flags = EdwardsFlags::default();
+            // Serialize 0.
+            P::BaseField::zero().serialize_with_flags(writer, flags)
+        } else {
+            // Note: although this says `from_x_sign` and we are
+            // using the sign of `y`. The logic works the same.
+            let flags = EdwardsFlags::from_x_sign(self.y > -self.y);
+            self.x.serialize_with_flags(writer, flags)
+        }
+    }
+
+    #[allow(unused_qualifications)]
+    #[inline]
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn serialize_uncompressed_old<W: Write>(
+        &self,
+        mut writer: W,
+    ) -> Result<(), SerializationError> {
+        self.x.serialize_uncompressed(&mut writer)?;
+        self.y.serialize_uncompressed(&mut writer)?;
+        Ok(())
+    }
+
+    #[allow(unused_qualifications)]
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn deserialize_uncompressed_old<R: Read>(reader: R) -> Result<Self, SerializationError> {
+        let p = Self::deserialize_unchecked(reader)?;
+
+        if !p.is_in_correct_subgroup_assuming_on_curve() {
+            return Err(SerializationError::InvalidData);
+        }
+        Ok(p)
+    }
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn deserialize_old<R: Read>(mut reader: R) -> Result<Self, SerializationError> {
+        let (x, flags): (P::BaseField, EdwardsFlags) =
+            CanonicalDeserializeWithFlags::deserialize_with_flags(&mut reader)?;
+        if x == P::BaseField::zero() {
+            Ok(Self::zero())
+        } else {
+            let p = GroupAffine::<P>::get_point_from_x_old(x, flags.is_positive())
+                .ok_or(SerializationError::InvalidData)?;
+            if !p.is_in_correct_subgroup_assuming_on_curve() {
+                return Err(SerializationError::InvalidData);
+            }
+            Ok(p)
+        }
+    }
+}
+impl<P: Parameters> GroupProjective<P> {
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn serialize_old<W: Write>(&self, writer: W) -> Result<(), SerializationError> {
+        let aff = GroupAffine::<P>::from(self.clone());
+        aff.serialize_old(writer)
+    }
+
+    #[allow(unused_qualifications)]
+    #[inline]
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn serialize_uncompressed_old<W: Write>(
+        &self,
+        writer: W,
+    ) -> Result<(), SerializationError> {
+        let aff = GroupAffine::<P>::from(self.clone());
+        aff.serialize_uncompressed(writer)
+    }
+
+    #[allow(unused_qualifications)]
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn deserialize_uncompressed_old<R: Read>(reader: R) -> Result<Self, SerializationError> {
+        let aff = GroupAffine::<P>::deserialize_uncompressed(reader)?;
+        Ok(aff.into())
+    }
+    /// This method is implemented for backwards compatibility with the old serialization format
+    /// and will be deprecated and then removed in a future version.
+    pub fn deserialize_old<R: Read>(reader: R) -> Result<Self, SerializationError> {
+        let aff = GroupAffine::<P>::deserialize_old(reader)?;
+        Ok(aff.into())
+    }
+}

serialize/src/flags.rs

@@ -17,18 +17,19 @@ pub trait Flags: Default + Clone + Copy + Sized {
     // bit masks: `0` and `1 << 7`.
     fn u8_bitmask(&self) -> u8;
 
-    // Tries to read `Self` from `value`. Should return `None` if the `Self::BIT_SIZE`
-    // most-significant bits of `value` do not correspond to those generated by
-    // `u8_bitmask`.
+    // Tries to read `Self` from `value`. Should return `None` if the
+    // `Self::BIT_SIZE` most-significant bits of `value` do not correspond to
+    // those generated by `u8_bitmask`.
     //
     // That is, this method ignores all but the top `Self::BIT_SIZE` bits, and
-    // decides whether these top bits correspond to a bitmask output by `u8_bitmask`.
+    // decides whether these top bits correspond to a bitmask output by
+    // `u8_bitmask`.
     fn from_u8(value: u8) -> Option<Self>;
 
-    // Convenience method that reads `Self` from `value`, just like `Self::from_u8`, but
-    // additionally zeroes out the bits corresponding to the resulting flag in `value`.
-    // If `Self::from_u8(*value)` would return `None`, then this method should
-    // *not* modify `value`.
+    // Convenience method that reads `Self` from `value`, just like `Self::from_u8`,
+    // but additionally zeroes out the bits corresponding to the resulting flag
+    // in `value`. If `Self::from_u8(*value)` would return `None`, then this
+    // method should *not* modify `value`.
     fn from_u8_remove_flags(value: &mut u8) -> Option<Self> {
         let flags = Self::from_u8(*value);
         if let Some(f) = flags {
@@ -123,9 +124,9 @@ impl Flags for SWFlags {
 
     #[inline]
     fn from_u8(value: u8) -> Option<Self> {
-        let x_sign = (value >> 7) & 1 == 1;
+        let y_sign = (value >> 7) & 1 == 1;
         let is_infinity = (value >> 6) & 1 == 1;
-        match (x_sign, is_infinity) {
+        match (y_sign, is_infinity) {
             // This is invalid because we only want *one* way to serialize
             // the point at infinity.
             (true, true) => None,
@@ -140,34 +141,34 @@ impl Flags for SWFlags {
 /// The default flags (empty) should not change the binary representation.
 #[derive(Clone, Copy, PartialEq, Eq)]
 pub enum EdwardsFlags {
-    PositiveY,
-    NegativeY,
+    PositiveX,
+    NegativeX,
 }
 
 impl EdwardsFlags {
     #[inline]
-    pub fn from_y_sign(is_positive: bool) -> Self {
+    pub fn from_x_sign(is_positive: bool) -> Self {
         if is_positive {
-            EdwardsFlags::PositiveY
+            EdwardsFlags::PositiveX
         } else {
-            EdwardsFlags::NegativeY
+            EdwardsFlags::NegativeX
         }
     }
 
     #[inline]
     pub fn is_positive(&self) -> bool {
         match self {
-            EdwardsFlags::PositiveY => true,
-            EdwardsFlags::NegativeY => false,
+            EdwardsFlags::PositiveX => true,
+            EdwardsFlags::NegativeX => false,
         }
     }
 }
 
 impl Default for EdwardsFlags {
     #[inline]
     fn default() -> Self {
-        // NegativeY doesn't change the serialization
-        EdwardsFlags::NegativeY
+        // PositiveX doesn't change the serialization
+        EdwardsFlags::PositiveX
     }
 }
 
@@ -177,7 +178,7 @@ impl Flags for EdwardsFlags {
     #[inline]
     fn u8_bitmask(&self) -> u8 {
         let mut mask = 0;
-        if let Self::PositiveY = self {
+        if let Self::NegativeX = self {
             mask |= 1 << 7;
         }
         mask
@@ -187,9 +188,9 @@ impl Flags for EdwardsFlags {
     fn from_u8(value: u8) -> Option<Self> {
         let x_sign = (value >> 7) & 1 == 1;
         if x_sign {
-            Some(Self::PositiveY)
+            Some(Self::NegativeX)
         } else {
-            Some(Self::NegativeY)
+            Some(Self::PositiveX)
         }
     }
 }