Trait for identity::KeyPair

[CPerezz]

I'm not too deep into the lib. But one thing that I've seen (and that I would love to be different) is the fact that the library imposes the crypto primitives that you can use (in the actual repo status RSA, Ed25519 and Sepck251 IIRC).

It would be really nice to move the KeyPair struct towards being a trait (most of the impls are indeed implementing kinda off the same methods).

That would basically enable more projects and users to use libp2p2 as their core networking library. Since if they work with different curves or primitives, it's messy to force the stack to have different sets of keys (which come from different curves) ones for the networking stack and others for the protocol stack.

In our case at Dusk for example, we use Jubjub curve, which is a twisted edwards curve as ed25519. But there's no actual way (at least that it's not super hacky) to be able to use the JubJub curve Keypairs within the libp2p protocol too.

One of the main problems I've spot (just by a superficial look to the lib, I'm for sure missing a lot of things) is the fact that we're using protobuf and hardcoding the primitive used for the KeyPair as ID inside of one of the proto fields.
But I do think this can be workarrounded no?

Have there been any thoughts about that in the past? Is this something that would be desired to have in the lib? I would appreciate some feedback and in case it's something wanted, I'm happy to help to design/implement the solution!

[mxinden]

While KeyPair being a struct instead of a trait is a rust-libp2p specific thing, the selection of key types are part of the libp2p specification: https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md

That said, I am not opposed to supporting external crypto primitives as key pairs. Obviously, rust-libp2p can not implement all curves out there directly, thus an abstraction like a KeyPair trait seems worth considering. I am not sure how intrusive the additional trait would be to the codebase though. The Protobuf KeyType enum could likely be extended, though extra care would need to be taken not to introduce conflicts in the future.

Note other projects like Substrate decouple network keys from chain keys, going with the libp2p spec'ed keys on the network.

[CPerezz]

AFAIU the requirements for the KeyPair apart from the typical ops to encrypt or sign stuff would mainly be serialization/derivation and things that can be specified in a spec. Also, it's not libp2p's responsibility to make sure they're implemented correctly (libp2p would just provides the traits).

Obviously you'll know better which are the implications of this change. But If the trait is defined here, we can still implement it for the actual primitives "wrapping" the actual implementation and porting the crate to trait-base KeyPair usage.

I'm happy to give it a try and do a first exploration impl to see how it goes. But first of all it would be nice to see if there's anything against that or someone else sees big issues on addressing this feature.

[mxinden]

Obviously you'll know better which are the implications of this change.

The only caveat I see today is the additional trait parameter we would need and how well that would play along with supporting multiple key types at runtime.

I'm happy to give it a try and do a first exploration impl to see how it goes.

Sure, sounds good. Though keep in mind that we might not accept it in case it is not worth the trade offs.