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

Specify peer ID hash function in public key #111

Closed
Stebalien opened this issue Nov 14, 2018 · 67 comments · Fixed by libp2p/go-libp2p-peer#40
Closed

Specify peer ID hash function in public key #111

Stebalien opened this issue Nov 14, 2018 · 67 comments · Fixed by libp2p/go-libp2p-peer#40
Assignees

Comments

@Stebalien
Copy link
Member

In the beginning, peer IDs were base58 encoded sha256 multihashes. However, being able to embed a key into a peer ID was rather tempting so we introduced a IDFromEd25519PublicKey function. This function created a different peer ID that embedded the public key.

Unfortunately,

  1. Users had to remember to call this.
  2. If a user called the normal IDFromPublicKey function, they'd get a different peer ID for the same key. This obviously isn't good as our security transports derive peer IDs from public keys instead of validating peer IDs (this makes it impossible to mis-validate).

Therefore, I submitted libp2p/go-libp2p-peer#30 which:

  1. Removed IDFromEd25519PublicKey.
  2. Changed inline peer IDs to just inline the protobuf (instead of introducing an entirely new format).
  3. Auto-inlined peer ids <= 42 bytes.

Unfortunately, this last bit appears to be a problem... At the time, I was under the impression that nobody had actually gotten around to using ed25519 keys as inlining didn't really work (libp2p would still use the sha256 peer IDs). However, it turns out that OpenBazaar was using them anyways.

Second, in retrospect, this wasn't even the optimal solution. Really, we also need to support arbitrary hash functions but this only adds support for the identity hash function.


So, my proposal here is to:

  1. Revert that change. I'm pretty sure OpenBazaar is the only ed25519 user in the wild so that should be safe.
  2. Introduce a new field in the public/private key protobuf that explicitly specifies the hash function (defaulting to "sha256" if missing).

Open question: Should we use this as an opportunity to switch to multibase-enabled peer IDs now? One significant issue with the current embedding method is that it makes it slightly harder to switch to multibase peer IDs (cc @lidel). I've previously proposed switching to CID-based peer IDs but that may be more work than it's worth (a motivation would be transfering keys over bitswap, easier to support large quantum-safe keys, format flexibility, etc.).

@Stebalien
Copy link
Member Author

@lidel
Copy link
Member

lidel commented Nov 14, 2018

Current representation of PeerIDs is base58, which is case-sensitive and thus not Origin-safe. In the context of IPFS CIDs switching to base32 the time feels right to solve Origin problem for IPNS websites as well.

So the ask from Web Browsers WG is:

@tomaka
Copy link
Member

tomaka commented Nov 14, 2018

To me having a peer ID that is anything more than the hash of a public key would feel very over-engineered.

@Stebalien
Copy link
Member Author

To me having a peer ID that is anything more than the hash of a public key would feel very over-engineered.

So, at the very least, we need to support multiple hash functions. That's what this is about.

The idea behind using CIDs is that would bring this in-line with everything else. In addition to being able to upgrade the hash function, we'd:

  • Get multibase for free.
  • Be able to change the underlying key format.
  • Be able to fetch keys over bitswap (maybe).
  • (maybe) be able to use IPLD-based identity systems as peer IDs.

Of course, the other side of this issue is that we'd be making libp2p depend on IPLD and I understand that the libp2p team may not be happy with that. So, let's try to not tie all of these together.

@Stebalien
Copy link
Member Author

Stebalien commented Nov 14, 2018

Concrete non-CID proposal (kill two birds with one stone):

  1. Remove (for now) support for embedding ed25519 keys.
  2. Add support for parsing multibase-prefixed peer IDs (in text).
  3. Add read support for multiple hash functions but tell people to not use other hash functions yet.
  4. ⏳ ⌛ ⏳ ⌛
  5. Switch to formatting peer IDs using multibase by default.
  6. Expose support for multiple hash functions. All new ed25519 keys specify the "identity" hash function by default.

Note 1: 5 is not a network-breaking change as these won't cross the network anyways.


