This repository has been archived by the owner on Mar 10, 2020. It is now read-only.
refactor: return peer ids as strings #581
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As part of the async iterator refactor some peer IDs were being returned as CIDs - the conversion was done in IPFS but not in libp2p, which meant that where IPFS exposes libp2p directly, peer IDs were being returned as strings, e.g. pubsub topic subscribers, message senders, etc. The aim is to convert these to CIDs in the long run but such a change needs to be driven from inside libp2p instead of piecemeal at the IPFS layer. This PR changes the `ipfs.id().id` and `ipfs.swarm.peers().[].peer` properties to strings so we can do the CID conversion in one go. Also adds more in-depth tests for `ipfs.id`.
achingbrain
added a commit
to ipfs-inactive/js-ipfs-http-client
that referenced
this pull request
Jan 24, 2020
This was referenced Jan 24, 2020
alanshaw
suggested changes
Jan 27, 2020
|
Is ready for re-review? |
|
Yes, should be good to go. |
alanshaw
suggested changes
Jan 29, 2020
| @@ -27,14 +27,15 @@ module.exports = (common, options) => { | |||
| after(() => common.clean()) | |||
|
|
|||
| it('should find other peers', async () => { | |||
| const res = await nodeA.dht.findPeer(nodeB.peerId.id) | |||
|
|
|||
| const nodeBId = await nodeB.id() | |||
There was a problem hiding this comment.
Why not use nodeB.peerId here?
There was a problem hiding this comment.
It's not part of the API defined in interface-js-ipfs-core so we should really remove it from ipfsd-ctl return values.
This is my mind wandering from the task in hand though so I'm happy to change it if required.
There was a problem hiding this comment.
Right, so it shouldn't be used in the interface tests because we can't guarantee ipfsd-ctl spawned the node(s) in the test. 👍 Sounds like a good plan, I'm happy to leave this like it is. Do you want to open an issue here to track that task?
There was a problem hiding this comment.
Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai>
alanshaw
pushed a commit
to ipfs/js-ipfs
that referenced
this pull request
Feb 3, 2020
Also updates all examples to use the new API. Depends on: - [x] ipfs-inactive/interface-js-ipfs-core#581 - [x] ipfs-inactive/js-ipfs-http-client#1226 - [x] libp2p/js-libp2p#545 BREAKING CHANGE: Where `PeerID`s were previously [CID](https://www.npmjs.com/package/cids)s, now they are Strings - `ipfs.bitswap.stat().peers[n]` is now a String (was a CID) - `ipfs.dht.findPeer().id` is now a String (was a CID) - `ipfs.dht.findProvs()[n].id` is now a String (was a CID) - `ipfs.dht.provide()[n].id` is now a String (was a CID) - `ipfs.dht.put()[n].id` is now a String (was a CID) - `ipfs.dht.query()[n].id` is now a String (was a CID) - `ipfs.id().id` is now a String (was a CID) - `ipfs.id().addresses[n]` are now [Multiaddr](https://www.npmjs.com/package/multiaddr)s (were Strings)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the async iterator refactor some peer IDs were being returned as CIDs - the conversion was done in IPFS but not in libp2p, which meant that where IPFS exposes libp2p directly, peer IDs were being returned as strings, e.g. pubsub topic subscribers, message senders, etc.
The aim is to convert these to CIDs in the long run but such a change needs to be driven from inside libp2p instead of piecemeal at the IPFS layer.
This PR changes the
ipfs.id().idandipfs.swarm.peers().[].peerproperties to strings so we can do the CID conversion in one go.Also adds more in-depth tests for
ipfs.idand documents the output from that method.BREAKING CHANGE:
Where
PeerIDs were previously CIDs, now they are Stringsipfs.bitswap.stat().peers[n]is now a String (was a CID)ipfs.dht.findPeer().idis now a String (was a CID)ipfs.dht.findProvs()[n].idis now a String (was a CID)ipfs.dht.provide()[n].idis now a String (was a CID)ipfs.dht.put()[n].idis now a String (was a CID)ipfs.dht.query()[n].idis now a String (was a CID)ipfs.id().idis now a String (was a CID)ipfs.id().addresses[n]are now Multiaddrs (were Strings)