chore: apply suggestions from code review

Co-authored-by: Jacob Heun <jacobheun@gmail.com>

Author: vasco-santos

doc/API.md

@@ -74,7 +74,7 @@ Creates an instance of Libp2p.
 | [options.dialer] | `object` | libp2p Dialer configuration
 | [options.metrics] | `object` | libp2p Metrics configuration
 | [options.peerId] | [`PeerId`][peer-id] | peerId instance (it will be created if not provided) |
-| [options.peerStore] | [`PeerId`][peer-id] | libp2p PeerStore configuration |
+| [options.peerStore] | `object` | libp2p PeerStore configuration |
 
 For Libp2p configurations and modules details read the [Configuration Document](./CONFIGURATION.md).
 

doc/CONFIGURATION.md

@@ -508,7 +508,9 @@ const node = await Libp2p.create({
 
 #### Configuring PeerStore
 
-PeerStore persistence is disabled in libp2p by default. You can enable and configure it as follows. Aside from enabled being `false` by default, it will need an implementation of a [datastore](https://github.com/ipfs/interface-datastore).
+PeerStore persistence is disabled in libp2p by default. You can enable and configure it as follows. Aside from enabled being `false` by default, it will need an implementation of a [datastore](https://github.com/ipfs/interface-datastore). Take into consideration that using the memory datastore will be ineffective for persistence.
+
+The threshold number represents the maximum number of "dirty peers" allowed in the PeerStore, i.e. peers that are not updated in the datastore. In this context, browser nodes should use a threshold of 1, since they might not "stop" properly in several scenarios and the PeerStore might end up with unflushed records when the window is closed.
 
 ```js
 const Libp2p = require('libp2p')
@@ -526,8 +528,8 @@ const node = await Libp2p.create({
   },
   datastore: new LevelStore('path/to/store'),
   peerStore: {
-    persistence: true,
-    threshold: 5
+    persistence: true, // Is persistence enabled (default: false)
+    threshold: 5 // Number of dirty peers allowed (default: 5)
   }
 })
 ```

src/index.js

@@ -47,12 +47,12 @@ class Libp2p extends EventEmitter {
     this.peerId = this._options.peerId
     this.datastore = this._options.datastore
 
-    this.peerStore = !(this.datastore && this._options.peerStore.persistence)
-      ? new PeerStore()
-      : new PersistentPeerStore({
+    this.peerStore = (this.datastore && this._options.peerStore.persistence)
+      ? new PersistentPeerStore({
         datastore: this.datastore,
         ...this._options.peerStore
       })
+      : new PeerStore()
 
     // Addresses {listen, announce, noAnnounce}
     this.addresses = this._options.addresses

src/peer-store/README.md

@@ -87,21 +87,21 @@ Access to its underlying books:
 
 The data stored in the PeerStore can be persisted if configured appropriately. Keeping a record of the peers already discovered by the peer, as well as their known data aims to improve the efficiency of peers joining the network after being offline.
 
-The libp2p node will need to receive a [datastore](https://github.com/ipfs/interface-datastore), in order to store this data in a persistent way. A [datastore](https://github.com/ipfs/interface-datastore) stores its data in a key-value fashion. As a result, we need coherent keys so that we do not overwrite data.
+The libp2p node will need to receive a [datastore](https://github.com/ipfs/interface-datastore), in order to persist this data across restarts. A [datastore](https://github.com/ipfs/interface-datastore) stores its data in a key-value fashion. As a result, we need coherent keys so that we do not overwrite data.
 
-The PeerStore should not be continuously updating the datastore with the new data observed. Accordingly, it should only store new data after reaching a certain threshold of "dirty" peers, as well as when the node is stopped.
+The PeerStore should not continuously update the datastore whenever data is changed. Instead, it should only store new data after reaching a certain threshold of "dirty" peers, as well as when the node is stopped, in order to batch writes to the datastore.
 
-Taking into account that a datastore allows queries using a key prefix, we can find all the information if we define a consistent namespace that allow us to find the content without having any information. The namespaces were defined as follows:
+The peer id will be appended to the datastore key for each data namespace. The namespaces were defined as follows:
 
 **AddressBook**
 
-All the knownw peer addresses are stored with a key pattern as follows:
+All the known peer addresses are stored with a key pattern as follows:
 
 `/peers/addrs/<b32 peer id no padding>`
 
 **ProtoBook**
 
-All the knownw peer protocols are stored with a key pattern as follows:
+All the known peer protocols are stored with a key pattern as follows:
 
 `/peers/protos/<b32 peer id no padding>`
 

src/peer-store/persistent/consts.js

@@ -1,9 +1,9 @@
 'use strict'
 
-module.exports.COMMON_NAMESPACE = '/peers/'
+module.exports.NAMESPACE_COMMON = '/peers/'
 
 // /peers/protos/<b32 peer id no padding>
-module.exports.ADDRESS_NAMESPACE = '/peers/addrs/'
+module.exports.NAMESPACE_ADDRESS = '/peers/addrs/'
 
 // /peers/addrs/<b32 peer id no padding>
-module.exports.PROTOCOL_NAMESPACE = '/peers/protos/'
+module.exports.NAMESPACE_PROTOCOL = '/peers/protos/'

src/peer-store/persistent/index.js

@@ -11,9 +11,9 @@ const PeerId = require('peer-id')
 const PeerStore = require('..')
 
 const {
-  ADDRESS_NAMESPACE,
-  COMMON_NAMESPACE,
-  PROTOCOL_NAMESPACE
+  NAMESPACE_ADDRESS,
+  NAMESPACE_COMMON,
+  NAMESPACE_PROTOCOL
 } = require('./consts')
 
 const Addresses = require('./pb/address-book.proto')
@@ -51,25 +51,25 @@ class PersistentPeerStore extends PeerStore {
    * @return {Promise<void>}
    */
   async start () {
-    log('Persistent PeerStore is starting')
+    log('PeerStore is starting')
 
     // Handlers for dirty peers
     this.on('change:protocols', this._addDirtyPeer)
     this.on('change:multiaddrs', this._addDirtyPeer)
 
     // Load data
-    for await (const entry of this._datastore.query({ prefix: COMMON_NAMESPACE })) {
+    for await (const entry of this._datastore.query({ prefix: NAMESPACE_COMMON })) {
       this._processDatastoreEntry(entry)
     }
 
-    log('Persistent PeerStore started')
+    log('PeerStore started')
   }
 
   async stop () {
-    log('Persistent PeerStore is stopping')
+    log('PeerStore is stopping')
     this.removeAllListeners()
     await this._commitData()
-    log('Persistent PeerStore stopped')
+    log('PeerStore stopped')
   }
 
   /**
@@ -131,7 +131,7 @@ class PersistentPeerStore extends PeerStore {
    */
   _batchAddressBook (peerId, batch) {
     const b32key = peerId.toString()
-    const key = new Key(`${ADDRESS_NAMESPACE}${b32key}`)
+    const key = new Key(`${NAMESPACE_ADDRESS}${b32key}`)
 
     const addresses = this.addressBook.get(peerId)
 
@@ -162,7 +162,7 @@ class PersistentPeerStore extends PeerStore {
    */
   _batchProtoBook (peerId, batch) {
     const b32key = peerId.toString()
-    const key = new Key(`${PROTOCOL_NAMESPACE}${b32key}`)
+    const key = new Key(`${NAMESPACE_PROTOCOL}${b32key}`)
 
     const protocols = this.protoBook.get(peerId)
 
@@ -185,7 +185,7 @@ class PersistentPeerStore extends PeerStore {
    * Process datastore entry and add its data to the correct book.
    * @private
    * @param {Object} params
-   * @param {string} params.key datastore key
+   * @param {Key} params.key datastore key
    * @param {Buffer} params.value datastore value stored
    */
   _processDatastoreEntry ({ key, value }) {