My concern is that this does make switching to CID-backed peer IDs painful. Again, we don't need that to be a network-breaking change as we can continue using raw multihash CIDs on the network (for now) however, technically, we could run into issues if we ever bump the CID version number to something that's a valid multihash codec.

@Stebalien
Copy link
Member Author

Stebalien commented Nov 14, 2018

Concrete CID proposal (kill three birds with a planet):

  1. Remove (for now) support for embedding ed25519 keys.
  2. Add support for parsing CID-v1 peer IDs (in text). In binary, we store them as multihashes.
  3. Add read support for multiple hash functions but tell people to not use other hash functions yet.
  4. ⏳ ⌛ ⏳ ⌛
  5. Switch to formatting peer IDs as CIDs (in text) by default.
  6. Expose support for multiple hash functions. All new ed25519 keys specify the "identity" hash function by default.

This doesn't add full CID-peer-ID support. Instead, it just gives us a way to convert between them and allows us to use CIDs in text (and gives us some room to upgrade later).

@daviddias
Copy link
Member

Using CIDs has another benefit. As we move to accepting multiple keys, we will eventually need to think of a key as a file that might have multiple GBs and using the same identified that we use for content (therefore it being called Content Identified) will enable us to support that smoothly. More notes on ipfs/specs#58

@Stebalien
Copy link
Member Author

Concrete CIDs all the way proposal (that's no moon):

  1. Remove (for now) support for embedding ed25519 keys.
  2. Add support for parsing CID-v1 peer IDs (in text).
  3. Add a concept of PIDv0. That is, all peer IDs for libp2p-crypto protobuf keys that don't specify their hash function get encoded to a multihash on the network. Others get encoded to PIDv1.
  4. Add read support for multiple hash functions but tell people to not use other hash functions yet.
  5. ⏳ ⌛ ⏳ ⌛
  6. Switch to formatting peer IDs as CIDs by default.
  7. Add support for multiple hash functions.
  8. Eventually, start (a) specifying SHA256 in key protobufs by default (forcing them to use PIDv1). At the same time, switch to ed25519 by default because it's better in every way anyways.

The primary reason to go with this over the "kill three birds with a planet" scheme is that that scheme only works as long as a CID version number never conflicts with a multihash code (other than 0). Once one does, we won't be able to distinguish between a PIDv0 and a CIDvN.

@tomaka
Copy link
Member

tomaka commented Nov 15, 2018

I don't think the text format of a peer ID should be relevant to the libp2p specs at all. This spec is about how things are transmitted over the network, and the peer ID is never transmitted in text format as far as I know.

Transmitting peer IDs as CIDs over the network would be wasteful, as you're only ever going to require raw data anyway if you don't want to run into tons of crazy corner cases.

If IPFS or IPLD or Filecoin want to format peer IDs as CIDs, they can do so and decode the ID into a multihash before using it in libp2p. However that would not be relevant to the libp2p specs.

@tomaka
Copy link
Member

tomaka commented Nov 15, 2018

We also need to mention that connecting to a peer whose public key reports a different hash function that the one we have should be treated as a key mismatch, even if the hash itself is correct.

Supporting multiple hash functions raises the question of the number of kbuckets in Kademlia, however I'm not an expert enough to know if this is actually a problem, or if it's attackable.

@vyzo
Copy link
Contributor

vyzo commented Nov 15, 2018

I very much want to preserve public key inlining in the identity, as it is a key property for pusbub message signing to be efficient.
Can we make sure we have inline keys from the get-go with whatever scheme we end up adopting?

@Stebalien
Copy link
Member Author

I don't think the text format of a peer ID should be relevant to the libp2p specs at all. This spec is about how things are transmitted over the network, and the peer ID is never transmitted in text format as far as I know.

Well, given that this is a system for humans, I'd have to strongly disagree. There would be no web without URLs/URIs.

These IDs do show up in text all the time. Usually in the form /p2p/... (well, right now we usually use /ipfs/Qm... but we're working on switching to /p2p/... to remove the association with ipfs).

Transmitting peer IDs as CIDs over the network would be wasteful, as you're only ever going to require raw data anyway if you don't want to run into tons of crazy corner cases.

CIDs give us flexibility. With raw multihashes, we're stuck with the current key format and can't change it. With CIDs, we can introduce new formats unambiguously (because the CID itself states the key format).

Supporting multiple hash functions raises the question of the number of kbuckets in Kademlia, however I'm not an expert enough to know if this is actually a problem, or if it's attackable.

KAD re-hashes everything anyways using sha2-256.

We also need to mention that connecting to a peer whose public key reports a different hash function that the one we have should be treated as a key mismatch, even if the hash itself is correct.

That would be a mismatch. We'd be mapping two names (peer IDs) onto two different peers. We never want to end up in a situation where we assign more than one peer ID to a single, logical, identity.

Can we make sure we have inline keys from the get-go with whatever scheme we end up adopting?

We can keep support, we just need to avoid creating keys with inlined peer IDs by default.

@tomaka
Copy link
Member

tomaka commented Nov 18, 2018

These IDs do show up in text all the time. Usually in the form /p2p/... (well, right now we usually use /ipfs/Qm... but we're working on switching to /p2p/... to remove the association with ipfs).

They show up only if you use the IPFS, Filecoin or IPNS CLI. As far as I understand, libp2p is supposed to be purely a library. Things built on top of libp2p can decide whether to a multibase and convert that to a multihash before passing to libp2p.

CIDs give us flexibility. With raw multihashes, we're stuck with the current key format and can't change it. With CIDs, we can introduce new formats unambiguously (because the CID itself states the key format).

The peer ID is the hash of an arbitrary vector of bytes. Right now this underlying vector of bytes is a protobuf struct, but could be changed to be anything.

@tomaka
Copy link
Member

tomaka commented Nov 18, 2018

KAD re-hashes everything anyways using sha2-256.

What do you mean "re-hashes"?
My concern is: if I perform a Kademlia FIND_NODE RPC query, and a remote send back a result containing SHA512 multihash, what is our local node supposed to do? Extend itself to 512 kbuckets?

@Stebalien
Copy link
Member Author

What do you mean "re-hashes"?

We treat peer IDs as opaque keys and always hash them with sha256 (like we do with other keys). Basically, we make no assumptions about the peer ID format when figuring out where a peer ID fits in the DHT keyspace.

As far as I understand, libp2p is supposed to be purely a library. Things built on top of libp2p can decide whether to a multibase and convert that to a multihash before passing to libp2p.

Libp2p is a library but it's also an interoperable network protocol. Peer IDs need interoperable, human-readable forms just like IP addresses, MAC addresses, etc.

The peer ID is the hash of an arbitrary vector of bytes. Right now this underlying vector of bytes is a protobuf struct, but could be changed to be anything.

You'd have to somehow communicate how the this "arbitrary byte vector" should be decoded and how it should be understood as a public key. We currently use the codec in CIDs to communicate this kind of information.

@tomaka
Copy link
Member

tomaka commented Nov 20, 2018

Libp2p is a library but it's also an interoperable network protocol. Peer IDs need interoperable, human-readable forms just like IP addresses, MAC addresses, etc.

But why do they need need a human-readable form, since they are only ever transmitted as bytes over the network?

I'm not saying their human-readable form shouldn't be defined. I'm saying that the network protocol shouldn't be changed because of out-of-scope human-readable reasons.

@tomaka
Copy link
Member

tomaka commented Nov 20, 2018

We treat peer IDs as opaque keys and always hash them with sha256 (like we do with other keys).

Does that mean that if a remote says that its public key should be hashed with sha512, then we hash it with sha256 anyway for DHT insertion purposes?

But then, if we receive a Kademlia RPC query, do we answer with sha256 hashes, or do we answer with sha512 hashes? Or both (the protocol can't do both at the moment)?
If we answer with the sha256 hash, then we have the problem of multiple peer IDs pointing to the same thing.
If we answer with the sha512 hash, then the remote has to connect to the peer first in order to determine its public key and hash it as sha256.
If we answer with both, we need to modify the Kademlia protocol, and also the remote can't verify that the hashes actually correspond to the same public key.

@ghost
Copy link

ghost commented Nov 20, 2018

Hey, can't quite pull up all the context to this at the moment, but my intuition in the past has been to make PeerIDs a subset of CIDs (so that the meaning of "CID" would change to "Cryptographic IDentifier"), and to do much of the same CIDv0/CIDv1 dance we did in content-land. It'd avoid duplication of concepts/protocols, and it'd be a migration that we already have some experience in.

It would also be super useful to do the same base32 thing that we're doing with content CIDs, so we can seamlessly use the peer CIDs in URLs.

I sense the discussion above is already going into that direction, and I can't add anything new to that at the moment, so I'll leave it at that :)

But why do they need need a human-readable form, since they are only ever transmitted as bytes over the network?

They get put into multiaddrs all the time (/p2p/<peerid> or legacy /ipfs/<peerid>), and any multiaddr protocol must have a human-readable representation. Addresses get logged in libp2p itself, and blessed libp2p tools have UIs as well (e.g. daemon, testing tools, etc.).

@tomaka
Copy link
Member

tomaka commented Nov 20, 2018

They get put into multiaddrs all the time (/p2p/ or legacy /ipfs/), and any multiaddr protocol must have a human-readable representation.

That's just moving the problem. Multiaddresses are also only ever transmitted as binary, and thus don't need to have their human-readable version defined as part of the network protocol.

I'm extremely confused as to why it's a good idea to make machine A decide in which format a peer id should be displayed on machine B, and not just let machine B decide.

Addresses get logged in libp2p itself, and blessed libp2p tools have UIs as well (e.g. daemon, testing tools, etc.).

That's not related to the wire protocol, which is what this is all about.

@whyrusleeping
Copy link
Contributor

Some notes from skimming this:

  • Kademlia hashes the binary encoded form of the peerID with sha256. The contents of the peer ID don't change this at all. The fact that peerIDs are currently the sha256 hash of the encoded public key has no effect on the fact that kademlia uses a sha256 hash. The keys here are just binary blobs.

@tomaka

To me having a peer ID that is anything more than the hash of a public key would feel very over-engineered.

What about having the peerID actually be the public key? I think that its unarguable how useful that is. How would you go about representing that distinction over the wire?

  • fragmentation in the human readable format for addresses smells bad to me. I think everyone agrees about how to write a human readable ipv4 address, Different systems don't render those differently (to the best of my knowledge). I'm not saying different systems can't choose to do that, they most certainly can, but I think that choosing to do so would cause more confusion to users.

@tomaka
Copy link
Member

tomaka commented Nov 20, 2018

Kademlia hashes the binary encoded form of the peerID with sha256. The contents of the peer ID don't change this at all. The fact that peerIDs are currently the sha256 hash of the encoded public key has no effect on the fact that kademlia uses a sha256 hash. The keys here are just binary blobs.

Does that mean that you store both the peerID and the sha256 of the public key?
I raised a few more questions in the comment above.

How would you go about representing that distinction over the wire?

I'm not sure I understand the question. We know whether we receive a peer ID or a public key based on the protocol. For example secio and identify send us a public key, while relay sends us peer IDs.

fragmentation in the human readable format for addresses smells bad to me. I think everyone agrees about how to write a human readable ipv4 address, Different systems don't render those differently (to the best of my knowledge). I'm not saying different systems can't choose to do that, they most certainly can, but I think that choosing to do so would cause more confusion to users.

I agree that everything should have the same human-readable representation, but I don't see how it is a good solution to let a remote node on the network choose what the representation of their key should be.

@Stebalien
Copy link
Member Author

I'm not saying their human-readable form shouldn't be defined. I'm saying that the network protocol shouldn't be changed because of out-of-scope human-readable reasons.

I completely agree. That's why I listed three options:

  1. Option 1 solves the multibase issue. This is purely a display issue.
  2. Let's just forget about option 2. That was trying to partially switch to CIDs while punting the hard work till later. However, it provided little to none of the benefits of option 3 while introducing complexity not present in option 1.
  3. Option 3 solves the multibase issue and then "futureproofs" peer IDs: the codec in the CID gives us room to add entirely new key formats (not just new key algorithms using the same protobuf format). This isn't about display at this point.

Why bundle these questions together? Unfortunately, wire format changes tend to affect display and parsing.

  1. If we support multiple hash functions but punt on this, we can't adopt multibase unambiguously (because a base58 encoded multihash may conflict with a multibase prefix).
  2. If we go with option 1 and then decide that we'd like to switch to full CIDs somewhere down the road, we may run into cases where it's difficult to determine if something is a raw multihash or a CID. If we switch now, we know that the only raw multihashes are sha2-256 multihashes so this is pretty easy (we already have to do this in ipfs/ipld so it's nothing new).

I agree that everything should have the same human-readable representation, but I don't see how it is a good solution to let a remote node on the network choose what the representation of their key should be.

That was never the intention here. The key's creator needs to be able to choose the hash function used in the peer ID so they don't end up with multiple peer IDs (not for display, for code) however, they don't get to, e.g., decide on the base encoding the user decides to use.

@tomaka
Copy link
Member

tomaka commented Nov 21, 2018

That was never the intention here. The key's creator needs to be able to choose the hash function used in the peer ID so they don't end up with multiple peer IDs (not for display, for code) however, they don't get to, e.g., decide on the base encoding the user decides to use.

I guess I misunderstood then.
The CID specs describe a CID as having a multibase, which is what I was talking about all along: https://github.com/ipld/cid#how-does-it-work---protocol-description
I guess you wouldn't have that multibase on the wire? (but then shouldn't it be called something else than a CID?)

@tomaka
Copy link
Member

tomaka commented Nov 21, 2018

I would still love to get some clarifications on how Kademlia would work after allowing hashes other than SHA-256.

@Stebalien
Copy link
Member Author

I guess you wouldn't have that multibase on the wire? (but then shouldn't it be called something else than a CID?)

Technically, it does. However, we don't actually do this anywhere (well, except for the CBOR format) because it's redundant (and I'm certainly not going to argue for it).

I would still love to get some clarifications on how Kademlia would work after allowing hashes other than SHA-256.

Kademlia uses sha-256 and only sha-256 for all keys. Given a peer ID ID = SOME_HASH(publicKey), we map this into kademlia's key-space with SHA256(ID).

Note: This does mean that SOME_HASH needs to be deterministic for a given key (otherwise the DHT position changes). That's why this proposal adds a HashFunction field to the public key datastructure.

@vyzo
Copy link
Contributor

vyzo commented Jan 7, 2019

that's a backwards incompatible change, we can't do that.

@tomaka
Copy link
Member

tomaka commented Jan 7, 2019

You can have a new version of floodsub and support both the old and new versions for a while.
This is the similar to having newer nodes sending you the inline key and old nodes sending you the hashed key.

@Stebalien
Copy link
Member Author

IMO, the original motivations for embedding the key in the peer ID has now mostly been obsoleted by simply embedding the key in the record (which I believe is the "more correct" solution). We can make the peer ID optional and eventually stop including it in favor of simply deriving it from the key. However, we should still support alternative hash functions for peer IDs:

  1. If we ever need to encrypt to a peer without a session (e.g., in a high-latency packet protocol), we may need the key up-front. In that case, we can save a round-trip if we can extract it from the peer ID.
  2. We may want to eventually move away from sha256 for either security or performance reasons. That means we need to somehow support alternative hashing algorithms.

@vyzo
Copy link
Contributor

vyzo commented Jan 15, 2019

A potential killer app for inlined keys is packet switching and packet protocols in general.

@anacrolix
Copy link

What multicodec code would be used in a CID scheme? What advantages/disadvantages are there for making a peer ID code?

@Zolmeister
Copy link

Zolmeister commented Jan 18, 2019

@anacrolix Implying (?!) that you could actually replace ipns:// with ipfs://. Interesting.

@Stebalien
Copy link
Member Author

@Zolmeister no.

@Stebalien
Copy link
Member Author

cc @andrewxhill, @momack2, @whyrusleeping

(please read #111 (comment) for context)

Update:

It turns out Textile was using ed25519 keys, with inlining. Other than requiring a forced-upgrade of the textile network, the only solution I can see for now is:

  1. Move forward on adding support for additional hash functions.
  2. Introduce a go-ipfs repo migration that adds the hash function to the public/private keys when appropriate (i.e., when not sha256). Textile may have to replicate this migration in their own software.
  3. Remove the hash function field from the protobuf when inlining. This will ensure that textile's peer IDs don't change. Unfortunately, this adds a special rule (which only applies to inlined keys so it's not that bad).
  4. Go with CIDs in text only (Specify peer ID hash function in public key #111 (comment)) proposal immediately. Unfortunately, modifying the key before hashing (computing the identity hash) throws a wrench in the whole CIDs as peer IDs proposal.

And do this all before the next go-ipfs release, which is almost a month overdue... 😭.


@andrewxhill does Textile use IPNS and, if so, how? Also, does textile hardcode any string-formatted ed25519 peer IDs anywhere that can't be automatically migrated? I ask because of consideration 3 above. That is, if we change the textual format of inlined peer IDs, will that be an issue?

Note: We can add a special case for base58 encoded inlined peer IDs for some migration period but that's not a permanent solution.

@sanderpick
Copy link

Thanks for the context... very helpful. The main reason I went this route was to avoid having to look up / save a fellow peer's public key as well as its ID within application logic. Also, they're much faster to generate than RSA keys (very noticeable difference on mobile).

We have been using IPNS for our profiles. However, for various reasons, we've moved away from that approach. Our next release does not use IPNS.

The only place we hardcode peer IDs is for our bootstrap peers. If the solution involves changing peer ID format, we'd have to come up with a migration for our groups, as well as apply the IPFS repo migration. Each peer would announce their new ID. Not the end of the world, but it would require some work... a migration period would be helpful.

@Stebalien
Copy link
Member Author

Note: the current proposal involves changing the textual format, not the network/binary format.

@sanderpick
Copy link

sanderpick commented Jan 22, 2019 via email

@Stebalien
Copy link
Member Author

Thinking through this a bit, my proposal doesn't work without a full network (go-ipfs and Textile) upgrade. So, we'll need a migration period.

New proposal: Same as above except:

  1. We provide a package-level variable in go-libp2p-peer to keep the peer inlining behavior present in 0.4.18. OpenBazaar has a forked network so they can easily turn this off but go-ipfs and Textile can leave it on.
  2. We add the migration to add the new "multihash hash function" field to the ed25519 keys.
  3. Once the network has had some time to upgrade (run the migration), we can turn this switch off by default.

Are you thinking this would land sometime next week after your team retreat?

Hopefully by the end of this week. We'll see what other snags we hit.

@daviddias
Copy link
Member

daviddias commented Jan 27, 2019

Once the network has had some time to upgrade (run the migration), we can turn this switch off by default.

What is "some time"? Two releases? 3 Months?

How will his affect the connectivity with JS/Rust nodes? Can we have some interop tests written at https://github.com/ipfs/interop before doing the release that test connectivity with different key combinations, namely:

  • goRSA to goRSA
  • goEd25519 to goEd25519
  • goRSA to goEd25519
  • jsRSA to jsRSA
  • jsEd25519 to jsEd25519
  • jsRSA to jsEd25519
  • goRSA to jsRSA
  • goEd25519 to jsRSA
  • goRSA to jsEd25519
  • goEd25519 to jsEd25519

@jonnycrunch
Copy link

@jonnycrunch just adding myself to the thread. with IPID DID method spec i have been publishing the DID document as IPLD to IPNS using ED25519 key (supported), but the actual DID public keys that published are secp256k1. For DID authentication, I was hoping to be as simple as QUIC over libp2p, but this won't be supported with secp256k1.

@Stebalien
Copy link
Member Author

What is "some time"? Two releases? 3 Months?

Once N% of nodes have upgraded, ideally. We can pull this information from the bootstrappers.

How will his affect the connectivity with JS/Rust nodes?

It will restore interoperability with ed25519 between go and js/rust.


@jonnycrunch what do your IPNS IDs look like?

@Stebalien
Copy link
Member Author

See #138 for a summary.

@jonnycrunch
Copy link

@Stebalien go-ipfs version 0.4.18 published to ed25519

ipfs key gen -t ed25519 my_ed25519_key

ipfs key list -l
12D3KooWJYcBs1QeeaxArMXowBN3Y4UamTSpkLjv4THrUhxMr6Jo my_ed25519_key

@jonnycrunch
Copy link

@Stebalien or using secp256k1 with bip32 HD wallet and hashing using Keccak256 as in ethereum with output as hex:

0x122f0fDcfAaC745381454C32648436bdfe4cBdda

@Stebalien
Copy link
Member Author

Got it.

lidel added a commit to libp2p/go-libp2p-core that referenced this issue Dec 10, 2019
What:

1. Supports decoding CIDs (of libp2p keys) as peer IDs 
   (libp2p/specs#216)
2. Continues to encode peer IDs as base58 multihashes by default.
3. Adds functions for converting between peer IDs and CIDs.
4. Deprecates IDB58{Decode,Encode}, replacing them with {Decode,Encode}.

Why:

1. We _need_ to support multibase somehow, so we can put peer IDs in domains.
2. This makes peer IDs fully self describing. That is, the CID itself indicates that it's a hash of a libp2p public key.
3. It's much easier to upgrade wire protocols than text. This change punts
pids-as-cids on the wire down the road but that's something we can revisit if it ever becomes relevant. 

See libp2p/specs#111 for context.

Deviations from that issue:

* This _retains_ the current peer ID inlining of ed25519 keys. That turned out to be a nightmare, one I'd like to punt a bit more down the road.
* Likewise, this _punts_ the question of embedding the multi-hash algorithm in the public key.
@Stebalien
Copy link
Member Author

I'm going to briefly revive this issue in order to kill it.

I still believe we should support specifying the hash function inside the public key. However, I don't believe we should do it as a mitigation for #138.

My original motivation was largely to undo a breaking change I had made. Unfortunately, reverting a breaking change is also a breaking change.

When I first proposed this solution, I was under the impression that only OpenBazaar was using Ed25519 keys. Since then, we've learned that:

  • Textile is also using Ed25519 keys and is using them for IPNS records.
  • IPLD DID is using ed25519 keys in IPNS records.
  • Anyone using secp256k1 would be affected by reverting this change.

Therefore, in the interest of causing the least breakage possible, I'm going to:

  1. Work with OpenBazaar to mitigate the impact of this issue. Luckily, OB is using a forked network (over TOR) so migrating is not a burning issue for them, as far as I know.
  2. Close this issue.

Resolution: Keep the current peer ID calculation logic.

marten-seemann pushed a commit to libp2p/go-libp2p that referenced this issue Aug 17, 2022
What:

1. Supports decoding CIDs (of libp2p keys) as peer IDs 
   (libp2p/specs#216)
2. Continues to encode peer IDs as base58 multihashes by default.
3. Adds functions for converting between peer IDs and CIDs.
4. Deprecates IDB58{Decode,Encode}, replacing them with {Decode,Encode}.

Why:

1. We _need_ to support multibase somehow, so we can put peer IDs in domains.
2. This makes peer IDs fully self describing. That is, the CID itself indicates that it's a hash of a libp2p public key.
3. It's much easier to upgrade wire protocols than text. This change punts
pids-as-cids on the wire down the road but that's something we can revisit if it ever becomes relevant. 

See libp2p/specs#111 for context.

Deviations from that issue:

* This _retains_ the current peer ID inlining of ed25519 keys. That turned out to be a nightmare, one I'd like to punt a bit more down the road.
* Likewise, this _punts_ the question of embedding the multi-hash algorithm in the public key.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.