Fix #47

CHANGELOG.md

@@ -27,6 +27,15 @@ The main features of this release are:
 - #129 (ark-ff) Move `ark_ff::{UniformRand, test_rng}` to `ark_std::{UniformRand, test_rng}`.
     Importing these from `ark-ff` is still possible, but is deprecated and will be removed in the following release.
 - #144 (ark-poly) Add `CanonicalSerialize` and `CanonicalDeserialize` trait bounds for `Polynomial`.
+- #160 (ark-serialize, ark-ff, ark-ec) 
+  - Remove `ConstantSerializedSize`; users should use `serialized_size*` (see next).
+  - Add `serialized_size_with_flags` method to `CanonicalSerializeWithFlags`. 
+  - Change `from_random_bytes_with_flags` to output `ark_serialize::Flags`.
+  - Change signatures of `Flags::from_u8*` to output `Option`.
+  - Change `Flags::from_u8*` to be more strict about the inputs they accept: 
+    if the top bits of the `u8` value do *not* correspond to one of the possible outputs of `Flags::u8_bitmask`, then these methods output `None`, whereas before they output
+    a default value.
+  Downstream users other than `ark-curves` should not see breakage unless they rely on these methods/traits explicitly.
 
 ### Features
 - #20 (ark-poly) Add structs/traits for multivariate polynomials
@@ -67,6 +76,7 @@ The main features of this release are:
 - #112 (ark-serialize) Make `bool`s checked serialization methods non-malleable.
 - #119 (ark-poly) Fix bugs in degree calculation if adding/subtracting same degree polynomials
      whose leading coefficients cancel.
+- #160 (ark-serialize, ark-ff, ark-ec) Support serializing when `MODULUS_BITS + FLAG_BITS` is greater than the multiple of 8 just greater than `MODULUS_BITS`, which is the case for the Pasta curves (fixes #47).
 
 
 ## v0.1.0 (Initial release of arkworks/algebra)

ec/src/lib.rs

@@ -19,7 +19,7 @@ use ark_ff::{
     fields::{Field, PrimeField, SquareRootField},
     UniformRand,
 };
