From 5e839efcd04e0b9b51bac651ecc0c7cda997c826 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 24 Jan 2020 12:48:45 +0000 Subject: [PATCH 1/5] refactor: return peer ids as strings 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`. --- SPEC/MISCELLANEOUS.md | 8 ++++++++ SPEC/SWARM.md | 2 +- package.json | 1 + src/miscellaneous/id.js | 16 +++++++++++++++- src/swarm/peers.js | 4 +--- src/utils/mocha.js | 1 + 6 files changed, 27 insertions(+), 5 deletions(-) diff --git a/SPEC/MISCELLANEOUS.md b/SPEC/MISCELLANEOUS.md index ae1af409..548ccb61 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: String[]` - A list of multiaddrs this node is listening on as strings +- `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..c8011eb8 100644 --- a/SPEC/SWARM.md +++ b/SPEC/SWARM.md @@ -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/miscellaneous/id.js b/src/miscellaneous/id.js index 3d25a045..4db36f02 100644 --- a/src/miscellaneous/id.js +++ b/src/miscellaneous/id.js @@ -2,6 +2,7 @@ 'use strict' const { getDescribe, getIt, expect } = require('../utils/mocha') +const Multiaddr = require('multiaddr') /** @typedef { import("ipfsd-ctl/src/factory") } Factory */ /** @@ -24,8 +25,21 @@ 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(res).to.have.a.property('publicKey') + expect(res).to.have.a.property('addresses').that.is.an('array').and.all.satisfy(ma => { + const isString = (ma instanceof String || typeof ma === 'string') + const asString = new Multiaddr(ma).toString() + + // TODO: remove when go-IPFS returns nu-school /p2p/ multiaddrs + if (ma.includes('/ipfs/')) { + ma = ma.replace(/ipfs/g, 'p2p') + } + + return isString && asString === 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/swarm/peers.js b/src/swarm/peers.js index 90a12e9d..b69bdcab 100644 --- a/src/swarm/peers.js +++ b/src/swarm/peers.js @@ -2,7 +2,6 @@ 'use strict' const multiaddr = require('multiaddr') -const CID = require('cids') const delay = require('delay') const { isNode } = require('ipfs-utils/src/env') const { getDescribe, getIt, expect } = require('../utils/mocha') @@ -41,8 +40,7 @@ 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(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 From 176094909dfb70c1964b26af28c220c598267194 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 27 Jan 2020 11:01:55 +0000 Subject: [PATCH 2/5] fix: assert returned peer id strings can be cids --- src/miscellaneous/id.js | 2 ++ src/swarm/peers.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/miscellaneous/id.js b/src/miscellaneous/id.js index 4db36f02..31b165fc 100644 --- a/src/miscellaneous/id.js +++ b/src/miscellaneous/id.js @@ -3,6 +3,7 @@ const { getDescribe, getIt, expect } = require('../utils/mocha') const Multiaddr = require('multiaddr') +const CID = require('cids') /** @typedef { import("ipfsd-ctl/src/factory") } Factory */ /** @@ -26,6 +27,7 @@ module.exports = (common, options) => { it('should get the node ID', async () => { const res = await ipfs.id() expect(res).to.have.a.property('id').that.is.a('string') + expect(CID.isCID(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 => { const isString = (ma instanceof String || typeof ma === 'string') diff --git a/src/swarm/peers.js b/src/swarm/peers.js index b69bdcab..423e44f4 100644 --- a/src/swarm/peers.js +++ b/src/swarm/peers.js @@ -2,6 +2,7 @@ 'use strict' const multiaddr = require('multiaddr') +const CID = require('cids') const delay = require('delay') const { isNode } = require('ipfs-utils/src/env') const { getDescribe, getIt, expect } = require('../utils/mocha') @@ -41,6 +42,7 @@ 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').that.is.a('string') + expect(CID.isCID(peer.peer)).to.equal(true) expect(peer).to.not.have.a.property('latency') /* TODO: These assertions must be uncommented as soon as From 9147ee67ee28dfd216c2158895faae32dc784406 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 27 Jan 2020 14:44:25 +0000 Subject: [PATCH 3/5] fix: update tests to expect string peer ids and multiaddr multiaddrs --- src/dht/find-peer.js | 11 ++++++----- src/miscellaneous/id.js | 14 ++------------ src/pubsub/peers.js | 6 ++++-- src/pubsub/subscribe.js | 4 +++- src/swarm/addrs.js | 9 ++++++--- src/swarm/peers.js | 2 +- 6 files changed, 22 insertions(+), 24 deletions(-) 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 31b165fc..de84289e 100644 --- a/src/miscellaneous/id.js +++ b/src/miscellaneous/id.js @@ -27,19 +27,9 @@ module.exports = (common, options) => { it('should get the node ID', async () => { const res = await ipfs.id() expect(res).to.have.a.property('id').that.is.a('string') - expect(CID.isCID(res.id)).to.equal(true) + 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 => { - const isString = (ma instanceof String || typeof ma === 'string') - const asString = new Multiaddr(ma).toString() - - // TODO: remove when go-IPFS returns nu-school /p2p/ multiaddrs - if (ma.includes('/ipfs/')) { - ma = ma.replace(/ipfs/g, 'p2p') - } - - return isString && asString === ma - }) + 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 423e44f4..404b9e4a 100644 --- a/src/swarm/peers.js +++ b/src/swarm/peers.js @@ -42,7 +42,7 @@ 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').that.is.a('string') - expect(CID.isCID(peer.peer)).to.equal(true) + 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 From 5dd360b996124448386d57354b56c7b8a92c6ea1 Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Wed, 29 Jan 2020 11:52:42 +0000 Subject: [PATCH 4/5] Update SPEC/MISCELLANEOUS.md Co-Authored-By: Alan Shaw --- SPEC/MISCELLANEOUS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SPEC/MISCELLANEOUS.md b/SPEC/MISCELLANEOUS.md index 548ccb61..ca43addd 100644 --- a/SPEC/MISCELLANEOUS.md +++ b/SPEC/MISCELLANEOUS.md @@ -23,7 +23,7 @@ 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: String[]` - A list of multiaddrs this node is listening on as strings +- `addresses: Multiaddr[]` - A list of multiaddrs this node is listening on - `agentVersion: String` - The agent version - `protocolVersion: String` - The supported protocol version From 9a00b4fe730057dfb85aad8baaea29d30e7ed303 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Wed, 29 Jan 2020 13:20:44 +0000 Subject: [PATCH 5/5] docs: update return types --- SPEC/BITSWAP.md | 8 ++++---- SPEC/DHT.md | 18 +++++++++--------- SPEC/SWARM.md | 4 ++-- 3 files changed, 15 insertions(+), 15 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/SWARM.md b/SPEC/SWARM.md index c8011eb8..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 */