Fix serde implementation for serde_json

[xu-cheng]

Based on discussion in #140.

We use the serde_bytes crate for serialization implementations, which simplifies codes and fixes issues for serde_json.

[tarcieri]

Interesting. Perhaps we should also consider using this in the ed25519 crate to replace the current serializers, if they are similarly problematic with CBOR:

https://github.com/RustCrypto/signatures/blob/76008e8/ed25519/src/lib.rs#L282-L332

[xu-cheng]

The CI error seems to be unrelated to this PR.

Interesting. Perhaps we should also consider using this in the ed25519 crate to replace the current serializers, if they are similarly problematic with CBOR:

I don't know. Basically, it is a choice between to serialize as &[u8] or [u8; LEN]. In some formats like bincode, the latter is clearly better as it doesn't need to store the bytes length. In other formats like json, there is actually no difference. Then there comes with CBOR, which requires to encode the length for each element in the array. As such, using dynamic bytes array is actually better.

Currently there are two main issues:

• There are inconsistencies in the serde implementations. The signature is using [u8; LEN], while the other types are using &[u8]. It would be nice to have a consistent serde implementation between all types.
• If we change the serde implementation in either ways, it would be breaking changes.

My suggestion would be to merge this PR first, as it fixes json without introducing breaking changes. The we can figure out what is the long term approach, which would definitely require a breaking change.

[isislovecruft]

Hi @xu-cheng, thanks for the PRs. (And thanks @tarcieri for the input.)

Let me see if I understood this correctly:

1. We had inconsistent types in the various serde impls (&[u8] versus [u8; N]).
2. One of these types (which one???) is breaking the ability to interface with serde-json.
3. This API fixes the inconsistency in a completely backwards compatible way (i.e. it should be fine to merge for ed25519-dalek version 1.0.1).
4. Later we should decide if we want to use (de-)serialisers which are optimised towards either formats like CBOR which store the length or formats like JSON which do not.

Did I understand everything correctly?

FWIW I believe this is a breaking change by upgrading bincode, since the Infinite struct went away.

[isislovecruft]

Also, yes, the CI failure was due to #145; it should be fixed now.

[xu-cheng]

One of these types (which one???) is breaking the ability to interface with serde-json.

It is &[u8], which requires special handle during deserialization. Here we use serde_bytes crate to handle it automatically.

FWIW I believe this is a breaking change by upgrading bincode, since the Infinite struct went away.

bincode is a develop dependency, which is only used in tests. So updating it should be fine.

Did I understand everything correctly?

Yes.

So currently, Signature's serde is using [u8; N] (https://github.com/RustCrypto/signatures/blob/76008e8cd4d81203b791ec9672addd77c1e294f9/ed25519/src/lib.rs#L305), while all other types (public/secret key and keypair) are using &[u8] in serde.

• For json, [u8; N] and &[u8] yield the same result.
• For formats like bincode. [u8; N] is slightly more efficient as it avoids storing the extra length of the array.
• For CBOR, &[u8] is way more efficient. Fix serde implementation #140 (comment)

[isislovecruft]

@xu-cheng Thanks!

I'm not sure which way to go in the future, but this looks like a good start to at least get JSON working. Thanks so much for fixing this!