Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

fix: ensure dials always use the latest PeerInfo from the PeerBook #312

Merged
merged 3 commits into from
Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 5 additions & 5 deletions src/dialer/queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,11 @@ function createConnectionWithProtocol ({ protocol, connection, callback }) {
class Queue {
/**
* @constructor
* @param {PeerInfo} peerInfo
* @param {string} peerId
* @param {Switch} _switch
*/
constructor (peerInfo, _switch) {
this.peerInfo = peerInfo
jacobheun marked this conversation as resolved.
Show resolved Hide resolved
this.id = peerInfo.id.toB58String()
constructor (peerId, _switch) {
this.id = peerId
this.switch = _switch
this._queue = []
this.isRunning = false
Expand Down Expand Up @@ -168,8 +167,9 @@ class Queue {
this._run()
})

const peerInfo = this.switch._peerBook.get(this.id)
let queuedDial = this._queue.shift()
let { connectionFSM, didCreate } = this._getOrCreateConnection(this.peerInfo)
let { connectionFSM, didCreate } = this._getOrCreateConnection(peerInfo)

// If the dial expects a ConnectionFSM, we can provide that back now
if (queuedDial.useFSM) {
Expand Down
2 changes: 1 addition & 1 deletion src/dialer/queueManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class DialQueueManager {
getQueue (peerInfo) {
const id = peerInfo.id.toB58String()

this._queue[id] = this._queue[id] || new Queue(peerInfo, this.switch)
this._queue[id] = this._queue[id] || new Queue(id, this.switch)
return this._queue[id]
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/get-peer-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ const multiaddr = require('multiaddr')
function getPeerInfo (peer, peerBook) {
let peerInfo

// Already a PeerInfo instance
// Already a PeerInfo instance,
// add to the peer book and return the latest value
if (PeerInfo.isPeerInfo(peer)) {
return peer
return peerBook.put(peer)
}

// Attempt to convert from Multiaddr instance (not string)
Expand Down
29 changes: 29 additions & 0 deletions test/get-peer-info.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,35 @@ describe('Get peer info', () => {
})
})

it('should add a peerInfo to the book', (done) => {
PeerId.createFromJSON(TestPeerInfos[1].id, (err, id) => {
const peerInfo = new PeerInfo(id)
expect(peerBook.has(peerInfo.id.toB58String())).to.eql(false)

expect(getPeerInfo(peerInfo, peerBook)).to.exist()
expect(peerBook.has(peerInfo.id.toB58String())).to.eql(true)
done(err)
})
})

it('should return the most up to date version of the peer', (done) => {
const ma1 = MultiAddr('/ip4/0.0.0.0/tcp/8080')
const ma2 = MultiAddr('/ip6/::/tcp/8080')
PeerId.createFromJSON(TestPeerInfos[1].id, (err, id) => {
const peerInfo = new PeerInfo(id)
peerInfo.multiaddrs.add(ma1)
expect(getPeerInfo(peerInfo, peerBook)).to.exist()

const peerInfo2 = new PeerInfo(id)
peerInfo2.multiaddrs.add(ma2)
const returnedPeerInfo = getPeerInfo(peerInfo2, peerBook)
expect(returnedPeerInfo.multiaddrs.toArray()).to.contain.members([
ma1, ma2
])
done(err)
})
})

it('an invalid peer type should throw an error', () => {
let func = () => {
getPeerInfo('/ip4/127.0.0.1/tcp/1234', peerBook)
Expand Down