chore: address review

doc/API.md

@@ -89,13 +89,13 @@ Creates an instance of Libp2p.
 | [options.addresses] | `{ listen: Array<string>, announce: Array<string>, noAnnounce: Array<string> }` | Addresses for transport listening and to advertise to the network |
 | [options.config] | `object` | libp2p modules configuration and core configuration |
 | [options.connectionManager] | [`object`](./CONFIGURATION.md#configuring-connection-manager) | libp2p Connection Manager [configuration](./CONFIGURATION.md#configuring-connection-manager) |
-| [options.transportManager] | [`object`](./CONFIGURATION.md#configuring-transport-manager) | libp2p transport manager [configuration]((./CONFIGURATION.md#configuring-transport-manager)) |
+| [options.transportManager] | [`object`](./CONFIGURATION.md#configuring-transport-manager) | libp2p transport manager [configuration](./CONFIGURATION.md#configuring-transport-manager) |
 | [options.datastore] | `object` | must implement [ipfs/interface-datastore](https://github.com/ipfs/interface-datastore) (in memory datastore will be used if not provided) |
-| [options.dialer] | [`object`](./CONFIGURATION.md#configuring-dialing) | libp2p Dialer [configuration]((./CONFIGURATION.md#configuring-dialing))
-| [options.keychain] | [`object`](./CONFIGURATION.md#setup-with-keychain) | keychain [configuration]((./CONFIGURATION.md#setup-with-keychain)) |
-| [options.metrics] | [`object`](./CONFIGURATION.md#configuring-metrics) | libp2p Metrics [configuration]((./CONFIGURATION.md#configuring-metrics)) |
+| [options.dialer] | [`object`](./CONFIGURATION.md#configuring-dialing) | libp2p Dialer [configuration](./CONFIGURATION.md#configuring-dialing)
+| [options.keychain] | [`object`](./CONFIGURATION.md#setup-with-keychain) | keychain [configuration](./CONFIGURATION.md#setup-with-keychain) |
+| [options.metrics] | [`object`](./CONFIGURATION.md#configuring-metrics) | libp2p Metrics [configuration](./CONFIGURATION.md#configuring-metrics) |
 | [options.peerId] | [`PeerId`][peer-id] | peerId instance (it will be created if not provided) |
-| [options.peerStore] | [`object`](./CONFIGURATION.md#configuring-peerstore) | libp2p PeerStore [configuration]((./CONFIGURATION.md#configuring-peerstore)) |
+| [options.peerStore] | [`object`](./CONFIGURATION.md#configuring-peerstore) | libp2p PeerStore [configuration](./CONFIGURATION.md#configuring-peerstore) |
 
 For Libp2p configurations and modules details read the [Configuration Document](./CONFIGURATION.md).
 

doc/CONFIGURATION.md

@@ -52,7 +52,7 @@ The libp2p ecosystem contains at least one module for each of these subsystems.
 
 After selecting the modules to use, it is also possible to configure each one according to your needs.
 
-Bear in mind that a **transport** and **connection encryption** are **required**, while all the other subsystems are optional.
+Bear in mind that a **transport** and **connection encryption** module are **required**, while all the other subsystems are optional.
 
 ### Transport
 

package.json

@@ -60,7 +60,7 @@
     "it-protocol-buffers": "^0.2.0",
     "libp2p-crypto": "^0.17.6",
     "libp2p-interfaces": "libp2p/js-libp2p-interfaces#feat/record-interface",
-    "libp2p-utils": "^0.1.2",
+    "libp2p-utils": "libp2p/js-libp2p-utils#feat/array-equals-for-non-primitive-types-with-equal-function",
     "mafmt": "^7.0.0",
     "merge-options": "^2.0.0",
     "moving-average": "^1.0.0",

src/insecure/plaintext.js

@@ -43,7 +43,7 @@ async function encrypt (localId, conn, remoteId) {
     throw new InvalidCryptoExchangeError('Remote did not provide its public key')
   }
 
-  if (remoteId && !peerId.isEqual(remoteId)) {
+  if (remoteId && !peerId.equals(remoteId)) {
     throw new UnexpectedPeerError()
   }
 

src/record/README.md

@@ -12,42 +12,57 @@ You can read further about the envelope in [libp2p/specs#217](https://github.com
 
 ### Usage
 
-- create an envelope with an instance of an `interface-record` implementation and prepare it for being exchanged:
+- create an envelope with an instance of an [interface-record](https://github.com/libp2p/js-libp2p-interfaces/tree/master/src/record) implementation and prepare it for being exchanged:
 
 ```js
-const Envelope = require('libp2p/src/record/envelop')
+// interface-record implementation example with the "libp2p-example" namespace
+const Record = require('libp2p-interfaces/src/record')
+
+class ExampleRecord extends Record {
+  constructor () {
+    super ('libp2p-example', Buffer.from('0302', 'hex'))
+  }
+
+  marshal () {}
 
-// ... create a record named rec with domain X
+  equals (other) {}
+}
+
+ExampleRecord.createFromProtobuf = () => {}
+```
 
+```js
+const Envelope = require('libp2p/src/record/envelop')
+const ExampleRecord = require('./example-record')
+
+const rec = new ExampleRecord()
 const e = await Envelope.seal(rec, peerId)
 const wireData = e.marshal()
 ```
 
-- consume a received envelope, as well as to get back the record:
+- consume a received envelope (`wireData`) and transform it back to a record:
 
 ```js
 const Envelope = require('libp2p/src/record/envelop')
-// const Record = ...
-
-// ... receive envelope data
+const ExampleRecord = require('./example-record')
 
-const domain = 'X'
+const domain = 'libp2p-example'
 let e
 
 try {
-  e = await Envelope.openAndCertify(data, domain)
+  e = await Envelope.openAndCertify(wireData, domain)
 } catch (err) {}
 
-const rec = Record.createFromProtobuf(e.payload)
+const rec = ExampleRecord.createFromProtobuf(e.payload)
 ```
 
 ## Peer Record
 
 All libp2p nodes keep a `PeerStore`, that among other information stores a set of known addresses for each peer, which can come from a variety of sources.
 
-Libp2p peer records were created to enable the distribution of verifiable address records, which we can prove originated from the addressed peer itself. With such guarantees, libp2p can prioritize addresses based on their authenticity, with the most strict strategy being to only dial certified addresses.
+Libp2p peer records were created to enable the distribution of verifiable address records, which we can prove originated from the addressed peer itself. With such guarantees, libp2p is able to prioritize addresses based on their authenticity, with the most strict strategy being to only dial certified addresses (no strategies implemented at the time of writing).
 
-A peer record contains the peers' publicly reachable listen addresses, and may be extended in the future to contain additional metadata relevant to routing. It also contains a `seqNumber` field, so that we can order peer records by time and identify if a received record is more recent than the stored one.
+A peer record contains the peers' publicly reachable listen addresses, and may be extended in the future to contain additional metadata relevant to routing. It also contains a `seqNumber` field, a timestamp per the spec, so that we can verify the most recent record.
 
 You can read further about the Peer Record in [libp2p/specs#217](https://github.com/libp2p/specs/pull/217).
 
@@ -78,29 +93,29 @@ const pr = PeerRecord.createFromProtobuf(data)
 
 Once a libp2p node has started and is listening on a set of multiaddrs, its own peer record can be created.
 
-The identify service is responsible for creating the self record when the identify protocol kicks in for the first time. This record should be stored for future needs of the identify protocol when connecting with other peers.
+The identify service is responsible for creating the self record when the identify protocol kicks in for the first time. This record will be stored for future needs of the identify protocol when connecting with other peers.
 
 #### Self record Updates
 
 **_NOT_YET_IMPLEMENTED_**
 
-While creating peer records is fairly trivial, addresses should not be static and can be modified at arbitrary times. This can happen via an Address Manager API, or even through AutoRelay/AutoNAT.
+While creating peer records is fairly trivial, addresses are not static and might be modified at arbitrary times. This can happen via an Address Manager API, or even through AutoRelay/AutoNAT.
 
-When a libp2p node changes its listen addresses, the identify service should be informed. Once that happens, the identify service should create a new self record and store it. With the new record, the identify push/delta protocol will be used to communicate this change to the connected peers.
+When a libp2p node changes its listen addresses, the identify service will be informed. Once that happens, the identify service creates a new self record and stores it. With the new record, the identify push/delta protocol will be used to communicate this change to the connected peers.
 
 #### Subsystem receiving a record
 
-Considering that a node can discover other peers' addresses from a variety of sources, Libp2p Peerstore should be able to differentiate the addresses that were obtained through a signed peer record.
+Considering that a node can discover other peers' addresses from a variety of sources, Libp2p Peerstore can differentiate the addresses that were obtained through a signed peer record.
 
-Once a record is received and its signature properly validated, its envelope should be stored in the AddressBook on its byte representations. However, the `seqNumber` number of the record must be compared with potentially stored records, so that we do not override correct data.
+Once a record is received and its signature properly validated, its envelope is stored in the AddressBook in its byte representation. The `seqNumber` remains unmarshalled so that we can quickly compare it against incoming records to determine the most recent record.
 
-The AddressBook Addresses must be updated with the content of the envelope with a certified property that allows other subsystems to identify that the known certified addresses of a peer.
+The AddressBook Addresses will be updated with the content of the envelope with a certified property. This allows other subsystems to identify the known certified addresses of a peer.
 
 #### Subsystem providing a record
 
-Libp2p subsystems that exchange other peers information should provide the envelope that they received by those peers. As a result, other peers can verify if the envelope was really created by the addressed peer.
+Libp2p subsystems that exchange other peers information will provide the envelope that they received by those peers. As a result, other peers can verify if the envelope was really created by the addressed peer.
 
-When a subsystem wants to provide a record, it should get it from the AddressBook if it exists. Other subsystems should also be able to provide the self record that will also be stored in the AddressBook.
+When a subsystem wants to provide a record, it will get it from the AddressBook, if it exists. Other subsystems are also able to provide the self record, since it is also stored in the AddressBook.
 
 ### Future Work
 

src/record/envelope/index.js

@@ -63,7 +63,7 @@ class Envelope {
    * @param {Envelope} other
    * @return {boolean}
    */
-  isEqual (other) {
+  equals (other) {
     return this.peerId.pubKey.bytes.equals(other.peerId.pubKey.bytes) &&
       this.payloadType.equals(other.payloadType) &&
       this.payload.equals(other.payload) &&
@@ -76,7 +76,7 @@ class Envelope {
    * @return {Promise<boolean>}
    */
   validate (domain) {
-    const signData = createSignData(domain, this.payloadType, this.payload)
+    const signData = formatSignaturePayload(domain, this.payloadType, this.payload)
 
     return this.peerId.pubKey.verify(signData, this.signature)
   }
@@ -89,7 +89,7 @@ class Envelope {
  * @param {Buffer} payload
  * @return {Buffer}
  */
-const createSignData = (domain, payloadType, payload) => {
+const formatSignaturePayload = (domain, payloadType, payload) => {
   // When signing, a peer will prepare a buffer by concatenating the following:
   // - The length of the domain separation string string in bytes
   // - The domain separation string, encoded as UTF-8
@@ -142,7 +142,7 @@ Envelope.seal = async (record, peerId) => {
   const payloadType = Buffer.from(record.codec)
   const payload = record.marshal()
 
-  const signData = createSignData(domain, payloadType, payload)
+  const signData = formatSignaturePayload(domain, payloadType, payload)
   const signature = await peerId.privKey.sign(signData)
 
   return new Envelope({
@@ -155,7 +155,7 @@ Envelope.seal = async (record, peerId) => {
 
 /**
  * Open and certify a given marshalled envelope.
- * Data is unmarshalled and the siganture validated with the given domain.
+ * Data is unmarshalled and the signature validated for the given domain.
  * @param {Buffer} data
  * @param {string} domain
  * @return {Envelope}

src/record/peer-record/index.js

@@ -3,24 +3,24 @@
 const multiaddr = require('multiaddr')
 const PeerId = require('peer-id')
 const Record = require('libp2p-interfaces/src/record')
+const arrayEquals = require('libp2p-utils/src/array-equals')
 
 const Protobuf = require('./peer-record.proto')
 const {
   ENVELOPE_DOMAIN_PEER_RECORD,
   ENVELOPE_PAYLOAD_TYPE_PEER_RECORD
 } = require('./consts')
 
-const arraysAreEqual = (a, b) => a.length === b.length && a.sort().every((item, index) => b[index].equals(item))
-
 /**
- * The PeerRecord is responsible for TODOTODOTRDO
+ * The PeerRecord is used for distributing peer routing records across the network.
+ * It contains the peer's reachable listen addresses.
  */
 class PeerRecord extends Record {
   /**
    * @constructor
    * @param {object} params
    * @param {PeerId} params.peerId
-   * @param {Array<multiaddr>} params.multiaddrs public addresses of the peer this record pertains to.
+   * @param {Array<multiaddr>} params.multiaddrs addresses of the associated peer.
    * @param {number} [params.seqNumber] monotonically-increasing sequence counter that's used to order PeerRecords in time.
    */
   constructor ({ peerId, multiaddrs = [], seqNumber = Date.now() }) {
@@ -55,11 +55,11 @@ class PeerRecord extends Record {
   }
 
   /**
-   * Verifies if the other PeerRecord is identical to this one.
+   * Returns true if `this` record equals the `other`.
    * @param {Record} other
    * @return {boolean}
    */
-  isEqual (other) {
+  equals (other) {
     // Validate PeerId
     if (!this.peerId.equals(other.peerId)) {
       return false
@@ -71,7 +71,7 @@ class PeerRecord extends Record {
     }
 
     // Validate multiaddrs
-    if (this.multiaddrs.length !== other.multiaddrs.length || !arraysAreEqual(this.multiaddrs, other.multiaddrs)) {
+    if (!arrayEquals(this.multiaddrs, other.multiaddrs)) {
       return false
     }
 

test/connection-manager/index.node.js

@@ -75,7 +75,7 @@ describe('Connection Manager', () => {
     expect(libp2p.connectionManager.emit.callCount).to.equal(1)
     const [event, connection] = libp2p.connectionManager.emit.getCall(0).args
     expect(event).to.equal('peer:connect')
-    expect(connection.remotePeer.isEqual(remoteLibp2p.peerId)).to.equal(true)
+    expect(connection.remotePeer.equals(remoteLibp2p.peerId)).to.equal(true)
 
     const libp2pConn = libp2p.connectionManager.get(remoteLibp2p.peerId)
     expect(libp2pConn).to.exist()

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

@@ -7,6 +7,7 @@ const { expect } = chai
 
 const pDefer = require('p-defer')
 const multiaddr = require('multiaddr')
+const arrayEquals = require('libp2p-utils/src/array-equals')
 
 const PeerStore = require('../../src/peer-store')
 
@@ -19,8 +20,6 @@ const addr1 = multiaddr('/ip4/127.0.0.1/tcp/8000')
 const addr2 = multiaddr('/ip4/127.0.0.1/tcp/8001')
 const addr3 = multiaddr('/ip4/127.0.0.1/tcp/8002')
 
-const arraysAreEqual = (a, b) => a.length === b.length && a.sort().every((item, index) => b[index] === item)
-
 describe('addressBook', () => {
   let peerId
 
@@ -194,7 +193,7 @@ describe('addressBook', () => {
       let changeTrigger = 2
       peerStore.on('change:multiaddrs', ({ multiaddrs }) => {
         changeTrigger--
-        if (changeTrigger === 0 && arraysAreEqual(multiaddrs, finalMultiaddrs)) {
+        if (changeTrigger === 0 && arrayEquals(multiaddrs, finalMultiaddrs)) {
           defer.resolve()
         }
       })

test/record/envelope.spec.js

@@ -25,7 +25,7 @@ class TestRecord extends Record {
     return Buffer.from(this.data)
   }
 
-  isEqual (other) {
+  equals (other) {
     return Buffer.compare(this.data, other.data)
   }
 }
@@ -74,8 +74,8 @@ describe('Envelope', () => {
     const unmarshalledEnvelope = await Envelope.openAndCertify(rawEnvelope, testRecord.domain)
     expect(unmarshalledEnvelope).to.exist()
 
-    const isEqual = envelope.isEqual(unmarshalledEnvelope)
-    expect(isEqual).to.eql(true)
+    const equals = envelope.equals(unmarshalledEnvelope)
+    expect(equals).to.eql(true)
   })
 
   it('throw on open and verify when a different domain is used', async () => {

test/record/peer-record.spec.js

@@ -72,32 +72,32 @@ describe('PeerRecord', () => {
     const unmarshalPeerRecord = PeerRecord.createFromProtobuf(rawData)
     expect(unmarshalPeerRecord).to.exist()
 
-    const isEqual = peerRecord.isEqual(unmarshalPeerRecord)
-    expect(isEqual).to.eql(true)
+    const equals = peerRecord.equals(unmarshalPeerRecord)
+    expect(equals).to.eql(true)
   })
 
-  it('isEqual returns false if the peer record has a different peerId', async () => {
+  it('equals returns false if the peer record has a different peerId', async () => {
     const peerRecord0 = new PeerRecord({ peerId })
 
     const [peerId1] = await peerUtils.createPeerId({ fixture: false })
     const peerRecord1 = new PeerRecord({ peerId: peerId1 })
 
-    const isEqual = peerRecord0.isEqual(peerRecord1)
-    expect(isEqual).to.eql(false)
+    const equals = peerRecord0.equals(peerRecord1)
+    expect(equals).to.eql(false)
   })
 
-  it('isEqual returns false if the peer record has a different seqNumber', () => {
+  it('equals returns false if the peer record has a different seqNumber', () => {
     const ts0 = Date.now()
     const peerRecord0 = new PeerRecord({ peerId, seqNumber: ts0 })
 
     const ts1 = ts0 + 20
     const peerRecord1 = new PeerRecord({ peerId, seqNumber: ts1 })
 
-    const isEqual = peerRecord0.isEqual(peerRecord1)
-    expect(isEqual).to.eql(false)
+    const equals = peerRecord0.equals(peerRecord1)
+    expect(equals).to.eql(false)
   })
 
-  it('isEqual returns false if the peer record has a different multiaddrs', () => {
+  it('equals returns false if the peer record has a different multiaddrs', () => {
     const multiaddrs = [
       multiaddr('/ip4/127.0.0.1/tcp/2000')
     ]
@@ -108,8 +108,8 @@ describe('PeerRecord', () => {
     ]
     const peerRecord1 = new PeerRecord({ peerId, multiaddrs: multiaddrs1 })
 
-    const isEqual = peerRecord0.isEqual(peerRecord1)
-    expect(isEqual).to.eql(false)
+    const equals = peerRecord0.equals(peerRecord1)
+    expect(equals).to.eql(false)
   })
 })
 
@@ -135,7 +135,7 @@ describe('PeerRecord inside Envelope', () => {
 
     const decodedPeerRecord = PeerRecord.createFromProtobuf(decodedE.payload)
 
-    const isEqual = peerRecord.isEqual(decodedPeerRecord)
-    expect(isEqual).to.eql(true)
+    const equals = peerRecord.equals(decodedPeerRecord)
+    expect(equals).to.eql(true)
   })
 })

test/upgrading/upgrader.spec.js

@@ -467,13 +467,13 @@ describe('libp2p.upgrader', () => {
 
     let [event, connection] = libp2p.connectionManager.emit.getCall(0).args
     expect(event).to.equal('peer:connect')
-    expect(connection.remotePeer.isEqual(remotePeer)).to.equal(true)
+    expect(connection.remotePeer.equals(remotePeer)).to.equal(true)
 
     // Close and check the disconnect event
     await Promise.all(connections.map(conn => conn.close()))
     expect(libp2p.connectionManager.emit.callCount).to.equal(2)
     ;([event, connection] = libp2p.connectionManager.emit.getCall(1).args)
     expect(event).to.equal('peer:disconnect')
-    expect(connection.remotePeer.isEqual(remotePeer)).to.equal(true)
+    expect(connection.remotePeer.equals(remotePeer)).to.equal(true)
   })
 })