ZcashFoundation / frost

Rust implementation of FROST (Flexible Round-Optimised Schnorr Threshold signatures) by the Zcash Foundation
https://frost.zfnd.org
Other
133 stars 50 forks source link

Make serialization methods consistent #661

Closed conradoplg closed 2 months ago

conradoplg commented 3 months ago

There are currently 3 types of (de)serialization functions:

  1. fn deserialize(bytes: <Something>::Serialization) which is used for Scalar, Field, and Signature types and wrappers
  2. fn deserialize(bytes: &<Something>::Serialization) same as the previous, but with a reference
  3. fn deserialize(bytes: &[u8]) which is used for all other structs

The obvious inconsistency is between 1 and 2. I was changing all of them to take a reference (option 2) but thinking about it, I wonder if we should make them all follow option 3.

The <Something>::Serialization seems like an implementation detail which users shouldn't need to handle. Currently, to deserialize e.g. an identifier you need something like

        frost_core::Identifier::<C>::deserialize(
            &bytes
                .try_into()
                .map_err(|_| frost_core::Error::<C>::DeserializationError)?,
        )

which is super annoying. The user needs to lookup what the <<C::Group as Group>::Field as Field>::Serialization type is, and figure out that they have to use TryFrom<Vec<u8>>.

If we changed to option 3 in the public API (while still using <<C::Group as Group>::Field as Field>::Serialization internally) then they just have to pass a &[u8] which is more straightforward.