Public Key encoding in protobuf compatible serialization world

[zmanian]

We want to adopt tendermint/go-amino#276.

If we adopt full protobuf compatibility, this changes the byte encodings of validator consensus keys. These manifest in human and developer-facing interfaces such as the Cosmos SDK cosmosvalpub encoded interfaces and in the Tendermint RPC.

It is desirable to have a prefix that identifies the cryptographic system that a given public key belongs to prevent key confusion attack.

The legacy system is described in here. These encodings are created by the amino format but are trivial to implement as a customer serializer.

My recommendation is that we special case the serialization of public keys in Tendermint.

<PrefixBytes> <Length> <ByteArray> seems like generally reasonable format. For compatibility reasons, ed25519 and secp256k1 keys can keep their amino prefix bytes. New key types can select any prefix bytes they desire that don't collide with a key type in github.com/tendermint/spec repo.

A strawman alternative is drop legacy compatibility and just do <PrefixBytes> <ByteArray> and reset the prefix for ed25519 to 0x001 and sepc256k1 to 0x0021.

[jackzampolin]

This seems like something we could easily do in a switch statement when decoding the key. With strong documentation/reproducibility on the legacy for others who need it. I would imagine we would also need a rust impl here for signatory

[zmanian]

By switch statement you mean basically a look up table?

I need to check but signatory might just be doing this manually. We were doing that at one point.

The length value is most basically superfluous. It does potentially allow you to send both an x,y coordinate instead of just x or y. This usually called "compressed" form.

[jackzampolin]

By switch statement you mean basically a look up table?

exactly

[tac0turtle]

@zmanian is this still applicable since we now adopted amino flavored JSON?

privval json encoding stays the same. remote signer encoding will change to oneof, but this change will be handled on the hsm (kms) side correct?

[zmanian]

My overall impression is that we don't need to do this and can close this.