Skip to content
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
merged 5 commits into from
Jan 29, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions SPEC/MISCELLANEOUS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: String[]` - A list of multiaddrs this node is listening on as strings
achingbrain marked this conversation as resolved.
Show resolved Hide resolved
- `agentVersion: String` - The agent version
- `protocolVersion: String` - The supported protocol version

**Example:**

```JavaScript
Expand Down
2 changes: 1 addition & 1 deletion SPEC/SWARM.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
alanshaw marked this conversation as resolved.
Show resolved Hide resolved
- `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
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
11 changes: 6 additions & 5 deletions src/dht/find-peer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use nodeB.peerId here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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', () => {
Expand Down
8 changes: 7 additions & 1 deletion src/miscellaneous/id.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
/**
Expand All @@ -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')
})
})
}
6 changes: 4 additions & 2 deletions src/pubsub/peers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion src/pubsub/subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
9 changes: 6 additions & 3 deletions src/swarm/addrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
})
})
Expand Down
4 changes: 2 additions & 2 deletions src/swarm/peers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
achingbrain marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
1 change: 1 addition & 0 deletions src/utils/mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down