From 153fd24476b679799f4813f1139269ccc8987d3f Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Wed, 29 Jan 2020 13:34:25 +0000 Subject: [PATCH] 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 --- SPEC/BITSWAP.md | 8 ++++---- SPEC/DHT.md | 18 +++++++++--------- SPEC/MISCELLANEOUS.md | 8 ++++++++ SPEC/SWARM.md | 6 +++--- package.json | 1 + src/dht/find-peer.js | 11 ++++++----- src/miscellaneous/id.js | 8 +++++++- src/pubsub/peers.js | 6 ++++-- src/pubsub/subscribe.js | 4 +++- src/swarm/addrs.js | 9 ++++++--- src/swarm/peers.js | 4 ++-- src/utils/mocha.js | 1 + 12 files changed, 54 insertions(+), 30 deletions(-) diff --git a/SPEC/BITSWAP.md b/SPEC/BITSWAP.md index 245f3a13..dd444ef6 100644 --- 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 index f856555f..8b67144e 100644 --- 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,7 +127,7 @@ Prints objects like: { extra: 'dial backoff', - id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z), + id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z', responses: [ { addrs: [ @@ -135,7 +135,7 @@ Prints objects like: Multiaddr(/ip4/172.20.0.3/tcp/4001), Multiaddr(/ip4/35.178.190.196/tcp/1024) ], - id: CID(QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8) + id: 'QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8' } ], type: 1 @@ -181,7 +181,7 @@ Prints objects like: { extra: 'dial backoff', - id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z), + id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z', responses: [ { addrs: [ @@ -189,7 +189,7 @@ Prints objects like: Multiaddr(/ip4/172.20.0.3/tcp/4001), Multiaddr(/ip4/35.178.190.196/tcp/1024) ], - id: CID(QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8) + id: 'QmRz5Nth4jTFuJJKcjyb6uwvrhxWbruRvamKY2PJxwJKw8' } ], type: 1 @@ -235,7 +235,7 @@ Prints objects like: { extra: 'dial backoff', - id: CID(QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z), + id: 'QmWtewmnzJiQevJPSmG9s8aC7yRfK2WXTCdRc1pCbDFu6z', responses: [ { addrs: [ @@ -243,7 +243,7 @@ Prints objects like: 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 index ae1af409..ca43addd 100644 --- a/SPEC/MISCELLANEOUS.md +++ b/SPEC/MISCELLANEOUS.md @@ -19,6 +19,14 @@ | -------- | -------- | | `Promise` | 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 index b82eb258..2743d798 100644 --- a/SPEC/SWARM.md +++ b/SPEC/SWARM.md @@ -18,7 +18,7 @@ | 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:** @@ -26,7 +26,7 @@ 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 index b0d41dbd..7b05045e 100644 --- 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 index 8e6c69e5..a742f45d 100644 --- 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 index 3d25a045..de84289e 100644 --- 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 index 22635fed..ff56ba3d 100644 --- 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 index 0df59523..17354377 100644 --- 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 index 86032250..994a4d0f 100644 --- 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 index 90a12e9d..404b9e4a 100644 --- 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 index 2c5c1b95..d7656312 100644 --- 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