jellyfish icon indicating copy to clipboard operation
jellyfish copied to clipboard

[meta] consistent serde strategy

Open alxiong opened this issue 1 year ago • 1 comments

Status Quo

Our unwritten serde strategy has been:

our preference is to support ark_serialize (on cryptographic objects like fields, curve points, and structs that contains them) and then automatically derive serde support, Then we intend to expose only serde in our downstream API

There has been many refactoring effort in the past, including @jbearer's tagged proc macro, and the following in jf-utils:

https://github.com/EspressoSystems/jellyfish/blob/bdc6272b606c7ed29a00fa75abe46cbc6c3a72b3/utilities/src/serialize.rs#L44-L87

so that we could use:

#[derive(Serialize, Deserialize)]
struct MyStruct<T: CanoncialSerialize + CanonicalDeserialize> { 
    #[serde(with = "canonical")]
    pub root: T,
}

Scope

This issue aims to

  • re-investigate how consistent are we applying this strategy and if there's better alternatives. (as brought up in https://github.com/EspressoSystems/jellyfish/issues/273#issuecomment-1564336253)
  • clean up/remove unnecessary helper utility like pub struct CanonicalBytes
  • minimize unnecessary layers of indirection or serde between different formats
  • document down a guide on serde for jellyfish (including how to write unit tests for newly introduced structs etc.)

References

a relevant upstream issue to keep an eye on: https://github.com/arkworks-rs/algebra/issues/178

cc @EspressoSystems/jellyfish

alxiong avatar Jun 01 '23 04:06 alxiong

Another proposition:

Only cryptographic objects (like fields, groups) and structs that immediately contains a field of these types need to derive CanonicalSerde from ark-serialize and use some "ark-serde to serde" mechanism (let it be tagged proc macro or serde's field attributes like serde(with=..)). All others and "outer" wrapper structs only have to derive standard serde::{Serialize, Deserialize}.

Currently, there's an anti-pattern, say if I want my simple struct with primitive types to have tagged bytes during serialization, I need to derive canonical serde for it in order to use #[tagged("TAG")], which is unnecessary. Lots of boilerplate.

use tagged_base64::tagged;
use ark_serialize::*;

#[tagged("FOO")]
enum Foo {
  BAR(u8),
  BAZ(String),
}

// while we can directly derive standard serde for enum, canonical serde doesn't support enum
// therefore, we have to manually impl them.

impl CanonicalSerialize for Foo {...}
impl CanonicalDeserialize for Foo {...}
impl Valid for Foo {...}

cc @jbearer

alxiong avatar Jun 08 '23 13:06 alxiong