Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Make nodeID calculation pluggable #27

Closed
wants to merge 2 commits into from
Closed

Conversation

mishto
Copy link

@mishto mishto commented Dec 31, 2017

The calculation of the NodeID from the public key seems arbitrary and applications should be able to override this calculation in ways that align to their usage.

@Stebalien
Copy link
Member

So, everything we make expects that there exists a universal mapping of PublicKey -> ID. If we start having multiple IDs for the same public key, all of our id1 == id2 checks will start failing. We actually have an instance of this in #29.

What's your usecase?

@mishto
Copy link
Author

mishto commented Feb 7, 2018

So my understanding of libp2p from how it's "advertised" is that it's a general library that happens to be used by IPFS: "Modular peer-to-peer networking stack".

My use case has nothing to do with IPFS but I find libp2p be useful in our blockchain application. I was able to plug in libp2p to replace our implementation of a p2p protocol and remove some 1K lines of complicated, error prone code from our codebase. However, our node IDs for different networks are set and the nodeIDs have permissions associate with them, etc., and are difficult to change.

I actually didn't understand why there are 2 functions to calculate the peer ID IDFromEd25519PublicKey and IDFromPrivateKey. I believe the clean implementation would be to add GetID to the PubKey interface so that every type of supported or pluggable key can decide how to calculate their nodeID.

@Stebalien
Copy link
Member

So my understanding of libp2p from how it's "advertised" is that it's a general library that happens to be used by IPFS: "Modular peer-to-peer networking stack".

Yes but, we'd generally like interoperability. If people start calculating their peer IDs differently, applications that use libp2p won't be able to talk to each other.

I actually didn't understand why there are 2 functions to calculate the peer ID IDFromEd25519PublicKey and IDFromPrivateKey. I believe the clean implementation would be to add GetID to the PubKey interface so that every type of supported or pluggable key can decide how to calculate their nodeID.

I agree. I believe this was done because our crypto library, go-libp2p-crypto shouldn't know anything about peer IDs. I'm trying to fix this now actually...


So, how are you planning on deriving the peer ID? I assume you're introducing a new key type to do this? My only concern is that keys of a given type should have a consistent format.

@Stebalien
Copy link
Member

I agree. I believe this was done because our crypto library, go-libp2p-crypto shouldn't know anything about peer IDs. I'm trying to fix this now actually...

Actually, I'd really like peer IDs to just be CIDs. Why do you need them to carry extra data? Can that not go in the key?

@mishto
Copy link
Author

mishto commented Feb 9, 2018

So, how are you planning on deriving the peer ID? I assume you're introducing a new key type to do this? My only concern is that keys of a given type should have a consistent format.

Yes, we already made the change for UnmarshalPublicKey to be able to take other keys. If we make the keys responsible to calculate their ID then keys of same type should be consistent.

Actually, I'd really like peer IDs to just be CIDs.

What's CIDs?

@Stebalien
Copy link
Member

My question was more "why do you need to add extra data to the peer IDs" but I now see that that's not the case, you just want to avoid replacing all the existing peer IDs in your system (totally reasonable).

However, would it really be that difficult to translate between the two? I understand that this is less than ideal but... see below.

So we don't overlook any solutions, could you explain exactly what you need? How do you generate your key IDs, why/how do they differ from ours (different key serialization format?), etc.

If we make the keys responsible to calculate their ID then keys of same type should be consistent.

So, one issue here is that we'd like to inline small public keys (e.g., ed25519 keys) into the peer ID (so we don't have to hunt them down in the DHT, this is important for performance). See #30 for the state of this.

However, this assumes that peer IDs have some structure we can understand. If they're just arbitrary identifiers generated by the key, we'll have no way to introspect into them to determine if we can extract the public key or if we have to ask the DHT for it.

Now, this isn't a deal-breaker, but working around it will take some work.

(Note, if you're using ed25519 keys, you may be able to use this inlining to write a simple function that converts our (inlined) peer IDs to yours)

What's CIDs?

It's an IPLD thing (IPLD objects are named by CID). Basically, we'd like peer keys to be IPLD objects so we don't have to have bunch of special purpose logic to manage them (we could manage them like we manage all other IPLD data).

@Stebalien
Copy link
Member

Closing due to inactivity. Feel free to open this again.

@Stebalien Stebalien closed this Aug 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants