chore: address review

libp2p · Sep 25, 2020 · 8f40eb6 · 8f40eb6
1 parent 173367b
commit 8f40eb6
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 61 deletions.
diff --git a/src/record/utils.js b/src/record/utils.js
@@ -0,0 +1,20 @@
+'use strict'
+
+const Envelope = require('./envelope')
+const PeerRecord = require('./peer-record')
+
+/**
+ * Create (or update if existing) self peer record and store it in the AddressBook.
+ * @param {libp2p} libp2p
+ * @returns {Promise<void>}
+ */
+async function updateSelfPeerRecord (libp2p) {
+  const peerRecord = new PeerRecord({
+    peerId: libp2p.peerId,
+    multiaddrs: libp2p.multiaddrs
+  })
+  const envelope = await Envelope.seal(peerRecord, libp2p.peerId)
+  libp2p.peerStore.addressBook.consumePeerRecord(envelope)
+}
+
+module.exports.updateSelfPeerRecord = updateSelfPeerRecord
diff --git a/src/transport-manager.js b/src/transport-manager.js
@@ -7,8 +7,7 @@ const debug = require('debug')
 const log = debug('libp2p:transports')
 log.error = debug('libp2p:transports:error')
 
-const Envelope = require('./record/envelope')
-const PeerRecord = require('./record/peer-record')
+const { updateSelfPeerRecord } = require('./record/utils')
 
 class TransportManager {
   /**
@@ -156,8 +155,8 @@ class TransportManager {
         this._listeners.get(key).push(listener)
 
         // Track listen/close events
-        listener.on('listening', () => this._createSelfPeerRecord())
-        listener.on('close', () => this._createSelfPeerRecord())
+        listener.on('listening', () => updateSelfPeerRecord(this.libp2p))
+        listener.on('close', () => updateSelfPeerRecord(this.libp2p))
 
         // We need to attempt to listen on everything
         tasks.push(listener.listen(addr))
@@ -226,26 +225,6 @@ class TransportManager {
 
     await Promise.all(tasks)
   }
-
-  /**
-   * Create self signed peer record raw envelope.
-   * @return {Uint8Array}
-   */
-  async _createSelfPeerRecord () {
-    try {
-      const peerRecord = new PeerRecord({
-        peerId: this.libp2p.peerId,
-        multiaddrs: this.libp2p.multiaddrs
-      })
-      const envelope = await Envelope.seal(peerRecord, this.libp2p.peerId)
-      this.libp2p.peerStore.addressBook.consumePeerRecord(envelope)
-
-      return this.libp2p.peerStore.addressBook.getRawEnvelope(this.libp2p.peerId)
-    } catch (err) {
-      log.error('failed to get self peer record')
-    }
-    return null
-  }
 }
 
 /**

diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js
@@ -40,21 +40,28 @@ describe('Dialing (direct, TCP)', () => {
   let peerStore
   let remoteAddr
 
-  before(async () => {
-    const [remotePeerId] = await Promise.all([
-      PeerId.createFromJSON(Peers[0])
+  beforeEach(async () => {
+    const [localPeerId, remotePeerId] = await Promise.all([
+      PeerId.createFromJSON(Peers[0]),
+      PeerId.createFromJSON(Peers[1])
     ])
+
+    peerStore = new PeerStore({ peerId: remotePeerId })
     remoteTM = new TransportManager({
       libp2p: {
-        addressManager: new AddressManager({ listen: [listenAddr] })
+        addressManager: new AddressManager({ listen: [listenAddr] }),
+        peerId: remotePeerId,
+        peerStore
       },
       upgrader: mockUpgrader
     })
     remoteTM.add(Transport.prototype[Symbol.toStringTag], Transport)
 
-    peerStore = new PeerStore({ peerId: remotePeerId })
     localTM = new TransportManager({
-      libp2p: {},
+      libp2p: {
+        peerId: localPeerId,
+        peerStore: new PeerStore({ peerId: localPeerId })
+      },
       upgrader: mockUpgrader
     })
     localTM.add(Transport.prototype[Symbol.toStringTag], Transport)
@@ -64,7 +71,7 @@ describe('Dialing (direct, TCP)', () => {
     remoteAddr = remoteTM.getAddrs()[0].encapsulate(`/p2p/${remotePeerId.toB58String()}`)
   })
 
-  after(() => remoteTM.close())
+  afterEach(() => remoteTM.close())
 
   afterEach(() => {
     sinon.restore()
@@ -110,7 +117,7 @@ describe('Dialing (direct, TCP)', () => {
       peerStore
     })
 
-    peerStore.addressBook.set(peerId, [remoteAddr])
+    peerStore.addressBook.set(peerId, remoteTM.getAddrs())
 
     const connection = await dialer.connectToPeer(peerId)
     expect(connection).to.exist()

diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js
@@ -8,7 +8,6 @@ const { expect } = chai
 const sinon = require('sinon')
 
 const { EventEmitter } = require('events')
-const delay = require('delay')
 const PeerId = require('peer-id')
 const duplexPair = require('it-pair/duplex')
 const multiaddr = require('multiaddr')
@@ -20,9 +19,9 @@ const { IdentifyService, multicodecs } = require('../../src/identify')
 const Peers = require('../fixtures/peers')
 const Libp2p = require('../../src')
 const Envelope = require('../../src/record/envelope')
-const PeerRecord = require('../../src/record/peer-record')
 const PeerStore = require('../../src/peer-store')
 const baseOptions = require('../utils/base-options.browser')
+const { updateSelfPeerRecord } = require('../../src/record/utils')
 const pkg = require('../../package.json')
 
 const { MULTIADDRS_WEBSOCKETS } = require('../fixtures/browser')
@@ -80,7 +79,7 @@ describe('Identify', () => {
     sinon.spy(localIdentify.peerStore.protoBook, 'set')
 
     // Transport Manager creates signed peer record
-    await _createSelfPeerRecord(remoteIdentify._libp2p)
+    await updateSelfPeerRecord(remoteIdentify._libp2p)
 
     // Run identify
     await Promise.all([
@@ -244,8 +243,8 @@ describe('Identify', () => {
       sinon.spy(remoteIdentify.peerStore.protoBook, 'set')
 
       // Transport Manager creates signed peer record
-      await _createSelfPeerRecord(localIdentify._libp2p)
-      await _createSelfPeerRecord(remoteIdentify._libp2p)
+      await updateSelfPeerRecord(localIdentify._libp2p)
+      await updateSelfPeerRecord(remoteIdentify._libp2p)
 
       // Run identify
       await Promise.all([
@@ -389,8 +388,6 @@ describe('Identify', () => {
 
       const connection = await libp2p.dialer.connectToPeer(remoteAddr)
       expect(connection).to.exist()
-      // Wait for nextTick to trigger the identify call
-      await delay(1)
 
       // Wait for identify to finish
       await libp2p.identifyService.identify.firstCall.returnValue
@@ -426,8 +423,6 @@ describe('Identify', () => {
 
       const connection = await libp2p.dialer.connectToPeer(remoteAddr)
       expect(connection).to.exist()
-      // Wait for nextTick to trigger the identify call
-      await delay(1)
 
       // Wait for identify to finish
       await libp2p.identifyService.identify.firstCall.returnValue
@@ -450,15 +445,3 @@ describe('Identify', () => {
     })
   })
 })
-
-// Self peer record creating on Transport Manager simulation
-const _createSelfPeerRecord = async (libp2p) => {
-  try {
-    const peerRecord = new PeerRecord({
-      peerId: libp2p.peerId,
-      multiaddrs: libp2p.multiaddrs
-    })
-    const envelope = await Envelope.seal(peerRecord, libp2p.peerId)
-    libp2p.peerStore.addressBook.consumePeerRecord(envelope)
-  } catch (_) {}
-}
diff --git a/test/transports/transport-manager.node.js b/test/transports/transport-manager.node.js
@@ -4,11 +4,11 @@
 const chai = require('chai')
 chai.use(require('dirty-chai'))
 const { expect } = chai
-const sinon = require('sinon')
 
 const AddressManager = require('../../src/address-manager')
 const TransportManager = require('../../src/transport-manager')
 const PeerStore = require('../../src/peer-store')
+const PeerRecord = require('../../src/record/peer-record')
 const Transport = require('libp2p-tcp')
 const PeerId = require('peer-id')
 const multiaddr = require('multiaddr')
@@ -27,11 +27,13 @@ describe('Transport Manager (TCP)', () => {
     localPeer = await PeerId.createFromJSON(Peers[0])
   })
 
-  before(() => {
+  beforeEach(() => {
     tm = new TransportManager({
       libp2p: {
+        peerId: localPeer,
+        multiaddrs: addrs,
         addressManager: new AddressManager({ listen: addrs }),
-        PeerStore: new PeerStore({ peerId: localPeer })
+        peerStore: new PeerStore({ peerId: localPeer })
       },
       upgrader: mockUpgrader,
       onConnection: () => {}
@@ -50,22 +52,36 @@ describe('Transport Manager (TCP)', () => {
   })
 
   it('should be able to listen', async () => {
-    sinon.spy(tm, '_createSelfPeerRecord')
-
     tm.add(Transport.prototype[Symbol.toStringTag], Transport)
     await tm.listen(addrs)
     expect(tm._listeners).to.have.key(Transport.prototype[Symbol.toStringTag])
     expect(tm._listeners.get(Transport.prototype[Symbol.toStringTag])).to.have.length(addrs.length)
 
-    // Created Self Peer record on new listen address
-    expect(tm._createSelfPeerRecord.callCount).to.equal(addrs.length)
-
     // Ephemeral ip addresses may result in multiple listeners
     expect(tm.getAddrs().length).to.equal(addrs.length)
     await tm.close()
     expect(tm._listeners.get(Transport.prototype[Symbol.toStringTag])).to.have.length(0)
   })
 
+  it('should create self signed peer record on listen', async () => {
+    let signedPeerRecord = await tm.libp2p.peerStore.addressBook.getPeerRecord(localPeer)
+    expect(signedPeerRecord).to.not.exist()
+
+    tm.add(Transport.prototype[Symbol.toStringTag], Transport)
+    await tm.listen(addrs)
+
+    // Should created Self Peer record on new listen address
+    signedPeerRecord = await tm.libp2p.peerStore.addressBook.getPeerRecord(localPeer)
+    expect(signedPeerRecord).to.exist()
+
+    const record = PeerRecord.createFromProtobuf(signedPeerRecord.payload)
+    expect(record).to.exist()
+    expect(record.multiaddrs.length).to.equal(addrs.length)
+    addrs.forEach((a, i) => {
+      expect(record.multiaddrs[i].equals(a)).to.be.true()
+    })
+  })
+
   it('should be able to dial', async () => {
     tm.add(Transport.prototype[Symbol.toStringTag], Transport)
     await tm.listen(addrs)