From 16f2bc37d2544896778edd1bf3d1d4b940873368 Mon Sep 17 00:00:00 2001
From: Jacob Heun <jacobheun@gmail.com>
Date: Thu, 21 Mar 2019 13:11:36 +0100
Subject: [PATCH] fix: ensure dials always use the latest PeerInfo from the
 PeerBook (#312)

* fix: ensure dials always use the latest PeerInfo from the PeerBook

This fixes an issue where if dial is called with a new instance
of PeerInfo, if it is the first dial to that peer, the queue was
forever associated with that instance. This is currently the case
when Circuit checks the HOP status of a potential relay. This ensures
that whenever we dial, we are updating the peer book and using the
latest PeerInfo in that dial request.

* test: add test for get peer info

* refactor: just use id with dialer queue
---
 src/dialer/queue.js        | 10 +++++-----
 src/dialer/queueManager.js |  2 +-
 src/get-peer-info.js       |  5 +++--
 test/get-peer-info.spec.js | 29 +++++++++++++++++++++++++++++
 4 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/dialer/queue.js b/src/dialer/queue.js
index b1c6cf1..8155fca 100644
--- a/src/dialer/queue.js
+++ b/src/dialer/queue.js
@@ -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
-    this.id = peerInfo.id.toB58String()
+  constructor (peerId, _switch) {
+    this.id = peerId
     this.switch = _switch
     this._queue = []
     this.isRunning = false
@@ -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) {
diff --git a/src/dialer/queueManager.js b/src/dialer/queueManager.js
index 775dc52..933173e 100644
--- a/src/dialer/queueManager.js
+++ b/src/dialer/queueManager.js
@@ -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]
   }
 }
diff --git a/src/get-peer-info.js b/src/get-peer-info.js
index 7e3a13f..68f38f3 100644
--- a/src/get-peer-info.js
+++ b/src/get-peer-info.js
@@ -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)
diff --git a/test/get-peer-info.spec.js b/test/get-peer-info.spec.js
index f339666..1678dad 100644
--- a/test/get-peer-info.spec.js
+++ b/test/get-peer-info.spec.js
@@ -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)