From 695f77ca9be5dddf478e507d9feb2a97eceeb935 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 9 Jul 2020 11:11:02 +0200 Subject: [PATCH 1/9] fix: not dial all known peers on startup --- src/index.js | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index ad7a78281c..99ca1b6e4d 100644 --- a/src/index.js +++ b/src/index.js @@ -467,10 +467,7 @@ class Libp2p extends EventEmitter { }) // Once we start, emit and dial any peers we may have already discovered - for (const peer of this.peerStore.peers.values()) { - this.emit('peer:discovery', peer.id) - this._maybeConnect(peer.id) - } + this._maybeConnectPeerStorePeers() // Peer discovery await this._setupPeerDiscovery() @@ -492,6 +489,33 @@ class Libp2p extends EventEmitter { peer.protocols && this.peerStore.protoBook.set(peer.id, peer.protocols) } + /** + * Will dial a substet of the peers already known + * @private + * @returns {void} + */ + _maybeConnectPeerStorePeers () { + if (!this._config.peerDiscovery.autoDial) { + return + } + + const minPeers = this._options.connectionManager.minPeers || 0 + + // TODO: this should be ordered by score on PeerStoreV2 + this.peerStore.peers.forEach(async (peer) => { + this.emit('peer:discovery', peer.id) + + if (minPeers > this.connectionManager.size && !this.connectionManager.get(peer.id)) { + log('connecting to previously discovered peer %s', peer.id.toB58String()) + try { + await this.dialer.connectToPeer(peer.id) + } catch (err) { + log.error('could not connect to previously discovered peer', err) + } + } + }) + } + /** * Will dial to the given `peerId` if the current number of * connected peers is less than the configured `ConnectionManager` From 3fcc2d610a73e5a975b3ab7691bdbe6c835bcc3e Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 9 Jul 2020 15:29:46 +0200 Subject: [PATCH 2/9] feat: connection manager should proactively connect to peers from peerStore --- src/connection-manager/index.js | 75 +++++++++++--- src/index.js | 38 ++----- test/connection-manager/index.node.js | 144 ++++++++++++++++++++++++++ test/peer-discovery/index.spec.js | 7 +- 4 files changed, 215 insertions(+), 49 deletions(-) diff --git a/src/connection-manager/index.js b/src/connection-manager/index.js index e0e64e7cdd..f08ee579a4 100644 --- a/src/connection-manager/index.js +++ b/src/connection-manager/index.js @@ -1,9 +1,12 @@ 'use strict' +const debug = require('debug') +const log = debug('libp2p:connection-manager') +log.error = debug('libp2p:connection-manager:error') + const errcode = require('err-code') const mergeOptions = require('merge-options') const LatencyMonitor = require('./latency-monitor') -const debug = require('debug')('libp2p:connection-manager') const retimer = require('retimer') const { EventEmitter } = require('events') @@ -57,7 +60,7 @@ class ConnectionManager extends EventEmitter { throw errcode(new Error('Connection Manager maxConnections must be greater than minConnections'), ERR_INVALID_PARAMETERS) } - debug('options: %j', this._options) + log('options: %j', this._options) this._libp2p = libp2p @@ -101,7 +104,8 @@ class ConnectionManager extends EventEmitter { }) this._onLatencyMeasure = this._onLatencyMeasure.bind(this) this._latencyMonitor.on('data', this._onLatencyMeasure) - debug('started') + this._maybeConnectN() + log('started') } /** @@ -113,7 +117,7 @@ class ConnectionManager extends EventEmitter { this._latencyMonitor && this._latencyMonitor.removeListener('data', this._onLatencyMeasure) await this._close() - debug('stopped') + log('stopped') } /** @@ -157,12 +161,12 @@ class ConnectionManager extends EventEmitter { _checkMetrics () { const movingAverages = this._libp2p.metrics.global.movingAverages const received = movingAverages.dataReceived[this._options.movingAverageInterval].movingAverage() - this._checkLimit('maxReceivedData', received) + this._checkMaxLimit('maxReceivedData', received) const sent = movingAverages.dataSent[this._options.movingAverageInterval].movingAverage() - this._checkLimit('maxSentData', sent) + this._checkMaxLimit('maxSentData', sent) const total = received + sent - this._checkLimit('maxData', total) - debug('metrics update', total) + this._checkMaxLimit('maxData', total) + log('metrics update', total) this._timer.reschedule(this._options.pollInterval) } @@ -188,7 +192,7 @@ class ConnectionManager extends EventEmitter { this._peerValues.set(peerIdStr, this._options.defaultPeerValue) } - this._checkLimit('maxConnections', this.size) + this._checkMaxLimit('maxConnections', this.size) } /** @@ -206,6 +210,7 @@ class ConnectionManager extends EventEmitter { this.connections.delete(peerId) this._peerValues.delete(connection.remotePeer.toB58String()) this.emit('peer:disconnect', connection) + this._maybeConnectN() } } @@ -248,7 +253,7 @@ class ConnectionManager extends EventEmitter { * @param {*} summary The LatencyMonitor summary */ _onLatencyMeasure (summary) { - this._checkLimit('maxEventLoopDelay', summary.avgMs) + this._checkMaxLimit('maxEventLoopDelay', summary.avgMs) } /** @@ -257,15 +262,53 @@ class ConnectionManager extends EventEmitter { * @param {string} name The name of the field to check limits for * @param {number} value The current value of the field */ - _checkLimit (name, value) { + _checkMaxLimit (name, value) { const limit = this._options[name] - debug('checking limit of %s. current value: %d of %d', name, value, limit) + log('checking limit of %s. current value: %d of %d', name, value, limit) if (value > limit) { - debug('%s: limit exceeded: %s, %d', this._peerId, name, value) + log('%s: limit exceeded: %s, %d', this._peerId, name, value) this._maybeDisconnectOne() } } + /** + * Proactively tries to connect to known peers stored in the PeerStore. + * It will keep the number of connections below the upper limit and sort + * the peers to connect based on wether we know their keys and protocols. + * @async + * @private + */ + async _maybeConnectN () { + const minConnections = this._options.minConnections + + // Already has enough connections + if (this.size >= minConnections) { + return + } + + // Sort peers on wether we know protocols of public keys for them + const peers = Array.from(this._libp2p.peerStore.peers.values()) + .sort((a, b) => { + if (b.protocols && b.protocols.length && (!a.protocols || !a.protocols.length)) { + return 1 + } else if (b.id.pubKey && !a.id.pubKey) { + return 1 + } + return -1 + }) + + for (let i = 0; i < peers.length && this.size < minConnections; i++) { + if (!this.get(peers[i].id)) { + log('connecting to a peerStore stored peer %s', peers[i].id.toB58String()) + try { + await this._libp2p.dialer.connectToPeer(peers[i].id) + } catch (err) { + log.error('could not connect to peerStore stored peer', err) + } + } + } + } + /** * If we have more connections than our maximum, close a connection * to the lowest valued peer. @@ -274,12 +317,12 @@ class ConnectionManager extends EventEmitter { _maybeDisconnectOne () { if (this._options.minConnections < this.connections.size) { const peerValues = Array.from(this._peerValues).sort(byPeerValue) - debug('%s: sorted peer values: %j', this._peerId, peerValues) + log('%s: sorted peer values: %j', this._peerId, peerValues) const disconnectPeer = peerValues[0] if (disconnectPeer) { const peerId = disconnectPeer[0] - debug('%s: lowest value peer is %s', this._peerId, peerId) - debug('%s: closing a connection to %j', this._peerId, peerId) + log('%s: lowest value peer is %s', this._peerId, peerId) + log('%s: closing a connection to %j', this._peerId, peerId) for (const connections of this.connections.values()) { if (connections[0].remotePeer.toB58String() === peerId) { connections[0].close() diff --git a/src/index.js b/src/index.js index 99ca1b6e4d..7bf3705a0a 100644 --- a/src/index.js +++ b/src/index.js @@ -459,15 +459,18 @@ class Libp2p extends EventEmitter { async _onDidStart () { this._isStarted = true - this.connectionManager.start() - this.peerStore.on('peer', peerId => { this.emit('peer:discovery', peerId) this._maybeConnect(peerId) }) - // Once we start, emit and dial any peers we may have already discovered - this._maybeConnectPeerStorePeers() + // Once we start, emit any peers we may have already discovered + // TODO: this should be removed, as we already discovered these peers in the past + for (const peer of this.peerStore.peers.values()) { + this.emit('peer:discovery', peer.id) + } + + this.connectionManager.start() // Peer discovery await this._setupPeerDiscovery() @@ -489,33 +492,6 @@ class Libp2p extends EventEmitter { peer.protocols && this.peerStore.protoBook.set(peer.id, peer.protocols) } - /** - * Will dial a substet of the peers already known - * @private - * @returns {void} - */ - _maybeConnectPeerStorePeers () { - if (!this._config.peerDiscovery.autoDial) { - return - } - - const minPeers = this._options.connectionManager.minPeers || 0 - - // TODO: this should be ordered by score on PeerStoreV2 - this.peerStore.peers.forEach(async (peer) => { - this.emit('peer:discovery', peer.id) - - if (minPeers > this.connectionManager.size && !this.connectionManager.get(peer.id)) { - log('connecting to previously discovered peer %s', peer.id.toB58String()) - try { - await this.dialer.connectToPeer(peer.id) - } catch (err) { - log.error('could not connect to previously discovered peer', err) - } - } - }) - } - /** * Will dial to the given `peerId` if the current number of * connected peers is less than the configured `ConnectionManager` diff --git a/test/connection-manager/index.node.js b/test/connection-manager/index.node.js index 3a3d000297..af8211f7c9 100644 --- a/test/connection-manager/index.node.js +++ b/test/connection-manager/index.node.js @@ -7,6 +7,9 @@ chai.use(require('chai-as-promised')) const { expect } = chai const sinon = require('sinon') +const delay = require('delay') +const pWaitFor = require('p-wait-for') + const peerUtils = require('../utils/creators/peer') const mockConnection = require('../utils/mockConnection') const baseOptions = require('../utils/base-options.browser') @@ -112,4 +115,145 @@ describe('libp2p.connections', () => { await libp2p.stop() await remoteLibp2p.stop() }) + + describe('proactive connections', () => { + let nodes = [] + + beforeEach(async () => { + nodes = await peerUtils.createPeer({ + number: 2, + config: { + addresses: { + listen: ['/ip4/127.0.0.1/tcp/0/ws'] + } + } + }) + }) + + afterEach(async () => { + await Promise.all(nodes.map((node) => node.stop())) + sinon.reset() + }) + + it('should connect to all the peers stored in the PeerStore, if their number is below minConnections', async () => { + const [libp2p] = await peerUtils.createPeer({ + fixture: false, + started: false, + config: { + addresses: { + listen: ['/ip4/127.0.0.1/tcp/0/ws'] + }, + connectionManager: { + minConnections: 3 + } + } + }) + + // Populate PeerStore before starting + libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + + await libp2p.start() + + // Wait for peers to connect + await pWaitFor(() => libp2p.connectionManager.size === 2) + + await libp2p.stop() + }) + + it('should connect to all the peers stored in the PeerStore until reaching the minConnections', async () => { + const minConnections = 1 + const [libp2p] = await peerUtils.createPeer({ + fixture: false, + started: false, + config: { + addresses: { + listen: ['/ip4/127.0.0.1/tcp/0/ws'] + }, + connectionManager: { + minConnections + } + } + }) + + // Populate PeerStore before starting + libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + + await libp2p.start() + + // Wait for peer to connect + await pWaitFor(() => libp2p.connectionManager.size === minConnections) + + // Wait more time to guarantee no other connection happened + await delay(200) + expect(libp2p.connectionManager.size).to.eql(minConnections) + + await libp2p.stop() + }) + + it('should connect to all the peers stored in the PeerStore until reaching the minConnections sorted', async () => { + const minConnections = 1 + const [libp2p] = await peerUtils.createPeer({ + fixture: false, + started: false, + config: { + addresses: { + listen: ['/ip4/127.0.0.1/tcp/0/ws'] + }, + connectionManager: { + minConnections + } + } + }) + + // Populate PeerStore before starting + libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + libp2p.peerStore.protoBook.set(nodes[1].peerId, ['/protocol-min-conns']) + + await libp2p.start() + + // Wait for peer to connect + await pWaitFor(() => libp2p.connectionManager.size === minConnections) + + // Should have connected to the peer with protocols + expect(libp2p.connectionManager.get(nodes[0].peerId)).to.not.exist() + expect(libp2p.connectionManager.get(nodes[1].peerId)).to.exist() + + await libp2p.stop() + }) + + it('should connect to peers in the PeerStore when a peer disconnected', async () => { + const minConnections = 1 + const [libp2p] = await peerUtils.createPeer({ + fixture: false, + config: { + addresses: { + listen: ['/ip4/127.0.0.1/tcp/0/ws'] + }, + connectionManager: { + minConnections + } + } + }) + + // Populate PeerStore after starting (discovery) + libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + libp2p.peerStore.protoBook.set(nodes[1].peerId, ['/protocol-min-conns']) + + // Wait for peer to connect + const conn = await libp2p.dial(nodes[0].peerId) + expect(libp2p.connectionManager.get(nodes[0].peerId)).to.exist() + expect(libp2p.connectionManager.get(nodes[1].peerId)).to.not.exist() + + await conn.close() + await pWaitFor(() => libp2p.connectionManager.size === minConnections) + expect(libp2p.connectionManager.get(nodes[0].peerId)).to.not.exist() + expect(libp2p.connectionManager.get(nodes[1].peerId)).to.exist() + + await libp2p.stop() + }) + }) }) diff --git a/test/peer-discovery/index.spec.js b/test/peer-discovery/index.spec.js index b6e355eed1..fcada30ca9 100644 --- a/test/peer-discovery/index.spec.js +++ b/test/peer-discovery/index.spec.js @@ -31,10 +31,13 @@ describe('peer discovery', () => { sinon.reset() }) - it('should dial know peers on startup', async () => { + it('should dial know peers on startup below the minConnections watermark', async () => { libp2p = new Libp2p({ ...baseOptions, - peerId + peerId, + connectionManager: { + minConnections: 2 + } }) libp2p.peerStore.addressBook.set(remotePeerId, [multiaddr('/ip4/165.1.1.1/tcp/80')]) From 4afb8019e5cefe4a643a024b426bc55011a26251 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 9 Jul 2020 15:43:21 +0200 Subject: [PATCH 3/9] chore: increase bundle size --- .aegir.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.aegir.js b/.aegir.js index 18d6236a58..df4a41f5f8 100644 --- a/.aegir.js +++ b/.aegir.js @@ -45,7 +45,7 @@ const after = async () => { } module.exports = { - bundlesize: { maxSize: '200kB' }, + bundlesize: { maxSize: '202kB' }, hooks: { pre: before, post: after From 6ac33285871f623353e92013f893758d8a789548 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 12:15:27 +0200 Subject: [PATCH 4/9] fix: do connMgr proactive dial on an interval --- src/connection-manager/index.js | 23 +++++++++++++++++++++-- test/connection-manager/index.node.js | 5 ++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/connection-manager/index.js b/src/connection-manager/index.js index f08ee579a4..3d2978beb2 100644 --- a/src/connection-manager/index.js +++ b/src/connection-manager/index.js @@ -25,6 +25,7 @@ const defaultOptions = { maxReceivedData: Infinity, maxEventLoopDelay: Infinity, pollInterval: 2000, + maybeConnectInterval: 10000, movingAverageInterval: 60000, defaultPeerValue: 1 } @@ -48,6 +49,7 @@ class ConnectionManager extends EventEmitter { * @param {Number} options.pollInterval How often, in milliseconds, metrics and latency should be checked. Default=2000 * @param {Number} options.movingAverageInterval How often, in milliseconds, to compute averages. Default=60000 * @param {Number} options.defaultPeerValue The value of the peer. Default=1 + * @param {Number} options.maybeConnectInterval How often, in milliseconds, it should preemptively guarantee connections are above the low watermark. Default=10000 */ constructor (libp2p, options) { super() @@ -76,8 +78,11 @@ class ConnectionManager extends EventEmitter { */ this.connections = new Map() + this._started = false this._timer = null + this._maybeConnectTimeout = null this._checkMetrics = this._checkMetrics.bind(this) + this._maybeConnectN = this._maybeConnectN.bind(this) } /** @@ -104,8 +109,11 @@ class ConnectionManager extends EventEmitter { }) this._onLatencyMeasure = this._onLatencyMeasure.bind(this) this._latencyMonitor.on('data', this._onLatencyMeasure) - this._maybeConnectN() + + this._started = true log('started') + + this._maybeConnectN() } /** @@ -113,9 +121,12 @@ class ConnectionManager extends EventEmitter { * @async */ async stop () { + clearTimeout(this._maybeConnectTimeout) + this._timer && this._timer.clear() this._latencyMonitor && this._latencyMonitor.removeListener('data', this._onLatencyMeasure) + this._started = false await this._close() log('stopped') } @@ -210,7 +221,6 @@ class ConnectionManager extends EventEmitter { this.connections.delete(peerId) this._peerValues.delete(connection.remotePeer.toB58String()) this.emit('peer:disconnect', connection) - this._maybeConnectN() } } @@ -279,6 +289,8 @@ class ConnectionManager extends EventEmitter { * @private */ async _maybeConnectN () { + this._isTryingToConnect = true + const minConnections = this._options.minConnections // Already has enough connections @@ -302,11 +314,18 @@ class ConnectionManager extends EventEmitter { log('connecting to a peerStore stored peer %s', peers[i].id.toB58String()) try { await this._libp2p.dialer.connectToPeer(peers[i].id) + + // Connection Manager was stopped + if (!this._started) { + return + } } catch (err) { log.error('could not connect to peerStore stored peer', err) } } } + + this._maybeConnectTimeout = setTimeout(this._maybeConnectN, this._options.maybeConnectInterval) } /** diff --git a/test/connection-manager/index.node.js b/test/connection-manager/index.node.js index af8211f7c9..17e4bc5712 100644 --- a/test/connection-manager/index.node.js +++ b/test/connection-manager/index.node.js @@ -226,6 +226,8 @@ describe('libp2p.connections', () => { it('should connect to peers in the PeerStore when a peer disconnected', async () => { const minConnections = 1 + const maybeConnectInterval = 1000 + const [libp2p] = await peerUtils.createPeer({ fixture: false, config: { @@ -233,7 +235,8 @@ describe('libp2p.connections', () => { listen: ['/ip4/127.0.0.1/tcp/0/ws'] }, connectionManager: { - minConnections + minConnections, + maybeConnectInterval } } }) From 9156e293878e4aba0ffc029ebd02fa23b2254643 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 12:48:57 +0200 Subject: [PATCH 5/9] chore: address review --- src/connection-manager/index.js | 22 +++++++++++----------- src/index.js | 5 ++++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/connection-manager/index.js b/src/connection-manager/index.js index 3d2978beb2..a3d6560033 100644 --- a/src/connection-manager/index.js +++ b/src/connection-manager/index.js @@ -25,7 +25,7 @@ const defaultOptions = { maxReceivedData: Infinity, maxEventLoopDelay: Infinity, pollInterval: 2000, - maybeConnectInterval: 10000, + autoDialInterval: 10000, movingAverageInterval: 60000, defaultPeerValue: 1 } @@ -49,7 +49,8 @@ class ConnectionManager extends EventEmitter { * @param {Number} options.pollInterval How often, in milliseconds, metrics and latency should be checked. Default=2000 * @param {Number} options.movingAverageInterval How often, in milliseconds, to compute averages. Default=60000 * @param {Number} options.defaultPeerValue The value of the peer. Default=1 - * @param {Number} options.maybeConnectInterval How often, in milliseconds, it should preemptively guarantee connections are above the low watermark. Default=10000 + * @param {boolean} options.autoDial Should preemptively guarantee connections are above the low watermark. Default=true + * @param {Number} options.autoDialInterval How often, in milliseconds, it should preemptively guarantee connections are above the low watermark. Default=10000 */ constructor (libp2p, options) { super() @@ -80,9 +81,9 @@ class ConnectionManager extends EventEmitter { this._started = false this._timer = null - this._maybeConnectTimeout = null + this._autoDialTimeout = null this._checkMetrics = this._checkMetrics.bind(this) - this._maybeConnectN = this._maybeConnectN.bind(this) + this._autoDial = this._autoDial.bind(this) } /** @@ -113,7 +114,7 @@ class ConnectionManager extends EventEmitter { this._started = true log('started') - this._maybeConnectN() + this._options.autoDial && this._autoDial() } /** @@ -121,8 +122,7 @@ class ConnectionManager extends EventEmitter { * @async */ async stop () { - clearTimeout(this._maybeConnectTimeout) - + this._autoDialTimeout && this._autoDialTimeout.clear() this._timer && this._timer.clear() this._latencyMonitor && this._latencyMonitor.removeListener('data', this._onLatencyMeasure) @@ -288,13 +288,13 @@ class ConnectionManager extends EventEmitter { * @async * @private */ - async _maybeConnectN () { - this._isTryingToConnect = true - + async _autoDial () { const minConnections = this._options.minConnections // Already has enough connections if (this.size >= minConnections) { + this._autoDialTimeout = retimer(this._autoDial, this._options.autoDialInterval) + return } @@ -325,7 +325,7 @@ class ConnectionManager extends EventEmitter { } } - this._maybeConnectTimeout = setTimeout(this._maybeConnectN, this._options.maybeConnectInterval) + this._autoDialTimeout = retimer(this._autoDial, this._options.autoDialInterval) } /** diff --git a/src/index.js b/src/index.js index 7bf3705a0a..23ba2830a4 100644 --- a/src/index.js +++ b/src/index.js @@ -65,7 +65,10 @@ class Libp2p extends EventEmitter { this._discovery = new Map() // Discovery service instances/references // Create the Connection Manager - this.connectionManager = new ConnectionManager(this, this._options.connectionManager) + this.connectionManager = new ConnectionManager(this, { + autoDial: this._config.peerDiscovery.autoDial, + ...this._options.connectionManager + }) // Create Metrics if (this._options.metrics.enabled) { From 2985a5f7db34e727b1cf5621a093b9bed7c5694a Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 13:02:39 +0200 Subject: [PATCH 6/9] chore: use retimer reschedule --- src/connection-manager/index.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/connection-manager/index.js b/src/connection-manager/index.js index a3d6560033..969877d8fd 100644 --- a/src/connection-manager/index.js +++ b/src/connection-manager/index.js @@ -291,10 +291,17 @@ class ConnectionManager extends EventEmitter { async _autoDial () { const minConnections = this._options.minConnections + const recursiveTimeoutTrigger = () => { + if (this._autoDialTimeout) { + this._autoDialTimeout.reschedule(this._options.autoDialInterval) + } else { + this._autoDialTimeout = retimer(this._autoDial, this._options.autoDialInterval) + } + } + // Already has enough connections if (this.size >= minConnections) { - this._autoDialTimeout = retimer(this._autoDial, this._options.autoDialInterval) - + recursiveTimeoutTrigger() return } @@ -325,7 +332,7 @@ class ConnectionManager extends EventEmitter { } } - this._autoDialTimeout = retimer(this._autoDial, this._options.autoDialInterval) + recursiveTimeoutTrigger() } /** From 4ea88384f408f896314a86959ee26365511b0969 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 14:29:03 +0200 Subject: [PATCH 7/9] chore: address review --- test/connection-manager/index.node.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/connection-manager/index.node.js b/test/connection-manager/index.node.js index 17e4bc5712..c5c755caa3 100644 --- a/test/connection-manager/index.node.js +++ b/test/connection-manager/index.node.js @@ -226,7 +226,7 @@ describe('libp2p.connections', () => { it('should connect to peers in the PeerStore when a peer disconnected', async () => { const minConnections = 1 - const maybeConnectInterval = 1000 + const autoDialInterval = 1000 const [libp2p] = await peerUtils.createPeer({ fixture: false, @@ -236,7 +236,7 @@ describe('libp2p.connections', () => { }, connectionManager: { minConnections, - maybeConnectInterval + autoDialInterval } } }) From c92d2ea464c14214a512f4071ab856a95e75e349 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 15:36:08 +0200 Subject: [PATCH 8/9] fix: use minConnections in default config --- src/config.js | 2 +- src/index.js | 3 +++ test/connection-manager/index.node.js | 12 ++++++------ test/connection-manager/index.spec.js | 6 ++++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/config.js b/src/config.js index f586c6dc87..7e3d630efe 100644 --- a/src/config.js +++ b/src/config.js @@ -12,7 +12,7 @@ const DefaultConfig = { noAnnounce: [] }, connectionManager: { - minPeers: 25 + minConnections: 25 }, transportManager: { faultTolerance: FaultTolerance.FATAL_ALL diff --git a/src/index.js b/src/index.js index 23ba2830a4..5d210ca75e 100644 --- a/src/index.js +++ b/src/index.js @@ -65,6 +65,9 @@ class Libp2p extends EventEmitter { this._discovery = new Map() // Discovery service instances/references // Create the Connection Manager + if (this._options.connectionManager.minPeers) { // Remove in 0.29 + this._options.connectionManager.minConnections = this._options.connectionManager.minPeers + } this.connectionManager = new ConnectionManager(this, { autoDial: this._config.peerDiscovery.autoDial, ...this._options.connectionManager diff --git a/test/connection-manager/index.node.js b/test/connection-manager/index.node.js index c5c755caa3..efa09d7cc7 100644 --- a/test/connection-manager/index.node.js +++ b/test/connection-manager/index.node.js @@ -243,18 +243,18 @@ describe('libp2p.connections', () => { // Populate PeerStore after starting (discovery) libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) - libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) - libp2p.peerStore.protoBook.set(nodes[1].peerId, ['/protocol-min-conns']) // Wait for peer to connect const conn = await libp2p.dial(nodes[0].peerId) expect(libp2p.connectionManager.get(nodes[0].peerId)).to.exist() - expect(libp2p.connectionManager.get(nodes[1].peerId)).to.not.exist() await conn.close() - await pWaitFor(() => libp2p.connectionManager.size === minConnections) - expect(libp2p.connectionManager.get(nodes[0].peerId)).to.not.exist() - expect(libp2p.connectionManager.get(nodes[1].peerId)).to.exist() + // Closed + await pWaitFor(() => libp2p.connectionManager.size === 0) + // Connected + await pWaitFor(() => libp2p.connectionManager.size === 1) + + expect(libp2p.connectionManager.get(nodes[0].peerId)).to.exist() await libp2p.stop() }) diff --git a/test/connection-manager/index.spec.js b/test/connection-manager/index.spec.js index 8ffb0ee19c..caf6becb8a 100644 --- a/test/connection-manager/index.spec.js +++ b/test/connection-manager/index.spec.js @@ -58,7 +58,8 @@ describe('Connection Manager', () => { config: { modules: baseOptions.modules, connectionManager: { - maxConnections: max + maxConnections: max, + minConnections: 2 } }, started: false @@ -96,7 +97,8 @@ describe('Connection Manager', () => { config: { modules: baseOptions.modules, connectionManager: { - maxConnections: max + maxConnections: max, + minConnections: 0 } }, started: false From 43a52a6f677a7deff18461eebac2ac409f0792f2 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 14 Jul 2020 15:51:56 +0200 Subject: [PATCH 9/9] chore: minPeers to minConnections everywhere --- doc/CONFIGURATION.md | 2 +- doc/GETTING_STARTED.md | 2 +- src/index.js | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 6a0bb44e02..17174bb1ef 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -270,7 +270,7 @@ const node = await Libp2p.create({ }, config: { peerDiscovery: { - autoDial: true, // Auto connect to discovered peers (limited by ConnectionManager minPeers) + autoDial: true, // Auto connect to discovered peers (limited by ConnectionManager minConnections) // The `tag` property will be searched when creating the instance of your Peer Discovery service. // The associated object, will be passed to the service when it is instantiated. [MulticastDNS.tag]: { diff --git a/doc/GETTING_STARTED.md b/doc/GETTING_STARTED.md index 216110c4a6..8b50efd7e0 100644 --- a/doc/GETTING_STARTED.md +++ b/doc/GETTING_STARTED.md @@ -217,7 +217,7 @@ const node = await Libp2p.create({ }, config: { peerDiscovery: { - autoDial: true, // Auto connect to discovered peers (limited by ConnectionManager minPeers) + autoDial: true, // Auto connect to discovered peers (limited by ConnectionManager minConnections) // The `tag` property will be searched when creating the instance of your Peer Discovery service. // The associated object, will be passed to the service when it is instantiated. [Bootstrap.tag]: { diff --git a/src/index.js b/src/index.js index 5d210ca75e..0fc2ce55d5 100644 --- a/src/index.js +++ b/src/index.js @@ -501,15 +501,15 @@ class Libp2p extends EventEmitter { /** * Will dial to the given `peerId` if the current number of * connected peers is less than the configured `ConnectionManager` - * minPeers. + * minConnections. * @private * @param {PeerId} peerId */ async _maybeConnect (peerId) { // If auto dialing is on and we have no connection to the peer, check if we should dial if (this._config.peerDiscovery.autoDial === true && !this.connectionManager.get(peerId)) { - const minPeers = this._options.connectionManager.minPeers || 0 - if (minPeers > this.connectionManager.size) { + const minConnections = this._options.connectionManager.minConnections || 0 + if (minConnections > this.connectionManager.size) { log('connecting to discovered peer %s', peerId.toB58String()) try { await this.dialer.connectToPeer(peerId)