PeerId: Why protobuf wrapped as multihash?

[yojoe]

I was confused that a 32 byte Ed25519 pubkey ends up being 38 byte PeerId since the documentation only says:
The data is a multihash of the public key of the peer.

So I expected PeerId.to_bytes() to have a size of 34 bytes: 1 byte overhead for the multihash type, 1 byte for the size and 32 byte for the pubkey.

After a few hours of digging through the sources I figured out that PeerId is a multihash of type identity, which wraps a protobuf encoding of two fields: pubkey type (2 bytes), pubkey data (34 bytes).

Is there any reason why libp2p mixes multihash and protobuf? Both protocols - multihash and protobuf - kind of achieve the same thing, so only one of them would be sufficient to encode a self-describing public key. I would argue, why not only use multihash and get rid of the additional protobuf encoding? Multihash has the types of public keys libp2p uses right now already defined:

rsa-pub -> 0x1205
ed25519-pub -> 0xed
secp256k1-pub -> 0xe7

Using multihash-only for the PeerId would not only have the benefit of a more efficient size (4 bytes saved in the best case) and less complexity, but in contrast to protobuf it is really self-describing, while protobuf is application specific and can only be interpreted via the respective .proto definition.

Any thoughts?

[thomaseizinger]

In principle, you are right. There is room for optimisation.

This would have to be discussed on a spec level (see https://github.com/libp2p/specs).

Especially go-libp2p has many deployments outside of our control so any change would have to be rolled out carefully and in a backwards-compatible way.

Personally, I am not a huge fan of protobuf but I see the benefit when it comes to describing protocol messages. For simpler data types like PeerId, I'd be in favor of trying to remove it.

We'd also have to figure out how this interacts with https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md which we are yet to support in rust-libp2p. See #2259.

[yojoe]

This would have to be discussed on a spec level (see https://github.com/libp2p/specs).

Thanks! I wasn't aware that protobuf encoding is required for peer-id keys at the spec level (https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#keys).

[yojoe]

I created a pull request to improve the PeerId documentation a bit:
#3084