Sr25519 support

[nazar-pc]

Description

I'm not 100% confident this should be scoped to Rust implementation, but I need Sr25519 in Rust in addition to Ed25519 support to play well with the protocol we have

Motivation

In Subspace we use Sr25519 for several things and use public key of existing identity for implicitly storing certain pieces of data on network nodes and having the same curve used in libp2p (specifically public key for PeerId) would make things so much easier for us.

Requirements

1. Extend Keypair and Identity to support Sr25519 (I have schnorrkel crate in mind right now) behind feature flag

Open questions

Is it possible to make libp2p extensible such that curves like this can be added without changes to core libp2p implementation?

Are you planning to do it yourself in a pull request?

Yes

[tomaka]

One thing to note is that sr25519 is an invention from @w3f
It doesn't have any RFC or any standard specification, and there exists only one implementation

(I have schnorrkel crate in mind right now)

Yes, there's no other choice anyway

[nazar-pc]

I was thinking maybe add something like https://github.com/multiformats/multicodec/blob/master/table.csv for crypto in libp2p and allow implementations to optionally implement additional cryptography just like Secp256k1 and ECDSA are optional already?

[thomaseizinger]

FWIW, with #349, negotiating the security protocol is removed from the connection setup to mitigate downgrade attacks.

I am not sure if pluggable curves would follow the same spirit.

[nazar-pc]

For our network not advertising anything except Sr25519 would work as well, even though that'd technically conflict with "Peer Ids and Keys" spec 🤔

It would be really unfortunate to fork libp2p just for this and for libp2p to be stuck with just the curves available today forever.

[marten-seemann]

Requirements

1. Extend Keypair and Identity to support Sr25519 (I have schnorrkel crate in mind right now) behind feature flag

Open questions

Is it possible to make libp2p extensible such that curves like this can be added without changes to core libp2p implementation?

Not sure I understand the request here. Do you want to add a new key type to the peer ID specification? Or is this about extending the Rust implementation?

[nazar-pc]

This issue was moved from rust-libp2p repo originally.

I need it in Rust implementation in particular, but on libp2p community call we decided that it would make sense to have a more general discussion in specs repo.

Whether adding new optional key type to the spec or make it extensible in some generic way doesn't make much difference for me as a user right now.

[MarcoPolo]

I think the request here is to support Sr25519 in at least the rust implementation. I'm not familiar enough with Sr25519 to speak on whether it is stable and widely used enough to be part of the peer ID spec for everyone.

But maybe the request here should be to support a custom key type in the peer ID spec, then closed networks like these can use their own key types.

[nazar-pc]

Sr25519 is used in Polkadot/Kusama and every single parachain there by extension plus many other Substrate-based chains, so I'd say it is pretty stable in that sense.

And our network is not closed (in a sense that it is private), but it will likely be closed in a sense of not converging with IPFS's network simply because IPFS's network will not support some of the features, but that would be ideal long-term goal for us actually. So just using one custom key type wouldn't be ideal, something like multicodec table would be preferred.

[MarcoPolo]

Thanks for the elaboration.

Maybe custom isn't the correct word. What about a new key type in the protobuf that gets a new MulticodecPrefixed enum. The bytes are the multicodec concated with the bytes for key, i.e. <multicodec><keyBytes> (the codec can define if the keyBytes are prefixed with a uvarint). Users can then register key implementations by multicodec. Would that satisfy this use case?

[nazar-pc]

I'm not sure how that would work in the network where there might be multiple of those custom key types.

[MarcoPolo]

Each key type would register its own multicodec or use one in the private range. When encoding the key you'd specify the key type as the MultcodecPrefixedenum in the protobuf. The encoding would then be something like:

0x08                0x04                          <uvarint bytes len>         <multicodec bytes>     <key bytes>
(field 1, varint)   multicodeprefixed key type    How many bytes come next    Which multicodec       multicodec specific bytes

Implementations would look at the multicodec and see if they have something registered for it. If yes they can parse it and use it as normal, and if not then they ignore the peer (and fail to decode to the key)

[nazar-pc]

Yes, I think it would fit my use case.

[MarcoPolo]

To circle back here, I think we should make a new issue for a custom key type (open to naming suggestions) that's specified by a multicodec in the bytes payload of the protobuf as described here. The issue would serve as the starting off point for a change in the specs proper, and as a place other users can chime in if they would benefit from something like this. We can link this issue as a use case (and optionally close it if you'd like).

Currently I don't think this is a high priority change in the library since there don't seem to be many users who need to specify their own key types, but I could be wrong and having a dedicated issue for that could help surface other users and use cases.

Feel free to start the new issue @nazar-pc, thanks!