search

multiformats / rust-multiaddr

multiaddr implementation in rust
https://crates.io/crates/multiaddr
Other
88 stars 47 forks source link

Making the `/p2p` protocol type-safe #73

Closed thomaseizinger closed 1 year ago

thomaseizinger commented 1 year ago

The /p2p protocol can only be followed by a PeerId: https://github.com/libp2p/specs/blob/master/addressing/README.md#the-p2p-multiaddr

As per spec, a PeerId can only be a multihash with either SHA256 or the identity hash in case the encoded public key is less than 42 bytes.

Currently, /p2p exposes a Multihash which is more general than that.

I see several options of how we can improve this situation:

  1. Extract the PeerId type from rust-libp2p into a dedicated crate and have multiaddr depend on that
  2. Move the entire identity module of libp2p-core into its own crate (keys + PeerId)
  3. Move the rust-libp2p PeerId type into multiaddr
  4. Define a minimal PeerId type within multiaddr that encodes the above invariants

All of the above have their pros and cons.

Extracting PeerId into its own crate feels a bit odd because it would be a very small crate. On the other hand, it would encode a very important concept in a concise form so it may be worth it.

The next step up would be a crate that encapsulates everything around keys in libp2p into its own crate, i.e. libp2p-identity. That is basically this module + the peer_id module.

Personally, I'd be in favor of option (2). I think it makes sense to break out this part into a separate crate. We can heavily feature-flag that one to the point where the multiaddr crate itself only depends on the bits that define the PeerId and doesn't come with any other dependencies.

Input welcome!

cc @mxinden @dignifiedquire

thomaseizinger commented 1 year ago

Cross referencing a discussion about API stability and extracting more crates in rust-libp2p: https://github.com/libp2p/rust-libp2p/discussions/3072#discussioncomment-4394785

thomaseizinger commented 1 year ago

(2) also makes sense when you consider that keys and PeerId are described in a single document in the spec: https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md

dignifiedquire commented 1 year ago

My preference would be (2)

mxinden commented 1 year ago

I am in favor of making /p2p type safe. I am indifferent on option 1 - 3, i.e. I am in support for all of them. I would consider (4) a compromise worth avoiding.

thomaseizinger commented 1 year ago

I will likely push (2) forward because it goes well with the planned modularization of libp2p-core.