From cb597b57d7ce302a94b2f1fd7c7ebebe3923597b Mon Sep 17 00:00:00 2001
From: Vasco Santos <vasco.santos@moxy.studio>
Date: Fri, 24 Apr 2020 13:01:04 +0200
Subject: [PATCH] chore: apply suggestions from code review

Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
---
 doc/API.md                          |  2 +-
 src/dialer/index.js                 | 13 +++++++++----
 src/{get-peer-id.js => get-peer.js} | 15 +++++++--------
 src/identify/index.js               |  2 +-
 src/index.js                        | 16 +++++++++-------
 5 files changed, 27 insertions(+), 21 deletions(-)
 rename src/{get-peer-id.js => get-peer.js} (74%)

diff --git a/doc/API.md b/doc/API.md
index 2f70a1b4..7cf74185 100644
--- a/doc/API.md
+++ b/doc/API.md
@@ -411,7 +411,7 @@ Once a content router succeeds, the iteration will stop. If the DHT is enabled,
 ```js
 // Iterate over the providers found for the given cid
 for await (const provider of libp2p.contentRouting.findProviders(cid)) {
-  console.log(provider.id, provider.addrs)
+  console.log(provider.id, provider.multiaddrs)
 }
 ```
 
diff --git a/src/dialer/index.js b/src/dialer/index.js
index 92b04153..5e460ab3 100644
--- a/src/dialer/index.js
+++ b/src/dialer/index.js
@@ -9,7 +9,7 @@ const log = debug('libp2p:dialer')
 log.error = debug('libp2p:dialer:error')
 
 const { DialRequest } = require('./dial-request')
-const getPeerId = require('../get-peer-id')
+const getPeer = require('../get-peer')
 
 const { codes } = require('../errors')
 const {
@@ -106,8 +106,13 @@ class Dialer {
    * @returns {DialTarget}
    */
   _createDialTarget (peer) {
-    const peerId = getPeerId(peer, this.peerStore)
-    let addrs = this.peerStore.addressBook.getMultiaddrsForPeer(peerId)
+    const { id, multiaddrs } = getPeer(peer)
+
+    if (multiaddrs) {
+      this.peerStore.addressBook.add(id, multiaddrs)
+    }
+
+    let addrs = this.peerStore.addressBook.getMultiaddrsForPeer(id)
 
     // If received a multiaddr to dial, it should be the first to use
     // But, if we know other multiaddrs for the peer, we should try them too.
@@ -117,7 +122,7 @@ class Dialer {
     }
 
     return {
-      id: peerId.toB58String(),
+      id: id.toB58String(),
       addrs
     }
   }
diff --git a/src/get-peer-id.js b/src/get-peer.js
similarity index 74%
rename from src/get-peer-id.js
rename to src/get-peer.js
index 9eb687e8..cb6cba4c 100644
--- a/src/get-peer-id.js
+++ b/src/get-peer.js
@@ -7,13 +7,13 @@ const errCode = require('err-code')
 const { codes } = require('./errors')
 
 /**
- * Converts the given `peer` to a `PeerId` instance.
+ * Converts the given `peer` to a `Peer` object.
  * If a multiaddr is received, the addressBook is updated.
  * @param {PeerId|Multiaddr|string} peer
  * @param {PeerStore} peerStore
- * @returns {PeerId}
+ * @returns {{ id: PeerId, multiaddrs: Array<Multiaddr> }}
  */
-function getPeerId (peer, peerStore) {
+function getPeer (peer) {
   if (typeof peer === 'string') {
     peer = multiaddr(peer)
   }
@@ -31,11 +31,10 @@ function getPeerId (peer, peerStore) {
     }
   }
 
-  if (addr && peerStore) {
-    peerStore.addressBook.add(peer, [addr])
+  return {
+    id: peer,
+    multiaddrs: addr ? [addr] : undefined
   }
-
-  return peer
 }
 
-module.exports = getPeerId
+module.exports = getPeer
diff --git a/src/identify/index.js b/src/identify/index.js
index 0381ed11..ec3b0203 100644
--- a/src/identify/index.js
+++ b/src/identify/index.js
@@ -49,7 +49,7 @@ class IdentifyService {
    * @param {Registrar} options.registrar
    * @param {Map<string, handler>} options.protocols A reference to the protocols we support
    * @param {PeerId} options.peerId The peer running the identify service
-   * @param {{ listen: Array<Multiaddr>}} options.addresses The peer aaddresses
+   * @param {{ listen: Array<Multiaddr>}} options.addresses The peer addresses
    */
   constructor (options) {
     /**
diff --git a/src/index.js b/src/index.js
index 5debdd56..b0e780c2 100644
--- a/src/index.js
+++ b/src/index.js
@@ -11,7 +11,7 @@ const PeerId = require('peer-id')
 const peerRouting = require('./peer-routing')
 const contentRouting = require('./content-routing')
 const pubsub = require('./pubsub')
-const getPeerId = require('./get-peer-id')
+const getPeer = require('./get-peer')
 const { validate: validateConfig } = require('./config')
 const { codes } = require('./errors')
 
@@ -293,11 +293,13 @@ class Libp2p extends EventEmitter {
    * @returns {Promise<Connection|*>}
    */
   async dialProtocol (peer, protocols, options) {
-    const peerId = getPeerId(peer, this.peerStore)
-    let connection = this.registrar.getConnection(peerId)
+    const { id, multiaddrs } = getPeer(peer, this.peerStore)
+    let connection = this.registrar.getConnection(id)
 
     if (!connection) {
       connection = await this.dialer.connectToPeer(peer, options)
+    } else {
+      this.peerStore.addressBook.add(id, multiaddrs)
     }
 
     // If a protocol was provided, create a new stream
@@ -314,9 +316,9 @@ class Libp2p extends EventEmitter {
    * @returns {Promise<void>}
    */
   async hangUp (peer) {
-    const peerId = getPeerId(peer)
+    const { id } = getPeer(peer)
 
-    const connections = this.registrar.connections.get(peerId.toB58String())
+    const connections = this.registrar.connections.get(id.toB58String())
 
     if (!connections) {
       return
@@ -335,9 +337,9 @@ class Libp2p extends EventEmitter {
    * @returns {Promise<number>}
    */
   ping (peer) {
-    const peerId = getPeerId(peer)
+    const { id } = getPeer(peer)
 
-    return ping(this, peerId)
+    return ping(this, id)
   }
 
   /**