maidsafe-archive / crust

Reliable p2p network connections in Rust with NAT traversal. One of the most needed libraries for any server-less / decentralised projects
BSD 3-Clause "New" or "Revised" License
956 stars 126 forks source link

More generic peer ID #1128

Open povilasb opened 5 years ago

povilasb commented 5 years ago

ETD + review: 2 days

Starting with https://github.com/maidsafe/crust/issues/1116 we are trying to simplify Crust public API. Currently the peer ID is

pub struct PeerId {
    /// Public signing key is not used by Crust itself, but it is passed to Crust users when
    /// new peers are found.
    pub pub_sign_key: PublicSignKey,
    /// Crust uses this field to actually identify different peers.
    pub pub_enc_key: PublicEncryptKey,
}

There are two problems with this approach:

  1. Crust does not use pub_sign_key, it just passes it to upper layers. After many discussions we've decided Crust should not transfer signing key during connect. This signing key is used by MaidSafe Routing component and by ignoring it Crust will become more generic. Once connection is established upper layer can execute their own handshaking code which exchanges arbitrary info opaque to Crust.

  2. PeerId fields are exposed publicly. This allows upper layers to tie with Crust implementation details. Ideally upper layers should not care what Crust ID is, even if it's X25519 public key, etc.

dirvine commented 5 years ago

BEst of both worlds. Crust use a UUID type identifier. Then this can be a trait people implement for themselves. So routing, for instance, would do a UUID.from(PubKey) or similar. In that case, the upper layer is giving CRUST the UUID, it does not need to though as CRUST could create it's own, but if the upper layer always gave us that even from a utility CRUST provides such as UUID::Generate() or similar it may help. Anyway ideas to consider.

povilasb commented 5 years ago

Crust used to have Uid trait which we just recently removed for simplicity: https://github.com/maidsafe/crust/issues/1116

ustulation commented 5 years ago

As discussed and agreed in HO, crust will not use (U)UID but continue with PeerId as majority agreed that made the most sense.