refactor: return peer ids as strings (#581)

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` and documents the output from that method. BREAKING CHANGE: Where `PeerID`s were previously [CID]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]s (were Strings) [CID]: https://www.npmjs.com/package/cids [Multiaddr]: https://www.npmjs.com/package/multiaddr
ipfs-inactive · Jan 29, 2020 · 153fd24 · 153fd24
1 parent ced5ea1
commit 153fd24
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 30 deletions.
diff --git a/SPEC/BITSWAP.md b/SPEC/BITSWAP.md
@@ -47,7 +47,7 @@ The returned object contains the following keys:
 
 - `provideBufLen` is an integer.
 - `wantlist` (array of [CID][cid]s)
-- `peers` (array of peer IDs as [CID][cid] instances)
+- `peers` (array of peer IDs as Strings)
 - `blocksReceived` is a [BigNumber Int][1]
 - `dataReceived` is a [BigNumber Int][1]
 - `blocksSent` is a [BigNumber Int][1]
@@ -64,9 +64,9 @@ console.log(stats)
 //   provideBufLen: 0,
 //   wantlist: [ CID('QmSoLPppuBtQSGwKDZT2M73ULpjvfd3aZ6ha4oFGL1KrGM') ],
 //   peers:
-//    [ CID('QmSoLPppuBtQSGwKDZT2M73ULpjvfd3aZ6ha4oFGL1KrGM'),
-//      CID('QmSoLSafTMBsPKadTEgaXctDQVcqN88CNLHXMkTNwMKPnu'),
-//      CID('QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd') ],
+//    [ 'QmSoLPppuBtQSGwKDZT2M73ULpjvfd3aZ6ha4oFGL1KrGM',
+//      'QmSoLSafTMBsPKadTEgaXctDQVcqN88CNLHXMkTNwMKPnu',
+//      'QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd' ],
 //   blocksReceived: 0,
 //   dataReceived: 0,
 //   blocksSent: 0,

diff --git a/SPEC/DHT.md b/SPEC/DHT.md
@@ -19,14 +19,14 @@ Where `peerId` is a Peer ID in `String`, [`CID`](https://github.com/multiformats
 
 | Type | Description |
 | -------- | -------- |
-| `Promise<{ id: CID, addrs: Multiaddr[] }>` | A promise that resolves to an object with `id` and `addrs`. `id` is a [`CID`](https://github.com/multiformats/js-cid) - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
+| `Promise<{ id: String, addrs: Multiaddr[] }>` | A promise that resolves to an object with `id` and `addrs`. `id` is a String - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
 
 **Example:**
 
 ```JavaScript
 const info = await ipfs.dht.findPeer('QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt')
 
-console.log(info.id.toString())
+console.log(info.id)
 /*
 QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt
 */
@@ -60,7 +60,7 @@ Note that if `options.numProviders` are not found an error will be thrown.
 
 | Type | Description |
 | -------- | -------- |
-| `AsyncIterable<{ id: CID, addrs: Multiaddr[] }>` | A async iterable that yields objects with `id` and `addrs`. `id` is a [`CID`](https://github.com/multiformats/js-cid) - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
+| `AsyncIterable<{ id: String, addrs: Multiaddr[] }>` | A async iterable that yields objects with `id` and `addrs`. `id` is a String - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
 
 **Example:**
 
@@ -127,15 +127,15 @@ Prints objects like:
 
 {
   extra: 'dial backoff',
-  id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z),
+  id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z',
   responses: [
     {
       addrs: [
         Multiaddr(/ip4/127.0.0.1/tcp/4001),
         Multiaddr(/ip4/172.20.0.3/tcp/4001),
         Multiaddr(/ip4/35.178.190.196/tcp/1024)
       ],
-      id: CID(QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8)
+      id: 'QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8'
     }
   ],
   type: 1
@@ -181,15 +181,15 @@ Prints objects like:
 
 {
   extra: 'dial backoff',
-  id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z),
+  id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z',
   responses: [
     {
       addrs: [
         Multiaddr(/ip4/127.0.0.1/tcp/4001),
         Multiaddr(/ip4/172.20.0.3/tcp/4001),
         Multiaddr(/ip4/35.178.190.196/tcp/1024)
       ],
-      id: CID(QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8)
+      id: 'QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8'
     }
   ],
   type: 1
@@ -235,15 +235,15 @@ Prints objects like:
 
 {
   extra: 'dial backoff',
-  id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z),
+  id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z',
   responses: [
     {
       addrs: [
         Multiaddr(/ip4/127.0.0.1/tcp/4001),
         Multiaddr(/ip4/172.20.0.3/tcp/4001),
         Multiaddr(/ip4/35.178.190.196/tcp/1024)
       ],
-      id: CID(QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8)
+      id: 'QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8'
     }
   ],
   type: 1

diff --git a/SPEC/MISCELLANEOUS.md b/SPEC/MISCELLANEOUS.md
@@ -19,6 +19,14 @@
 | -------- | -------- |
 | `Promise<Object>` | An object with the Peer identity |
 
+The Peer identity has the following properties:
+
+- `id: String` - the Peer ID
+- `publicKey: String` - the public key of the peer as a base64 encoded string
+- `addresses: Multiaddr[]` - A list of multiaddrs this node is listening on
+- `agentVersion: String` - The agent version
+- `protocolVersion: String` - The supported protocol version
+
 **Example:**
 
 ```JavaScript

diff --git a/SPEC/SWARM.md b/SPEC/SWARM.md
@@ -18,15 +18,15 @@
 
 | Type | Description |
 | -------- | -------- |
-| `Promise<{ id: CID, addrs: Multiaddr[] }>` | A promise that resolves to an object with `id` and `addrs`. `id` is a [`CID`](https://github.com/multiformats/js-cid) - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
+| `Promise<{ id: String, addrs: Multiaddr[] }>` | A promise that resolves to an object with `id` and `addrs`. `id` is a String - the peer's ID and `addrs` is an array of [Multiaddr](https://github.com/multiformats/js-multiaddr/) - addresses for the peer. |
 
 **Example:**
 
 ```JavaScript
 const peerInfos = await ipfs.swarm.addrs()
 
 peerInfos.forEach(info => {
-  console.log(info.id.toString())
+  console.log(info.id)
   /*
   QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt
   */
@@ -131,7 +131,7 @@ A great source of [examples][] can be found in the tests for this API.
 The returned array has the following form:
 
 - `addr: Multiaddr`
-- `peer: CID`
+- `peer: String`
 - `latency: String` - Only if `verbose: true`  was passed
 - `muxer: String` - The type of stream muxer the peer is usng
 - `streams: string[]` - Only if `verbose: true`, a list of currently open streams

diff --git a/package.json b/package.json
@@ -40,6 +40,7 @@
   "dependencies": {
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
+    "chai-things": "^0.2.0",
     "cids": "~0.7.1",
     "delay": "^4.3.0",
     "dirty-chai": "^2.0.1",

diff --git a/src/dht/find-peer.js b/src/dht/find-peer.js
@@ -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()
+      const res = await nodeA.dht.findPeer(nodeBId.id)
       const id = res.id.toString()
-      const nodeAddresses = nodeB.peerId.addresses.map((addr) => addr.split('/ipfs/')[0]) // remove '/ipfs/'
-      const peerAddresses = res.addrs.map(ma => ma.toString().split('/ipfs/')[0])
+
+      const nodeAddresses = nodeBId.addresses.map((addr) => addr.nodeAddress())
+      const peerAddresses = res.addrs.map(ma => ma.nodeAddress())
 
       expect(id).to.be.eql(nodeB.peerId.id)
-      expect(nodeAddresses).to.include(peerAddresses[0])
+      expect(peerAddresses).to.deep.include(nodeAddresses[0])
     })
 
     it('should fail to find other peer if peer does not exist', () => {

diff --git a/src/miscellaneous/id.js b/src/miscellaneous/id.js
@@ -2,6 +2,8 @@
 'use strict'
 
 const { getDescribe, getIt, expect } = require('../utils/mocha')
+const Multiaddr = require('multiaddr')
+const CID = require('cids')
 
 /** @typedef { import("ipfsd-ctl/src/factory") } Factory */
 /**
@@ -24,8 +26,12 @@ module.exports = (common, options) => {
 
     it('should get the node ID', async () => {
       const res = await ipfs.id()
-      expect(res).to.have.a.property('id')
+      expect(res).to.have.a.property('id').that.is.a('string')
+      expect(CID.isCID(new CID(res.id))).to.equal(true)
       expect(res).to.have.a.property('publicKey')
+      expect(res).to.have.a.property('addresses').that.is.an('array').and.all.satisfy(ma => Multiaddr.isMultiaddr(ma))
+      expect(res).to.have.a.property('agentVersion').that.is.a('string')
+      expect(res).to.have.a.property('protocolVersion').that.is.a('string')
     })
   })
 }
diff --git a/src/pubsub/peers.js b/src/pubsub/peers.js
@@ -26,8 +26,10 @@ module.exports = (common, options) => {
       ipfs2 = (await common.spawn({ type: 'go' })).api
       ipfs3 = (await common.spawn({ type: 'go' })).api
 
-      const ipfs2Addr = ipfs2.peerId.addresses.find((a) => a.includes('127.0.0.1'))
-      const ipfs3Addr = ipfs3.peerId.addresses.find((a) => a.includes('127.0.0.1'))
+      const ipfs2Addr = ipfs2.peerId.addresses
+        .find(ma => ma.nodeAddress().address === '127.0.0.1')
+      const ipfs3Addr = ipfs3.peerId.addresses
+        .find(ma => ma.nodeAddress().address === '127.0.0.1')
 
       await ipfs1.swarm.connect(ipfs2Addr)
       await ipfs1.swarm.connect(ipfs3Addr)

diff --git a/src/pubsub/subscribe.js b/src/pubsub/subscribe.js
@@ -153,7 +153,9 @@ module.exports = (common, options) => {
           ipfs2.pubsub.setMaxListeners(100)
         }
 
-        const ipfs2Addr = ipfs2.peerId.addresses.find((a) => a.includes('127.0.0.1'))
+        const ipfs2Addr = ipfs2.peerId.addresses
+          .find(ma => ma.nodeAddress().address === '127.0.0.1')
+
         return ipfs1.swarm.connect(ipfs2Addr)
       })
 

diff --git a/src/swarm/addrs.js b/src/swarm/addrs.js
@@ -32,9 +32,12 @@ module.exports = (common, options) => {
       const peerInfos = await ipfsA.swarm.addrs()
       expect(peerInfos).to.not.be.empty()
       expect(peerInfos).to.be.an('array')
-      peerInfos.forEach(m => {
-        expect(CID.isCID(m.id)).to.be.true()
-        m.addrs.forEach(addr => expect(Multiaddr.isMultiaddr(addr)).to.be.true())
+
+      expect(peerInfos).to.all.satisfy(peerInfo => {
+        expect(CID.isCID(new CID(peerInfo.id))).to.be.true()
+        expect(peerInfo).to.have.a.property('addrs').that.is.an('array').and.all.satisfy(ma => Multiaddr.isMultiaddr(ma))
+
+        return true
       })
     })
   })

diff --git a/src/swarm/peers.js b/src/swarm/peers.js
@@ -41,8 +41,8 @@ module.exports = (common, options) => {
 
       expect(peer).to.have.a.property('addr')
       expect(multiaddr.isMultiaddr(peer.addr)).to.equal(true)
-      expect(peer).to.have.a.property('peer')
-      expect(CID.isCID(peer.peer)).to.equal(true)
+      expect(peer).to.have.a.property('peer').that.is.a('string')
+      expect(CID.isCID(new CID(peer.peer))).to.equal(true)
       expect(peer).to.not.have.a.property('latency')
 
       /* TODO: These assertions must be uncommented as soon as

diff --git a/src/utils/mocha.js b/src/utils/mocha.js
@@ -6,6 +6,7 @@ const chai = require('chai')
 // Do not reorder these statements - https://github.com/chaijs/chai/issues/1298
 chai.use(require('chai-as-promised'))
 chai.use(require('dirty-chai'))
+chai.use(require('chai-things'))
 
 module.exports.expect = chai.expect