Make serialization methods consistent (#674)

* make serialization consistent

* fix SignatureShare and Identifier serialization

* Update frost-core/CHANGELOG.md

frost-core/CHANGELOG.md

@@ -17,6 +17,18 @@ Entries are listed in reverse chronological order.
     `VerifiableSecretSharingCommitment::serialize()`,
     `NonceCommitment::serialize()`, `Signature::serialize()`,
     `VerifyingKey::serialize()` can now all return an error.
+* Changed the `serialize()` and `deserialize()` methods of all Scalar- and
+  Element-wrapping structs; instead of taking or returning a
+  `Field::Serialization` or `Element::Serialization` trait (which are usually
+  defined by ciphersuites as arrays of specific sizes), they simply respectively
+  take `&[u8]` and return `Vec<u8>`, exactly as the other structs, which should
+  greatly simplify non-serde serialization code. You can port existing code with
+  e.g. `x.serialize().as_ref()` -> `x.serialize()` and
+  `X::deserialize(bytes.try_into().unwrap())` -> `X::deserialize(&bytes)`.
+* Removed the `ops::{Mul, MulAssign, Sub}` implementation for `Identifier`.
+  These were being used internally, but library users shouldn't need to use them.
+  If you have low-level code that relied on it, use `Identifier::{new,
+  to_scalar}` to handle the underlying scalar.
 * Removed `batch::Item::into()` which created a batch Item from a triple of
   VerifyingKey, Signature and message. Use the new `batch::Item::new()` instead
   (which can return an error).

frost-core/src/identifier.rs

@@ -5,10 +5,11 @@ use core::{
     hash::{Hash, Hasher},
 };
 
-use crate::{Ciphersuite, Error, Field, FieldError, Group, Scalar};
+use alloc::vec::Vec;
 
-#[cfg(feature = "serde")]
-use crate::serialization::ScalarSerialization;
+use crate::{
+    serialization::SerializableScalar, Ciphersuite, Error, Field, FieldError, Group, Scalar,
+};
 
 /// A FROST participant identifier.
 ///
@@ -18,23 +19,34 @@ use crate::serialization::ScalarSerialization;
 #[derive(Copy, Clone, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(feature = "serde", serde(bound = "C: Ciphersuite"))]
-#[cfg_attr(feature = "serde", serde(try_from = "ScalarSerialization<C>"))]
-#[cfg_attr(feature = "serde", serde(into = "ScalarSerialization<C>"))]
-pub struct Identifier<C: Ciphersuite>(Scalar<C>);
+// We use these to add a validation step since zero scalars should cause an
+// error when deserializing.
+#[cfg_attr(feature = "serde", serde(try_from = "SerializableScalar<C>"))]
+#[cfg_attr(feature = "serde", serde(into = "SerializableScalar<C>"))]
+pub struct Identifier<C: Ciphersuite>(SerializableScalar<C>);
 
 impl<C> Identifier<C>
 where
     C: Ciphersuite,
 {
     /// Create a new Identifier from a scalar. For internal use only.
-    fn new(scalar: Scalar<C>) -> Result<Self, Error<C>> {
+    #[cfg_attr(feature = "internals", visibility::make(pub))]
+    #[cfg_attr(docsrs, doc(cfg(feature = "internals")))]
+    pub(crate) fn new(scalar: Scalar<C>) -> Result<Self, Error<C>> {
         if scalar == <<C::Group as Group>::Field>::zero() {
             Err(FieldError::InvalidZeroScalar.into())
         } else {
-            Ok(Self(scalar))
+            Ok(Self(SerializableScalar(scalar)))
         }
     }
 
+    /// Get the inner scalar.
+    #[cfg_attr(feature = "internals", visibility::make(pub))]
+    #[cfg_attr(docsrs, doc(cfg(feature = "internals")))]
+    pub(crate) fn to_scalar(&self) -> Scalar<C> {
+        self.0 .0
+    }
+
     /// Derive an Identifier from an arbitrary byte string.
     ///
     /// This feature is not part of the specification and is just a convenient
@@ -50,39 +62,36 @@ where
     }
 
     /// Serialize the identifier using the ciphersuite encoding.
-    pub fn serialize(&self) -> <<C::Group as Group>::Field as Field>::Serialization {
-        <<C::Group as Group>::Field>::serialize(&self.0)
+    pub fn serialize(&self) -> Vec<u8> {
+        self.0.serialize()
     }
 
     /// Deserialize an Identifier from a serialized buffer.
     /// Returns an error if it attempts to deserialize zero.
-    pub fn deserialize(
-        buf: &<<C::Group as Group>::Field as Field>::Serialization,
-    ) -> Result<Self, Error<C>> {
-        let scalar = <<C::Group as Group>::Field>::deserialize(buf)?;
-        Self::new(scalar)
+    pub fn deserialize(bytes: &[u8]) -> Result<Self, Error<C>> {
+        Ok(Self(SerializableScalar::deserialize(bytes)?))
     }
 }
 
 #[cfg(feature = "serde")]
-impl<C> TryFrom<ScalarSerialization<C>> for Identifier<C>
+impl<C> TryFrom<SerializableScalar<C>> for Identifier<C>
 where
     C: Ciphersuite,
 {
     type Error = Error<C>;
 
-    fn try_from(value: ScalarSerialization<C>) -> Result<Self, Self::Error> {
-        Self::deserialize(&value.0)
+    fn try_from(s: SerializableScalar<C>) -> Result<Self, Self::Error> {
+        Self::new(s.0)
     }
 }
 
 #[cfg(feature = "serde")]
-impl<C> From<Identifier<C>> for ScalarSerialization<C>
+impl<C> From<Identifier<C>> for SerializableScalar<C>
 where
     C: Ciphersuite,
 {
-    fn from(value: Identifier<C>) -> Self {
-        Self(value.serialize())
+    fn from(i: Identifier<C>) -> Self {
+        i.0
     }
 }
 
@@ -94,9 +103,7 @@ where
 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_tuple("Identifier")
-            .field(&hex::encode(
-                <<C::Group as Group>::Field>::serialize(&self.0).as_ref(),
-            ))
+            .field(&hex::encode(self.serialize()))
             .finish()
     }
 }
@@ -107,9 +114,7 @@ where
     C: Ciphersuite,
 {
     fn hash<H: Hasher>(&self, state: &mut H) {
-        <<C::Group as Group>::Field>::serialize(&self.0)
-            .as_ref()
-            .hash(state)
+        self.serialize().hash(state)
     }
 }
 
@@ -118,8 +123,10 @@ where
     C: Ciphersuite,
 {
     fn cmp(&self, other: &Self) -> core::cmp::Ordering {
-        let serialized_self = <<C::Group as Group>::Field>::little_endian_serialize(&self.0);
-        let serialized_other = <<C::Group as Group>::Field>::little_endian_serialize(&other.0);
+        let serialized_self =
+            <<C::Group as Group>::Field>::little_endian_serialize(&self.to_scalar());
+        let serialized_other =
+            <<C::Group as Group>::Field>::little_endian_serialize(&other.to_scalar());
         // The default cmp uses lexicographic order; so we need the elements in big endian
         serialized_self
             .as_ref()
@@ -138,28 +145,6 @@ where
     }
 }
 
-impl<C> core::ops::Mul<Scalar<C>> for Identifier<C>
-where
-    C: Ciphersuite,
-{
-    type Output = Scalar<C>;
-
-    fn mul(self, scalar: Scalar<C>) -> Scalar<C> {
-        self.0 * scalar
-    }
-}
-
-impl<C> core::ops::Sub for Identifier<C>
-where
-    C: Ciphersuite,
-{
-    type Output = Self;
-
-    fn sub(self, rhs: Identifier<C>) -> Self::Output {
-        Self(self.0 - rhs.0)
-    }
-}
-
 impl<C> TryFrom<u16> for Identifier<C>
 where
     C: Ciphersuite,
@@ -182,7 +167,7 @@ where
                     sum = sum + one;
                 }
             }
-            Ok(Self(sum))
+            Self::new(sum)
         }
     }
 }