chore: apply suggestions from code review

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)
     }
 

src/peer-store/index.js

@@ -37,7 +37,7 @@ class PeerStore extends EventEmitter {
   /**
    * @constructor
    */
-  constructor ({ peerId } = {}) {
+  constructor ({ peerId }) {
     super()
 
     this._peerId = peerId

src/peer-store/persistent/index.js

@@ -189,13 +189,15 @@ class PersistentPeerStore extends PeerStore {
 
       const encodedData = Addresses.encode({
         addrs: entry.addresses.map((address) => ({
-          multiaddr: address.multiaddr.buffer
+          multiaddr: address.multiaddr.buffer,
+          isCertified: address.isCertified
         })),
         certified_record: entry.record ? {
           seq: entry.record.seqNumber,
           raw: entry.record.raw
         } : undefined
       })
+
       batch.put(key, encodedData)
     } catch (err) {
       log.error(err)
@@ -302,7 +304,8 @@ class PersistentPeerStore extends PeerStore {
             peerId,
             {
               addresses: decoded.addrs.map((address) => ({
-                multiaddr: multiaddr(address.multiaddr)
+                multiaddr: multiaddr(address.multiaddr),
+                isCertified: Boolean(address.isCertified)
               })),
               record: decoded.certified_record ? {
                 raw: decoded.certified_record.raw,

src/peer-store/persistent/pb/address-book.proto.js

@@ -7,6 +7,9 @@ message Addresses {
   // Address represents a single multiaddr.
   message Address {
     required bytes multiaddr = 1;
+
+    // Flag to indicate if the address comes from a certified source.
+    optional bool isCertified = 2;
   }
 
   // CertifiedRecord contains a serialized signed PeerRecord used to

src/record/envelope/index.js

@@ -112,7 +112,12 @@ const formatSignaturePayload = (domain, payloadType, payload) => {
   ])
 }
 
-const unmarshalEnvelope = async (data) => {
+/**
+ * Unmarshal a serialized Envelope protobuf message.
+ * @param {Buffer} data
+ * @return {Promise<Envelope>}
+ */
+Envelope.createFromProtobuf = async (data) => {
   const envelopeData = Protobuf.decode(data)
   const peerId = await PeerId.createFromPubKey(envelopeData.public_key)
 
@@ -124,13 +129,6 @@ const unmarshalEnvelope = async (data) => {
   })
 }
 
-/**
- * Unmarshal a serialized Envelope protobuf message.
- * @param {Buffer} data
- * @return {Promise<Envelope>}
- */
-Envelope.createFromProtobuf = unmarshalEnvelope
-
 /**
 * Seal marshals the given Record, places the marshaled bytes inside an Envelope
 * and signs it with the given peerId's private key.
@@ -163,7 +161,7 @@ Envelope.seal = async (record, peerId) => {
  * @return {Envelope}
  */
 Envelope.openAndCertify = async (data, domain) => {
-  const envelope = await unmarshalEnvelope(data)
+  const envelope = await Envelope.createFromProtobuf(data)
   const valid = await envelope.validate(domain)
 
   if (!valid) {

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
@@ -106,13 +106,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)

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(() => {
+  before(async () => {
-    peerStore = new PeerStore()
+    [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()

test/identify/index.spec.js

@@ -51,7 +51,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -61,7 +61,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -104,7 +104,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -114,7 +114,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: listenMaddrs
       },
       protocols
@@ -163,7 +163,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: localPeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: localPeer }),
         multiaddrs: []
       },
       protocols
@@ -172,7 +172,7 @@ describe('Identify', () => {
       libp2p: {
         peerId: remotePeer,
         connectionManager: new EventEmitter(),
-        peerStore: new PeerStore(),
+        peerStore: new PeerStore({ peerId: remotePeer }),
         multiaddrs: []
       },
       protocols
@@ -209,7 +209,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: localPeer,
           connectionManager: new EventEmitter(),
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: localPeer }),
           multiaddrs: listenMaddrs
         },
         protocols: new Map([
@@ -222,7 +222,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: remotePeer,
           connectionManager,
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: remotePeer }),
           multiaddrs: []
         }
       })
@@ -270,7 +270,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: localPeer,
           connectionManager: new EventEmitter(),
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: localPeer }),
           multiaddrs: listenMaddrs
         },
         protocols: new Map([
@@ -283,7 +283,7 @@ describe('Identify', () => {
         libp2p: {
           peerId: remotePeer,
           connectionManager,
-          peerStore: new PeerStore(),
+          peerStore: new PeerStore({ peerId: remotePeer }),
           multiaddrs: []
         }
       })

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
       })
 

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
   })
 

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
     })
 

