penumbra-zone / ed25519-consensus

Ed25519 suitable for use in consensus-critical contexts.
44 stars 12 forks source link

Use Base64 for Serde serialization by default #7

Open hdevalence opened 1 year ago

hdevalence commented 1 year ago

Currently, the provided serde implementations in this crate encode byte data as arrays. This causes very ugly output when using text-based formats like JSON or TOML, which encode an array of the numeric byte values.

Instead, it would be preferable if the default serialization used an intermediate encoding that could produce nicer output. One compatibility factor is that in Go, Ed25519 keys are usually modeled as byte[] values, and Go's encoding/json will Base64-encode byte[] values. This means that Go programs will often end up with Base64-encoded Ed25519 keys. For instance, I believe this is the reason Tendermint's priv_validator_key.json uses Base64. If we used Base64 as a default Serde encoding (rather than, e.g., hex), we would allow people to copy-paste encoded values more easily.

However, this would be a breaking API change. To understand the impact, I did a Github code search for code referencing this crate, and looked through every page of the results to scan for places that use ed25519_consensus types with Serialize/Deserialize. The only user I saw that wasn't already customizing Serde implementations was Anoma, but it looks like they primarily use a custom implementation of BorshDeserialize to actually encode and decode data.

Based on this search, it seems like there should be a pretty minimal impact, and that we could ship the encoding change in a 3.0 without causing much difficulty for downstream consumers to upgrade.

tzemanovic commented 1 year ago

we're using it in https://github.com/anoma/namada / Anoma, but as you mentioned, we're using borsh so we're not affected