-use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, ConstantSerializedSize};
+use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
 use ark_std::{
     fmt::{Debug, Display},
     hash::Hash,
@@ -223,7 +223,6 @@ pub trait AffineCurve:
     + ToBytes
     + FromBytes
     + CanonicalSerialize
-    + ConstantSerializedSize
     + CanonicalDeserialize
     + Copy
     + Clone

ec/src/models/short_weierstrass_jacobian.rs

@@ -1,6 +1,6 @@
 use ark_serialize::{
     CanonicalDeserialize, CanonicalDeserializeWithFlags, CanonicalSerialize,
-    CanonicalSerializeWithFlags, ConstantSerializedSize, Flags, SWFlags, SerializationError,
+    CanonicalSerializeWithFlags, SWFlags, SerializationError,
 };
 use ark_std::{
     fmt::{Display, Formatter, Result as FmtResult},
@@ -188,16 +188,15 @@ impl<P: Parameters> AffineCurve for GroupAffine<P> {
     }
 
     fn from_random_bytes(bytes: &[u8]) -> Option<Self> {
-        P::BaseField::from_random_bytes_with_flags(bytes).and_then(|(x, flags)| {
-            let infinity_flag_mask = SWFlags::Infinity.u8_bitmask();
-            let positive_flag_mask = SWFlags::PositiveY.u8_bitmask();
+        P::BaseField::from_random_bytes_with_flags::<SWFlags>(bytes).and_then(|(x, flags)| {
             // if x is valid and is zero and only the infinity flag is set, then parse this
             // point as infinity. For all other choices, get the original point.
-            if x.is_zero() && flags == infinity_flag_mask {
+            if x.is_zero() && flags.is_infinity() {
                 Some(Self::zero())
+            } else if let Some(y_is_positve) = flags.is_positive() {
+                Self::get_point_from_x(x, y_is_positve) // Unwrap is safe because it's not zero.
             } else {
-                let is_positive = flags & positive_flag_mask != 0;
-                Self::get_point_from_x(x, is_positive)
+                None
             }
         })
     }
@@ -728,7 +727,7 @@ impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
 
     #[inline]
     fn serialized_size(&self) -> usize {
-        Self::SERIALIZED_SIZE
+        P::BaseField::zero().serialized_size_with_flags::<SWFlags>()
     }
 
     #[allow(unused_qualifications)]
@@ -746,15 +745,10 @@ impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
 
     #[inline]
     fn uncompressed_size(&self) -> usize {
-        Self::UNCOMPRESSED_SIZE
+        self.x.serialized_size() + self.y.serialized_size_with_flags::<SWFlags>()
     }
 }
 
-impl<P: Parameters> ConstantSerializedSize for GroupAffine<P> {
-    const SERIALIZED_SIZE: usize = <P::BaseField as ConstantSerializedSize>::SERIALIZED_SIZE;
-    const UNCOMPRESSED_SIZE: usize = 2 * <P::BaseField as ConstantSerializedSize>::SERIALIZED_SIZE;
-}
-
 impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
     #[allow(unused_qualifications)]
     fn deserialize<R: Read>(reader: R) -> Result<Self, SerializationError> {

ec/src/models/twisted_edwards_extended.rs

@@ -4,7 +4,7 @@ use crate::{
 };
 use ark_serialize::{
     CanonicalDeserialize, CanonicalDeserializeWithFlags, CanonicalSerialize,
-    CanonicalSerializeWithFlags, ConstantSerializedSize, EdwardsFlags, Flags, SerializationError,
+    CanonicalSerializeWithFlags, EdwardsFlags, SerializationError,
 };
 use ark_std::{
     fmt::{Display, Formatter, Result as FmtResult},
@@ -142,17 +142,15 @@ impl<P: Parameters> AffineCurve for GroupAffine<P> {
     }
 
     fn from_random_bytes(bytes: &[u8]) -> Option<Self> {
-        let x = P::BaseField::from_random_bytes_with_flags(bytes);
-        if let Some((x, flags)) = x {
-            let parsed_flags = EdwardsFlags::from_u8(flags);
+        P::BaseField::from_random_bytes_with_flags::<EdwardsFlags>(bytes).and_then(|(x, flags)| {
+            // if x is valid and is zero, then parse this
+            // point as infinity.
             if x.is_zero() {
                 Some(Self::zero())
             } else {
-                Self::get_point_from_x(x, parsed_flags.is_positive())
+                Self::get_point_from_x(x, flags.is_positive())
             }
-        } else {
-            None
-        }
+        })
     }
 
     #[inline]
@@ -729,7 +727,7 @@ impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
 
     #[inline]
     fn serialized_size(&self) -> usize {
-        Self::SERIALIZED_SIZE
+        P::BaseField::zero().serialized_size_with_flags::<EdwardsFlags>()
     }
 
     #[allow(unused_qualifications)]
@@ -742,15 +740,11 @@ impl<P: Parameters> CanonicalSerialize for GroupAffine<P> {
 
     #[inline]
     fn uncompressed_size(&self) -> usize {
-        Self::UNCOMPRESSED_SIZE
+        // x  + y
+        self.x.serialized_size() + self.y.serialized_size()
     }
 }
 
-impl<P: Parameters> ConstantSerializedSize for GroupAffine<P> {
-    const SERIALIZED_SIZE: usize = <P::BaseField as ConstantSerializedSize>::SERIALIZED_SIZE;
-    const UNCOMPRESSED_SIZE: usize = 2 * <P::BaseField as ConstantSerializedSize>::SERIALIZED_SIZE;
-}
-
 impl<P: Parameters> CanonicalDeserialize for GroupAffine<P> {
     #[allow(unused_qualifications)]
     fn deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> {

ff/src/biginteger/macros.rs

@@ -209,15 +209,10 @@ macro_rules! bigint_impl {
 
             #[inline]
             fn serialized_size(&self) -> usize {
-                Self::SERIALIZED_SIZE
+                Self::NUM_LIMBS * 8
             }
         }
 
-        impl ConstantSerializedSize for $name {
-            const SERIALIZED_SIZE: usize = Self::NUM_LIMBS * 8;
-            const UNCOMPRESSED_SIZE: usize = Self::SERIALIZED_SIZE;
-        }
-
         impl CanonicalDeserialize for $name {
             #[inline]
             fn deserialize<R: Read>(reader: R) -> Result<Self, SerializationError> {

ff/src/biginteger/mod.rs

@@ -3,9 +3,7 @@ use crate::{
     fields::BitIteratorBE,
     UniformRand,
 };
-use ark_serialize::{
-    CanonicalDeserialize, CanonicalSerialize, ConstantSerializedSize, SerializationError,
-};
+use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, SerializationError};
 use ark_std::{
     fmt::{Debug, Display},
     io::{Read, Result as IoResult, Write},
@@ -39,7 +37,6 @@ pub trait BigInteger:
     ToBytes
     + FromBytes
     + CanonicalSerialize
-    + ConstantSerializedSize
     + CanonicalDeserialize
     + Copy
     + Clone

ff/src/fields/macros.rs

@@ -1,90 +1,86 @@
 macro_rules! impl_prime_field_serializer {
     ($field: ident, $params: ident, $byte_size: expr) => {
-        impl<P: $params> ark_serialize::CanonicalSerializeWithFlags for $field<P> {
-            #[allow(unused_qualifications)]
-            fn serialize_with_flags<W: ark_std::io::Write, F: ark_serialize::Flags>(
+        impl<P: $params> CanonicalSerializeWithFlags for $field<P> {
+            fn serialize_with_flags<W: ark_std::io::Write, F: Flags>(
                 &self,
                 mut writer: W,
                 flags: F,
-            ) -> Result<(), ark_serialize::SerializationError> {
-                const BYTE_SIZE: usize = $byte_size;
-
-                let (output_bit_size, output_byte_size) =
-                    ark_serialize::buffer_bit_byte_size($field::<P>::size_in_bits());
-                if F::len() > (output_bit_size - P::MODULUS_BITS as usize) {
-                    return Err(ark_serialize::SerializationError::NotEnoughSpace);
+            ) -> Result<(), SerializationError> {
+                // All reasonable `Flags` should be less than 8 bits in size
+                // (256 values are enough for anyone!)
+                if F::BIT_SIZE > 8 {
+                    return Err(SerializationError::NotEnoughSpace);
                 }
 
-                let mut bytes = [0u8; BYTE_SIZE];
-                self.write(&mut bytes[..])?;
+                // Calculate the number of bytes required to represent a field element
+                // serialized with `flags`. If `F::BIT_SIZE < 8`,
+                // this is at most `$byte_size + 1`
+                let output_byte_size = buffer_byte_size(P::MODULUS_BITS as usize + F::BIT_SIZE);
+
+                // Write out `self` to a temporary buffer.
+                // The size of the buffer is $byte_size + 1 because `F::BIT_SIZE`
+                // is at most 8 bits.
+                let mut bytes = [0u8; $byte_size + 1];
+                self.write(&mut bytes[..$byte_size])?;
 
+                // Mask out the bits of the last byte that correspond to the flag.
                 bytes[output_byte_size - 1] |= flags.u8_bitmask();
 
                 writer.write_all(&bytes[..output_byte_size])?;
                 Ok(())
             }
-        }
 
-        impl<P: $params> ark_serialize::ConstantSerializedSize for $field<P> {
-            const SERIALIZED_SIZE: usize = ark_serialize::buffer_byte_size(
-                <$field<P> as crate::PrimeField>::Params::MODULUS_BITS as usize,
-            );
-            const UNCOMPRESSED_SIZE: usize = Self::SERIALIZED_SIZE;
+            // Let `m = 8 * n` for some `n` be the smallest multiple of 8 greater
+            // than `P::MODULUS_BITS`.
+            // If `(m - P::MODULUS_BITS) >= F::BIT_SIZE` , then this method returns `n`;
+            // otherwise, it returns `n + 1`.
+            fn serialized_size_with_flags<F: Flags>(&self) -> usize {
+                buffer_byte_size(P::MODULUS_BITS as usize + F::BIT_SIZE)
+            }
         }
 
-        impl<P: $params> ark_serialize::CanonicalSerialize for $field<P> {
-            #[allow(unused_qualifications)]
+        impl<P: $params> CanonicalSerialize for $field<P> {
             #[inline]
             fn serialize<W: ark_std::io::Write>(
                 &self,
                 writer: W,
-            ) -> Result<(), ark_serialize::SerializationError> {
-                use ark_serialize::*;
-                self.serialize_with_flags(writer, ark_serialize::EmptyFlags)
+            ) -> Result<(), SerializationError> {
+                self.serialize_with_flags(writer, EmptyFlags)
             }
 
             #[inline]
             fn serialized_size(&self) -> usize {
-                use ark_serialize::*;
-                Self::SERIALIZED_SIZE
+                self.serialized_size_with_flags::<EmptyFlags>()
             }
         }
 
-        impl<P: $params> ark_serialize::CanonicalDeserializeWithFlags for $field<P> {
-            #[allow(unused_qualifications)]
-            fn deserialize_with_flags<R: ark_std::io::Read, F: ark_serialize::Flags>(
+        impl<P: $params> CanonicalDeserializeWithFlags for $field<P> {
+            fn deserialize_with_flags<R: ark_std::io::Read, F: Flags>(
                 mut reader: R,
-            ) -> Result<(Self, F), ark_serialize::SerializationError> {
-                const BYTE_SIZE: usize = $byte_size;
-
-                let (output_bit_size, output_byte_size) =
-                    ark_serialize::buffer_bit_byte_size($field::<P>::size_in_bits());
-                if F::len() > (output_bit_size - P::MODULUS_BITS as usize) {
-                    return Err(ark_serialize::SerializationError::NotEnoughSpace);
+            ) -> Result<(Self, F), SerializationError> {
+                // All reasonable `Flags` should be less than 8 bits in size
+                // (256 values are enough for anyone!)
+                if F::BIT_SIZE > 8 {
+                    return Err(SerializationError::NotEnoughSpace);
                 }
+                // Calculate the number of bytes required to represent a field element
+                // serialized with `flags`. If `F::BIT_SIZE < 8`,
+                // this is at most `$byte_size + 1`
+                let output_byte_size = buffer_byte_size(P::MODULUS_BITS as usize + F::BIT_SIZE);
 
-                let mut masked_bytes = [0; BYTE_SIZE];
+                let mut masked_bytes = [0; $byte_size + 1];
                 reader.read_exact(&mut masked_bytes[..output_byte_size])?;
 
-                let flags = F::from_u8_remove_flags(&mut masked_bytes[output_byte_size - 1]);
+                let flags = F::from_u8_remove_flags(&mut masked_bytes[output_byte_size - 1])
+                    .ok_or(SerializationError::UnexpectedFlags)?;
 
                 Ok((Self::read(&masked_bytes[..])?, flags))
             }
         }
 
-        impl<P: $params> ark_serialize::CanonicalDeserialize for $field<P> {
-            #[allow(unused_qualifications)]
-            fn deserialize<R: ark_std::io::Read>(
-                mut reader: R,
-            ) -> Result<Self, ark_serialize::SerializationError> {
-                const BYTE_SIZE: usize = $byte_size;
-
-                let (_, output_byte_size) =
-                    ark_serialize::buffer_bit_byte_size($field::<P>::size_in_bits());
-
-                let mut masked_bytes = [0; BYTE_SIZE];
-                reader.read_exact(&mut masked_bytes[..output_byte_size])?;
-                Ok(Self::read(&masked_bytes[..])?)
+        impl<P: $params> CanonicalDeserialize for $field<P> {
+            fn deserialize<R: ark_std::io::Read>(reader: R) -> Result<Self, SerializationError> {
+                Self::deserialize_with_flags::<R, EmptyFlags>(reader).map(|(r, _)| r)
             }
         }
     };
@@ -303,28 +299,50 @@ macro_rules! impl_Fp {
             }
 
             #[inline]
-            fn from_random_bytes_with_flags(bytes: &[u8]) -> Option<(Self, u8)> {
-                use ark_serialize::*;
-                let mut result_bytes = [0u8; $limbs * 8];
-                for (result_byte, in_byte) in result_bytes.iter_mut().zip(bytes.iter()) {
-                    *result_byte = *in_byte;
-                }
-
-                let mask: u64 = 0xffffffffffffffff >> P::REPR_SHAVE_BITS;
-                // the flags will be at the same byte with the lowest shaven bits or the one after
-                let flags_byte_position: usize = 7 - P::REPR_SHAVE_BITS as usize / 8;
-                let flags_mask: u8 = ((1 << P::REPR_SHAVE_BITS % 8) - 1) << (8 - P::REPR_SHAVE_BITS % 8);
-                // take the last 8 bytes and pass the mask
-                let last_bytes = &mut result_bytes[($limbs - 1) * 8..];
-                let mut flags: u8 = 0;
-                for (i, (b, m)) in last_bytes.iter_mut().zip(&mask.to_le_bytes()).enumerate() {
-                    if i == flags_byte_position {
-                        flags = *b & flags_mask
+            fn from_random_bytes_with_flags<F: Flags>(bytes: &[u8]) -> Option<(Self, F)> {
+                if F::BIT_SIZE > 8 {
+                    return None
+                } else {
+                    let mut result_bytes = [0u8; $limbs * 8 + 1];
+                    // Copy the input into a temporary buffer.
+                    result_bytes.iter_mut().zip(bytes).for_each(|(result, input)| {
+                        *result = *input;
+                    });
+                    // This mask retains everything in the last limb
+                    // that is below `P::MODULUS_BITS`.
+                    let last_limb_mask = (u64::MAX >> P::REPR_SHAVE_BITS).to_le_bytes();
+                    let mut last_bytes_mask = [0u8; 9];
+                    last_bytes_mask[..8].copy_from_slice(&last_limb_mask);
+
+
+                    // Length of the buffer containing the field element and the flag.
+                    let output_byte_size = buffer_byte_size(P::MODULUS_BITS as usize + F::BIT_SIZE);
+                    // Location of the flag is the last byte of the serialized
+                    // form of the field element.
+                    let flag_location = output_byte_size - 1;
+
+                    // At which byte is the flag located in the last limb?
+                    let flag_location_in_last_limb = flag_location - (8 * ($limbs - 1));
+
+                    // Take all but the last 9 bytes.
+                    let last_bytes = &mut result_bytes[8 * ($limbs - 1)..];
+
+                    // The mask only has the last `F::BIT_SIZE` bits set
+                    let flags_mask = u8::MAX << (8 - F::BIT_SIZE);
+
+                    // Mask away the remaining bytes, and try to reconstruct the
+                    // flag
+                    let mut flags: u8 = 0;
+                    for (i, (b, m)) in last_bytes.iter_mut().zip(&last_bytes_mask).enumerate() {
+                        if i == flag_location_in_last_limb {
+                            flags = *b & flags_mask
+                        }
+                        *b &= m;
                     }
-                    *b &= m;
+                    Self::deserialize(&result_bytes[..($limbs * 8)])
+                        .ok()
+                        .and_then(|f| F::from_u8(flags).map(|flag| (f, flag)))
                 }
-
-                Self::deserialize(&mut &result_bytes[..]).ok().map(|f| (f, flags))
             }
 
             #[inline]