Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make serialization methods consistent #661

Closed
conradoplg opened this issue May 21, 2024 · 0 comments · Fixed by #674
Closed

Make serialization methods consistent #661

conradoplg opened this issue May 21, 2024 · 0 comments · Fixed by #674
Labels
I-usability FROST is hard to understand or use

Comments

@conradoplg
Copy link
Contributor

conradoplg commented May 21, 2024

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.

@mpguerra mpguerra added the I-usability FROST is hard to understand or use label May 22, 2024
@mpguerra mpguerra moved this to Product Backlog in FROST May 22, 2024
@mpguerra mpguerra moved this from Product Backlog to Sprint Backlog in FROST May 28, 2024
@conradoplg conradoplg changed the title SignatureShare::deserialize should take a reference Make serialization methods consistent Jun 4, 2024
@mpguerra mpguerra added this to the FROST v2.0.0 Release milestone Jun 5, 2024
@mpguerra mpguerra moved this from Sprint Backlog to In Progress in FROST Jun 18, 2024
@mpguerra mpguerra linked a pull request Jun 18, 2024 that will close this issue
@mpguerra mpguerra moved this from In Progress to Review/QA in FROST Jun 18, 2024
@mergify mergify bot closed this as completed in #674 Jun 20, 2024
@github-project-automation github-project-automation bot moved this from Review/QA to Done in FROST Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-usability FROST is hard to understand or use
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants