nucypher / rust-umbral

Umbral implementation in Rust
52 stars 20 forks source link

Use `serde` exclusively for serialization #109

Closed fjarri closed 1 year ago

fjarri commented 1 year ago

Currently we have a two-step system: umbral objects are serialized manually as byte blobs (with SerializableToArray trait) and then the result can be (optionally) serialized with serde (to enable composition in nucypher-core). Originally, this choice was made for the following reasons:

I don't think there was anything else, was there?

So I think it might be the time to retire SerializableToArray/DeserializableFromArray and just use serde. The upside is significant code simplification and less boilerplate when adding new structures (which I bumped into when working on #107). The downsides are:

Speaking of the last point, we might as well just make serde a mandatory dependency (this will help avoid some problems with documenting the serde trait support, too).

Also this will resolve #95 - we will just use hex for small objects and that's it.

@piotr-roslaniec, @jMyles, @KPrasch, @theref - thoughts?

piotr-roslaniec commented 1 year ago

I don't think there was anything else, was there?

I don't recall any other reasons.

Changed ABI, so protocol versions in nucypher-core will have to be bumped.

IMHO benefits outweigh this minor inconvenience. To my best knowledge there are few adopters of nucypher-core, and from the perspective of nucypher-ts, this is not an issue (breaking change, transparent to the user).

Less efficient packaging in text formats (...)

So this is a breaking change in terms of serialization behavior, but not for the clients that implement their own serialization format (e.g. serialize into raw bytes, then serialize into base64 in the client). Do you think it would be possible to use serde_as macros to circumvent this issue? I don't have enough experience with it to recommend it for this particular issue, so just throwing an idea.

All the users who need serialization (and umbral is not particularly useful without it) will have to use serde and a crate for whatever format they use, which are somewhat heavy dependencies.

I think serde is so heavily entrenched in the crate ecosystem some users may view it only as a benefit. I try to implement a philosophy of refactoring into a blessed crate when possible.

fjarri commented 1 year ago

So this is a breaking change in terms of serialization behavior, but not for the clients that implement their own serialization format (e.g. serialize into raw bytes, then serialize into base64 in the client).

No, for them too, since the binary format will change as well.

Do you think it would be possible to use serde_as macros to circumvent this issue?

Unfortunately it does not change serde's behavior with arrays, the length is still serialized.

fjarri commented 1 year ago

Some specific things to consider in relation with this: