From 7f216a90296a792b2dbd3ac41ab378e4e254e94a Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 11 Dec 2019 14:17:34 +0100 Subject: [PATCH 1/4] feat: discovery modules from transports should be added --- doc/CONFIGURATION.md | 31 ++++++++ package.json | 1 + src/index.js | 17 +++- src/transport-manager.js | 8 ++ test/peer-discovery/index.spec.js | 125 +++++++++++++++++++++--------- 5 files changed, 143 insertions(+), 39 deletions(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 801e4f15fe..e11fb320ce 100644 --- 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) +Take into consideration that `peer-discovery` services within transports (such as `js-libp2p-webrtc-star`) are automatically gathered from the `transport` and 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 index eb874cc2e8..6ba5bcd654 100644 --- 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 index aad21e288a..d382206a02 100644 --- a/src/index.js +++ b/src/index.js @@ -437,7 +437,7 @@ class Libp2p extends EventEmitter { * @returns {Promise} */ _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.filter((service) => service.tag === DiscoveryService.tag).length) { // not already added let discoveryService if (typeof DiscoveryService === 'function') { @@ -462,6 +463,18 @@ class Libp2p extends EventEmitter { } } + // 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()) } } diff --git a/src/transport-manager.js b/src/transport-manager.js index 5e4a9201a7..d11189ab86 100644 --- 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} + */ + 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 index 3f220e47fa..2c7592d95b 100644 --- a/test/peer-discovery/index.spec.js +++ b/test/peer-discovery/index.spec.js @@ -10,57 +10,108 @@ 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[0].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).to.have.lengthOf(1) + }) + + 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).to.have.lengthOf(0) + }) }) }) From 00539eb5212e86589ae1941a15b556f8aa69ede1 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 11 Dec 2019 16:21:01 +0100 Subject: [PATCH 2/4] chore: apply suggestions from code review Co-Authored-By: Jacob Heun --- doc/CONFIGURATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index e11fb320ce..9904d1c68c 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -110,7 +110,7 @@ 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) -Take into consideration that `peer-discovery` services within transports (such as `js-libp2p-webrtc-star`) are automatically gathered from the `transport` and do not need to be added in the discovery modules. However, these transports can also be configured and disabled as the other ones. +**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). From 91f960880df0b402948730a58aae6ca139cf7671 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 11 Dec 2019 16:35:09 +0100 Subject: [PATCH 3/4] chore: address review --- src/index.js | 8 ++++---- test/peer-discovery/index.spec.js | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index d382206a02..2aef2883b1 100644 --- 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() @@ -449,7 +449,7 @@ class Libp2p extends EventEmitter { } if (config.enabled && - !this._discovery.filter((service) => service.tag === DiscoveryService.tag).length) { // not already added + !this._discovery.has(DiscoveryService.tag)) { // not already added let discoveryService if (typeof DiscoveryService === 'function') { @@ -459,7 +459,7 @@ class Libp2p extends EventEmitter { } discoveryService.on('peer', this._onDiscoveryPeer) - this._discovery.push(discoveryService) + this._discovery.set(DiscoveryService.tag, discoveryService) } } @@ -475,7 +475,7 @@ class Libp2p extends EventEmitter { } } - return this._discovery.map(d => d.start()) + return Array.from(this._discovery.values()).map(d => d.start()) } } diff --git a/test/peer-discovery/index.spec.js b/test/peer-discovery/index.spec.js index 2c7592d95b..66e08d37bd 100644 --- a/test/peer-discovery/index.spec.js +++ b/test/peer-discovery/index.spec.js @@ -61,7 +61,7 @@ describe('peer discovery', () => { await libp2p.start() const discoverySpy = sinon.spy() libp2p.on('peer:discovery', discoverySpy) - libp2p._discovery[0].emit('peer', libp2p.peerInfo) + libp2p._discovery.get('mdns').emit('peer', libp2p.peerInfo) expect(discoverySpy.called).to.eql(false) }) @@ -91,7 +91,8 @@ describe('peer discovery', () => { await libp2p.start() - expect(libp2p._discovery).to.have.lengthOf(1) + 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 () => { @@ -111,7 +112,7 @@ describe('peer discovery', () => { await libp2p.start() - expect(libp2p._discovery).to.have.lengthOf(0) + expect(libp2p._discovery.size).to.eql(0) }) }) }) From cbb80489919527b44fb74bb808bf6a0ee03e4a69 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 11 Dec 2019 16:50:33 +0100 Subject: [PATCH 4/4] chore: apply suggestions from code review Co-Authored-By: Jacob Heun --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 2aef2883b1..8ef3252c6c 100644 --- a/src/index.js +++ b/src/index.js @@ -475,7 +475,7 @@ class Libp2p extends EventEmitter { } } - return Array.from(this._discovery.values()).map(d => d.start()) + return Promise.all(Array.from(this._discovery.values(), d => d.start())) } }