Skip to content

feat(ed255519): Introduce serde_bytes optional dependency#337

Merged
tarcieri merged 1 commit intoRustCrypto:masterfrom
connec:serde_bytes_support
Jul 21, 2021
Merged

feat(ed255519): Introduce serde_bytes optional dependency#337
tarcieri merged 1 commit intoRustCrypto:masterfrom
connec:serde_bytes_support

Conversation

@connec
Copy link
Copy Markdown
Contributor

@connec connec commented Jul 21, 2021

This allows consumers to opt-in to serde_bytes support by setting the
std, serde, and serde_bytes features. This makes it convenient to
optimise (de)serialization for formats with efficient byte array
encodings, e.g.

#[derive(serde::Deserialize, serde::Serialize)]
struct MySignature {
    #[serde(with = "serde_bytes")]
    signature: ed25519::Signature,
}

Without first-class support for serde_bytes, consumers would have to
write their own (de)serialization machinery for this common case.

@connec
Copy link
Copy Markdown
Contributor Author

connec commented Jul 21, 2021

In contrast to #338 this has the advantage of keeping the features orthogonal, so --all-features still works fine and testing is kept simple. The downside is an additional dependency in the tree, and more complex integration for consumers (e.g. the need for an extra attr).

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Jul 21, 2021

@connec I think overall I prefer this approach to one which introduces crate features which don't properly unify i.e. #338

It does introduce an extra dependency, but I'm okay with that given the provenance of that particular dependency (i.e. it's by serde's authors and therefore is as trustworthy as serde itself)

@connec
Copy link
Copy Markdown
Contributor Author

connec commented Jul 21, 2021

I've pushed an update that removes the compile_error! and the dependence on TryFrom<Vec<T>> for [T; N]. For the latter, based on the docs I've just removed the visit_byte_buf method, since there's no way to create a >32 element array from a Vec with a copy as far as I can tell.

This allows consumers to opt-in to `serde_bytes` support by setting the
`serde_bytes` feature. This makes it convenient to optimise
(de)serialization for formats with efficient byte array encodings, e.g.

    #[derive(serde::Deserialize, serde::Serialize)]
    struct MySignature {
        #[serde(with = "serde_bytes")]
        signature: ed25519::Signature,
    }

Without first-class support for `serde_bytes`, consumers would have to
write their own (de)serialization machinery for this common case.
@tarcieri tarcieri merged commit 2de5f02 into RustCrypto:master Jul 21, 2021
@tarcieri
Copy link
Copy Markdown
Member

Looks good, thank you!

It might be good to add the example from the description to the rustdoc as well.

@connec connec deleted the serde_bytes_support branch July 21, 2021 18:08
@tarcieri tarcieri mentioned this pull request Jul 21, 2021
@tarcieri
Copy link
Copy Markdown
Member

Released in #340

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants