rust-libp2p icon indicating copy to clipboard operation
rust-libp2p copied to clipboard
verified publisher icon libp2p

Change representation of PeerId to CIDv1

Open JerryHue opened this issue 4 years ago • 2 comments

Addresses #2259.

To encapsulate both errors from Multihash and CID, we change the interface of from_bytes to return a PeerIdError in the case of Err.

JerryHue avatar Nov 01 '21 07:11 JerryHue

Friendly ping @JerryHue. Are you still interested in taking this fix to the finish line? :)

mxinden avatar Mar 02 '22 14:03 mxinden

Hello again, @mxinden! I would like to try to bring this to the finish line throughout this week, sorry for the excruciatingly long wait. I wonder if I should improve this PR by taking into account @tomaka's comments on the original issue. What do you think?

JerryHue avatar Mar 08 '22 02:03 JerryHue

@mxinden I believe this is waiting for your input.

thomaseizinger avatar Nov 02 '22 03:11 thomaseizinger

This PR would break compatibility with Substrate/Polkadot and is a no-no for us.

tomaka avatar Nov 02 '22 07:11 tomaka

This PR would break compatibility with Substrate/Polkadot and is a no-no for us.

Thanks for mentioning this. This definitely needs an upgrade path where we first support reading in a CIDv1 PeerId as described in the RFC.

thomaseizinger avatar Nov 02 '22 12:11 thomaseizinger

For what it's worth, I've taken the decision for Polkadot a while ago to always use CIDv0. The spec precisely mentions how to encode it: https://spec.polkadot.network/#defn-peer-id. Being able to easily encode/decode PeerIds, and ensuring uniqueness of PeerIds, are more important than conforming to some libp2p spec outside of our control and that brings zero benefits for us.

It's not a deal breaker for adding support for CIDv1 in rust-libp2p, as long as there's always an option to force CIDv0.

tomaka avatar Nov 02 '22 12:11 tomaka

@mxinden I believe this is waiting for your input.

As far as I can tell blocked on addressing comments above.

It's not a deal breaker for adding support for CIDv1 in rust-libp2p, as long as there's always an option to force CIDv0.

I suggest we support reading CIDv1 as a first iteration, though always writing and handling CIDv0s.

mxinden avatar Nov 14 '22 10:11 mxinden

This pull request has merge conflicts. Could you please resolve them @JerryHue? 🙏

mergify[bot] avatar Nov 15 '22 00:11 mergify[bot]

This pull request has merge conflicts. Could you please resolve them @JerryHue? 🙏

mergify[bot] avatar Jan 14 '23 00:01 mergify[bot]

This pull request has merge conflicts. Could you please resolve them @JerryHue? 🙏

mergify[bot] avatar Mar 16 '23 00:03 mergify[bot]