chore: apply suggestions from code review

libp2p · Jul 17, 2020 · 90b8761 · 90b8761
1 parent 864d153
commit 90b8761
Show file tree

Hide file tree

Showing 12 changed files with 58 additions and 57 deletions.
diff --git a/src/peer-store/address-book.js b/src/peer-store/address-book.js
@@ -88,7 +88,7 @@ class AddressBook extends Book {
     }
 
     // Verify peerId
-    if (peerRecord.peerId.toB58String() !== envelope.peerId.toB58String()) {
+    if (!peerRecord.peerId.equals(envelope.peerId)) {
       log('signing key does not match PeerId in the PeerRecord')
       return false
     }
@@ -220,10 +220,10 @@ class AddressBook extends Book {
     const id = peerId.toB58String()
 
     const entry = this.data.get(id) || {}
-    const rec = entry.addresses
+    const rec = entry.addresses || []
 
     // Add recorded uniquely to the new array (Union)
-    rec && rec.forEach((addr) => {
+    rec.forEach((addr) => {
       if (!addresses.find(r => r.multiaddr.equals(addr.multiaddr))) {
         addresses.push(addr)
       }
@@ -244,7 +244,7 @@ class AddressBook extends Book {
     log(`added provided multiaddrs for ${id}`)
 
     // Notify the existance of a new peer
-    if (!rec) {
+    if (!entry.addresses) {
       this._ps.emit('peer', peerId)
     }
 

diff --git a/src/peer-store/index.js b/src/peer-store/index.js
@@ -37,7 +37,7 @@ class PeerStore extends EventEmitter {
   /**
    * @constructor
    */
-  constructor ({ peerId } = {}) {
+  constructor ({ peerId }) {
     super()
 
     this._peerId = peerId

diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js
@@ -55,7 +55,7 @@ describe('Dialing (direct, TCP)', () => {
     })
     remoteTM.add(Transport.prototype[Symbol.toStringTag], Transport)
 
-    peerStore = new PeerStore()
+    peerStore = new PeerStore({ peerId: remotePeerId })
     localTM = new TransportManager({
       libp2p: {},
       upgrader: mockUpgrader
@@ -97,13 +97,13 @@ describe('Dialing (direct, TCP)', () => {
   })
 
   it('should be able to connect to a given peer id', async () => {
-    const peerStore = new PeerStore()
+    const peerId = await PeerId.createFromJSON(Peers[0])
+    const peerStore = new PeerStore({ peerId })
     const dialer = new Dialer({
       transportManager: localTM,
       peerStore
     })
 
-    const peerId = await PeerId.createFromJSON(Peers[0])
     peerStore.addressBook.set(peerId, [remoteAddr])
 
     const connection = await dialer.connectToPeer(peerId)

diff --git a/test/dialing/direct.spec.js b/test/dialing/direct.spec.js
@@ -13,7 +13,6 @@ const Transport = require('libp2p-websockets')
 const Muxer = require('libp2p-mplex')
 const { NOISE: Crypto } = require('libp2p-noise')
 const multiaddr = require('multiaddr')
-const PeerId = require('peer-id')
 const AggregateError = require('aggregate-error')
 const { AbortError } = require('libp2p-interfaces/src/transport/errors')
 
@@ -24,7 +23,6 @@ const PeerStore = require('../../src/peer-store')
 const TransportManager = require('../../src/transport-manager')
 const Libp2p = require('../../src')
 
-const Peers = require('../fixtures/peers')
 const { MULTIADDRS_WEBSOCKETS } = require('../fixtures/browser')
 const mockUpgrader = require('../utils/mockUpgrader')
 const createMockConnection = require('../utils/mockConnection')
@@ -35,9 +33,11 @@ const remoteAddr = MULTIADDRS_WEBSOCKETS[0]
 describe('Dialing (direct, WebSockets)', () => {
   let localTM
   let peerStore
+  let peerId
 
-  before(() => {
-    peerStore = new PeerStore()
+  before(async () => {
+    [peerId] = await createPeerId()
+    peerStore = new PeerStore({ peerId })
     localTM = new TransportManager({
       libp2p: {},
       upgrader: mockUpgrader,
@@ -132,7 +132,6 @@ describe('Dialing (direct, WebSockets)', () => {
         }
       }
     })
-    const peerId = await PeerId.createFromJSON(Peers[0])
 
     const connection = await dialer.connectToPeer(peerId)
     expect(connection).to.exist()
@@ -149,7 +148,6 @@ describe('Dialing (direct, WebSockets)', () => {
         }
       }
     })
-    const peerId = await PeerId.createFromJSON(Peers[0])
 
     await expect(dialer.connectToPeer(peerId))
       .to.eventually.be.rejectedWith(AggregateError)
@@ -198,7 +196,6 @@ describe('Dialing (direct, WebSockets)', () => {
     const deferredDial = pDefer()
     sinon.stub(localTM, 'dial').callsFake(() => deferredDial.promise)
 
-    const [peerId] = await createPeerId()
     // Perform 3 multiaddr dials
     dialer.connectToPeer(peerId)
 
@@ -245,7 +242,6 @@ describe('Dialing (direct, WebSockets)', () => {
     })
 
     // Perform 3 multiaddr dials
-    const [peerId] = await createPeerId()
     const dialPromise = dialer.connectToPeer(peerId)
 
     // Let the call stack run
@@ -266,14 +262,9 @@ describe('Dialing (direct, WebSockets)', () => {
   })
 
   describe('libp2p.dialer', () => {
-    let peerId
     let libp2p
     let remoteLibp2p
 
-    before(async () => {
-      peerId = await PeerId.createFromJSON(Peers[0])
-    })
-
     afterEach(async () => {
       sinon.restore()
       libp2p && await libp2p.stop()

diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js
@@ -50,7 +50,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -60,7 +60,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -103,7 +103,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -113,7 +113,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -156,7 +156,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: []
       },
       protocols
@@ -165,7 +165,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: []
       },
       protocols
@@ -202,7 +202,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: localPeer,
           connectionManager: new EventEmitter(),
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: localPeer }),
           multiaddrs: listenMaddrs
         },
         protocols: new Map([
@@ -215,7 +215,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: remotePeer,
           connectionManager,
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: remotePeer }),
           multiaddrs: []
         }
       })
@@ -263,7 +263,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: localPeer,
           connectionManager: new EventEmitter(),
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: localPeer }),
           multiaddrs: listenMaddrs
         },
         protocols: new Map([
@@ -276,7 +276,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: remotePeer,
           connectionManager,
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: remotePeer }),
           multiaddrs: []
         }
       })

diff --git a/test/peer-store/address-book.spec.js b/test/peer-store/address-book.spec.js
@@ -36,7 +36,7 @@ describe('addressBook', () => {
     let peerStore, ab
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       ab = peerStore.addressBook
     })
 
@@ -150,7 +150,7 @@ describe('addressBook', () => {
     let peerStore, ab
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       ab = peerStore.addressBook
     })
 
@@ -278,7 +278,7 @@ describe('addressBook', () => {
     let peerStore, ab
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       ab = peerStore.addressBook
     })
 
@@ -313,7 +313,7 @@ describe('addressBook', () => {
     let peerStore, ab
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       ab = peerStore.addressBook
     })
 
@@ -349,7 +349,7 @@ describe('addressBook', () => {
     let peerStore, ab
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       ab = peerStore.addressBook
     })
 
@@ -405,9 +405,9 @@ describe('addressBook', () => {
   describe('certified records', () => {
     let peerStore, ab
 
-    describe('consumes successfully a valid peer record and stores its data', () => {
+    describe('consumes a valid peer record and stores its data', () => {
       beforeEach(() => {
-        peerStore = new PeerStore()
+        peerStore = new PeerStore({ peerId })
         ab = peerStore.addressBook
       })
 
@@ -600,7 +600,7 @@ describe('addressBook', () => {
 
     describe('fails to consume invalid peer records', () => {
       beforeEach(() => {
-        peerStore = new PeerStore()
+        peerStore = new PeerStore({ peerId })
         ab = peerStore.addressBook
       })
 

diff --git a/test/peer-store/key-book.spec.js b/test/peer-store/key-book.spec.js
@@ -19,7 +19,7 @@ describe('keyBook', () => {
 
   beforeEach(async () => {
     [peerId] = await peerUtils.createPeerId()
-    peerStore = new PeerStore()
+    peerStore = new PeerStore({ peerId })
     kb = peerStore.keyBook
   })
 

diff --git a/test/peer-store/metadata-book.spec.js b/test/peer-store/metadata-book.spec.js
@@ -25,7 +25,7 @@ describe('metadataBook', () => {
     let peerStore, mb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       mb = peerStore.metadataBook
     })
 
@@ -158,7 +158,7 @@ describe('metadataBook', () => {
     let peerStore, mb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       mb = peerStore.metadataBook
     })
 
@@ -194,7 +194,7 @@ describe('metadataBook', () => {
     let peerStore, mb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       mb = peerStore.metadataBook
     })
 
@@ -243,7 +243,7 @@ describe('metadataBook', () => {
     let peerStore, mb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       mb = peerStore.metadataBook
     })
 
@@ -300,7 +300,7 @@ describe('metadataBook', () => {
     let peerStore, mb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       mb = peerStore.metadataBook
     })
 

diff --git a/test/peer-store/peer-store.spec.js b/test/peer-store/peer-store.spec.js
@@ -23,15 +23,15 @@ describe('peer-store', () => {
   let peerIds
   before(async () => {
     peerIds = await peerUtils.createPeerId({
-      number: 4
+      number: 5
     })
   })
 
   describe('empty books', () => {
     let peerStore
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId: peerIds[4] })
     })
 
     it('has an empty map of peers', () => {
@@ -61,7 +61,7 @@ describe('peer-store', () => {
     let peerStore
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId: peerIds[4] })
 
       // Add peer0 with { addr1, addr2 } and { proto1 }
       peerStore.addressBook.set(peerIds[0], [addr1, addr2])
@@ -163,7 +163,7 @@ describe('peer-store', () => {
     let peerStore
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId: peerIds[4] })
     })
 
     it('returns peers if only addresses are known', () => {