feat: discovery modules from transports should be added (#510)

* feat: discovery modules from transports should be added * chore: apply suggestions from code review Co-Authored-By: Jacob Heun <jacobheun@gmail.com> * chore: address review * chore: apply suggestions from code review Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
libp2p · Dec 12, 2019 · af96dcc · af96dcc
1 parent f540112
commit af96dcc
Show file tree

Hide file tree

Showing 5 changed files with 147 additions and 42 deletions.
diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md
@@ -110,6 +110,8 @@ Some available peer discovery modules are:
 - [js-libp2p-kad-dht](https://github.com/libp2p/js-libp2p-kad-dht)
 - [js-libp2p-webrtc-star](https://github.com/libp2p/js-libp2p-webrtc-star)
 
+**Note**: `peer-discovery` services within transports (such as `js-libp2p-webrtc-star`) are automatically gathered from the `transport`, via it's `discovery` property. As such, they do not need to be added in the discovery modules. However, these transports can also be configured and disabled as the other ones.
+
 If none of the available peer discovery protocols fulfills your needs, you can create a libp2p compatible one. A libp2p peer discovery protocol just needs to be compliant with the [Peer Discovery Interface](https://github.com/libp2p/js-interfaces/tree/master/src/peer-discovery).
 
 If you want to know more about libp2p peer discovery, you should read the following content:
@@ -268,6 +270,35 @@ const node = await Libp2p.create({
 })
 ```
 
+#### Setup webrtc transport and discovery
+
+```js
+
+const Libp2p = require('libp2p')
+const WS = require('libp2p-websockets')
+const WebRTCStar = require('libp2p-webrtc-star')
+const MPLEX = require('libp2p-mplex')
+const SECIO = require('libp2p-secio')
+
+const node = await Libp2p.create({
+  modules: {
+    transport: [
+      WS,
+      WebRTCStar
+    ],
+    streamMuxer: [MPLEX],
+    connEncryption: [SECIO],
+  },
+  config: {
+    peerDiscovery: {
+      webRTCStar: {
+        enabled: true
+      }
+    }
+  }
+})
+```
+
 #### Customizing Pubsub
 
 ```js

diff --git a/package.json b/package.json
@@ -96,6 +96,7 @@
     "libp2p-mplex": "^0.9.1",
     "libp2p-secio": "^0.12.1",
     "libp2p-tcp": "^0.14.1",
+    "libp2p-webrtc-star": "^0.17.0",
     "libp2p-websockets": "^0.13.1",
     "nock": "^10.0.6",
     "p-defer": "^3.0.0",

diff --git a/src/index.js b/src/index.js
@@ -48,7 +48,7 @@ class Libp2p extends EventEmitter {
     this._modules = this._options.modules
     this._config = this._options.config
     this._transport = [] // Transport instances/references
-    this._discovery = [] // Discovery service instances/references
+    this._discovery = new Map() // Discovery service instances/references
 
     this.peerStore = new PeerStore()
 
@@ -437,7 +437,7 @@ class Libp2p extends EventEmitter {
    * @returns {Promise<void>}
    */
   _setupPeerDiscovery () {
-    for (const DiscoveryService of this._modules.peerDiscovery || []) {
+    const setupService = (DiscoveryService) => {
       let config = {
         enabled: true // on by default
       }
@@ -448,7 +448,8 @@ class Libp2p extends EventEmitter {
         config = { ...config, ...this._config.peerDiscovery[DiscoveryService.tag] }
       }
 
-      if (config.enabled) {
+      if (config.enabled &&
+        !this._discovery.has(DiscoveryService.tag)) { // not already added
         let discoveryService
 
         if (typeof DiscoveryService === 'function') {
@@ -458,11 +459,23 @@ class Libp2p extends EventEmitter {
         }
 
         discoveryService.on('peer', this._onDiscoveryPeer)
-        this._discovery.push(discoveryService)
+        this._discovery.set(DiscoveryService.tag, discoveryService)
+      }
+    }
+
+    // Discovery modules
+    for (const DiscoveryService of this._modules.peerDiscovery || []) {
+      setupService(DiscoveryService)
+    }
+
+    // Transport modules with discovery
+    for (const Transport of this.transportManager.getTransports()) {
+      if (Transport.discovery) {
+        setupService(Transport.discovery)
       }
     }
 
-    return this._discovery.map(d => d.start())
+    return Promise.all(Array.from(this._discovery.values(), d => d.start()))
   }
 }
 

diff --git a/src/transport-manager.js b/src/transport-manager.js
@@ -103,6 +103,14 @@ class TransportManager {
     return addrs
   }
 
+  /**
+   * Returns all the transports instances.
+   * @returns {Iterator<Transport>}
+   */
+  getTransports () {
+    return this._transports.values()
+  }
+
   /**
    * Finds a transport that matches the given Multiaddr
    * @param {Multiaddr} ma

diff --git a/test/peer-discovery/index.spec.js b/test/peer-discovery/index.spec.js
@@ -10,57 +10,109 @@ const defer = require('p-defer')
 const mergeOptions = require('merge-options')
 
 const MulticastDNS = require('libp2p-mdns')
+const WebRTCStar = require('libp2p-webrtc-star')
 
 const Libp2p = require('../../src')
 const baseOptions = require('../utils/base-options.browser')
 const { createPeerInfo } = require('../utils/creators/peer')
 
 describe('peer discovery', () => {
-  let peerInfo
-  let remotePeerInfo
-  let libp2p
+  describe('basic functions', () => {
+    let peerInfo
+    let remotePeerInfo
+    let libp2p
 
-  before(async () => {
-    [peerInfo, remotePeerInfo] = await createPeerInfo({ number: 2 })
-  })
-
-  afterEach(async () => {
-    libp2p && await libp2p.stop()
-    sinon.reset()
-  })
+    before(async () => {
+      [peerInfo, remotePeerInfo] = await createPeerInfo({ number: 2 })
+    })
 
-  it('should dial know peers on startup', async () => {
-    libp2p = new Libp2p({
-      ...baseOptions,
-      peerInfo
+    afterEach(async () => {
+      libp2p && await libp2p.stop()
+      sinon.reset()
     })
-    libp2p.peerStore.add(remotePeerInfo)
-    const deferred = defer()
-    sinon.stub(libp2p.dialer, 'connectToPeer').callsFake((remotePeerInfo) => {
-      expect(remotePeerInfo).to.equal(remotePeerInfo)
-      deferred.resolve()
+
+    it('should dial know peers on startup', async () => {
+      libp2p = new Libp2p({
+        ...baseOptions,
+        peerInfo
+      })
+      libp2p.peerStore.add(remotePeerInfo)
+      const deferred = defer()
+      sinon.stub(libp2p.dialer, 'connectToPeer').callsFake((remotePeerInfo) => {
+        expect(remotePeerInfo).to.equal(remotePeerInfo)
+        deferred.resolve()
+      })
+      const spy = sinon.spy()
+      libp2p.on('peer:discovery', spy)
+
+      libp2p.start()
+      await deferred.promise
+      expect(spy.getCall(0).args).to.eql([remotePeerInfo])
     })
-    const spy = sinon.spy()
-    libp2p.on('peer:discovery', spy)
 
-    libp2p.start()
-    await deferred.promise
-    expect(spy.getCall(0).args).to.eql([remotePeerInfo])
+    it('should ignore self on discovery', async () => {
+      libp2p = new Libp2p(mergeOptions(baseOptions, {
+        peerInfo,
+        modules: {
+          peerDiscovery: [MulticastDNS]
+        }
+      }))
+
+      await libp2p.start()
+      const discoverySpy = sinon.spy()
+      libp2p.on('peer:discovery', discoverySpy)
+      libp2p._discovery.get('mdns').emit('peer', libp2p.peerInfo)
+
+      expect(discoverySpy.called).to.eql(false)
+    })
   })
 
-  it('should ignore self on discovery', async () => {
-    libp2p = new Libp2p(mergeOptions(baseOptions, {
-      peerInfo,
-      modules: {
-        peerDiscovery: [MulticastDNS]
-      }
-    }))
+  describe('discovery modules from transports', () => {
+    let peerInfo, libp2p
 
-    await libp2p.start()
-    const discoverySpy = sinon.spy()
-    libp2p.on('peer:discovery', discoverySpy)
-    libp2p._discovery[0].emit('peer', libp2p.peerInfo)
+    before(async () => {
+      [peerInfo] = await createPeerInfo()
+    })
+
+    it('should add discovery module if present in transports and enabled', async () => {
+      libp2p = new Libp2p(mergeOptions(baseOptions, {
+        peerInfo,
+        modules: {
+          transport: [WebRTCStar]
+        },
+        config: {
+          peerDiscovery: {
+            webRTCStar: {
+              enabled: true
+            }
+          }
+        }
+      }))
+
+      await libp2p.start()
+
+      expect(libp2p._discovery.size).to.eql(1)
+      expect(libp2p._discovery.has('webRTCStar')).to.eql(true)
+    })
+
+    it('should not add discovery module if present in transports but disabled', async () => {
+      libp2p = new Libp2p(mergeOptions(baseOptions, {
+        peerInfo,
+        modules: {
+          transport: [WebRTCStar]
+        },
+        config: {
+          peerDiscovery: {
+            webRTCStar: {
+              enabled: false
+            }
+          }
+        }
+      }))
 
-    expect(discoverySpy.called).to.eql(false)
+      await libp2p.start()
+
+      expect(libp2p._discovery.size).to.eql(0)
+    })
   })
 })