Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate private keys from the peer id representation #148

Open
peterbraden opened this issue Apr 29, 2021 · 3 comments
Open

Separate private keys from the peer id representation #148

peterbraden opened this issue Apr 29, 2021 · 3 comments
Labels
kind/discussion Topical discussion; usually not changes to codebase

Comments

@peterbraden
Copy link

PeerId's may or may not have a private key and we may need to detect this in other code.

The type of privKey is: public privKey: Uint8Array; , however currently the only way to workout if the peer id has a private key is to query peerId.privKey === undefined.

Alternatively we could document that checking whether peerId.privKey === undefined is a stable part of the API and update the types.

@vasco-santos vasco-santos added the need/triage Needs initial labeling and prioritization label Apr 29, 2021
@Gozala
Copy link

Gozala commented Jun 26, 2021

@vasco-santos why do we even have private key on PeerId ? It feels kind wrong, I think rust-libp2p takes a better approach by having a node identity encapsulating a key pair, from which PeerId is derived.

Is there reason why we could not do something along those lines in js ?

@vasco-santos
Copy link
Member

The PeerId has the private key, mostly for historical reasons. We used to leverage the PeerId instances to keep the Private Key of self (generated when creating libp2p) and to keep the Public Keys of the nodes we know of, which of course was a mess, specially when we ended up with multiple instances of PeerId and some had Public Key and others not.

Meanwhile, we added the PeerStore to libp2p, which opens the door to keep the public keys in the KeyBook instead of the PeerId. Having a node identity opens the door for storing the self private key (and also self peer record, which currently is stored in the AddressBook).

I think going towards the direction of not needing the PeerId as it is, and replace it to a single representation like your suggestion (binary tagger representation) would be great. But, this is a decision that needs a proper design issue taking into account all the angles, as this will change completely how libp2p is created (currently receives a PeerId or creates one) and how the encryption modules will act (they now receive the PeerId with the keys to do their thing).


@peterbraden what is the use case you have in mind for hasPrivateKey? Only self node with have access to its own PrivateKey and I feel that only libp2p core and libp2p encryption modules will need to interact with it

@peterbraden
Copy link
Author

It's pretty much exactly what you are saying - we want to differentiate between peer id's that can be used for 'self node' and the more common case where they just have a public key.

In our situation we want to initiate a node with a given PeerId, but require that that peer id has the private key in order to error if it is not supplied:

  public constructor(private id: PeerId) {
    if (!id.privKey) { throw new Error('We need a private key for ourself') }
    ...
  }

We would be better served by the different representation you suggest (a PublicKey type, and a KeyPair).

@jacobheun jacobheun added kind/discussion Topical discussion; usually not changes to codebase and removed need/triage Needs initial labeling and prioritization labels Aug 27, 2021
@jacobheun jacobheun changed the title Add hasPrivateKey method() Separate private keys from the peer id representation Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Topical discussion; usually not changes to codebase
Projects
None yet
Development

No branches or pull requests

4 participants