test/peer-store/peer-store.spec.js

@@ -23,7 +23,7 @@ describe('peer-store', () => {
   let peerIds
   before(async () => {
     peerIds = await peerUtils.createPeerId({
-      number: 4
+      number: 5
     })
   })
 
@@ -31,7 +31,7 @@ describe('peer-store', () => {
     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', () => {

test/peer-store/persisted-peer-store.spec.js

@@ -17,11 +17,16 @@ const peerUtils = require('../utils/creators/peer')
 
 describe('Persisted PeerStore', () => {
   let datastore, peerStore
+  let peerId
+
+  before(async () => {
+    [peerId] = await peerUtils.createPeerId({ fixture: false })
+  })
 
   describe('start and stop flows', () => {
     beforeEach(() => {
       datastore = new MemoryDatastore()
-      peerStore = new PeerStore({ datastore })
+      peerStore = new PeerStore({ datastore, peerId })
     })
 
     afterEach(() => peerStore.stop())
@@ -54,7 +59,7 @@ describe('Persisted PeerStore', () => {
   describe('simple setup with content stored per change (threshold 1)', () => {
     beforeEach(() => {
       datastore = new MemoryDatastore()
-      peerStore = new PeerStore({ datastore, threshold: 1 })
+      peerStore = new PeerStore({ datastore, peerId, threshold: 1 })
     })
 
     afterEach(() => peerStore.stop())
@@ -319,10 +324,12 @@ describe('Persisted PeerStore', () => {
       const storedPeer0 = peerStore.get(peers[0])
       expect(storedPeer0.id.toB58String()).to.eql(peers[0].toB58String())
       expect(storedPeer0.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[0].toString()])
+      expect(storedPeer0.addresses.map((a) => a.isCertified)).to.have.members([true])
 
       const storedPeer1 = peerStore.get(peers[1])
       expect(storedPeer1.id.toB58String()).to.eql(peers[1].toB58String())
       expect(storedPeer1.addresses.map((a) => a.multiaddr.toString())).to.have.members([multiaddrs[1].toString()])
+      expect(storedPeer1.addresses.map((a) => a.isCertified)).to.have.members([true])
     })
 
     it('should delete certified peer records from the datastore on delete', async () => {
@@ -377,7 +384,7 @@ describe('Persisted PeerStore', () => {
   describe('setup with content not stored per change (threshold 2)', () => {
     beforeEach(() => {
       datastore = new MemoryDatastore()
-      peerStore = new PeerStore({ datastore, threshold: 2 })
+      peerStore = new PeerStore({ datastore, peerId, threshold: 2 })
     })
 
     afterEach(() => peerStore.stop())

test/peer-store/proto-book.spec.js

@@ -27,7 +27,7 @@ describe('protoBook', () => {
     let peerStore, pb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       pb = peerStore.protoBook
     })
 
@@ -121,7 +121,7 @@ describe('protoBook', () => {
     let peerStore, pb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       pb = peerStore.protoBook
     })
 
@@ -228,7 +228,7 @@ describe('protoBook', () => {
     let peerStore, pb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       pb = peerStore.protoBook
     })
 
@@ -258,7 +258,7 @@ describe('protoBook', () => {
     let peerStore, pb
 
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       pb = peerStore.protoBook
     })
 

test/registrar/registrar.spec.js

@@ -21,10 +21,15 @@ const multicodec = '/test/1.0.0'
 describe('registrar', () => {
   let peerStore
   let registrar
+  let peerId
+
+  before(async () => {
+    [peerId] = await peerUtils.createPeerId()
+  })
 
   describe('errors', () => {
     beforeEach(() => {
-      peerStore = new PeerStore()
+      peerStore = new PeerStore({ peerId })
       registrar = new Registrar({ peerStore, connectionManager: new EventEmitter() })
     })