From c7e923a8125151f9d922912973723ab6a5254840 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Mon, 6 Dec 2021 19:15:20 +0000 Subject: [PATCH] fix: remove peer routing search-for-self The peer routing module starts a recurring process that searches for peers close to our peer id. This makes the DHT module query the network for peers. Thing is the DHT module is already doing this because periodically searching for peers close to us is in the DHT spec so this ends up making redundant queries. This PR removes the recurring task configured by the peer routing module. --- doc/CONFIGURATION.md | 9 +-- package.json | 2 - src/config.js | 15 +--- src/dialer/dial-request.js | 1 - src/index.js | 3 - src/peer-routing.js | 53 ------------ test/dialing/dial-request.spec.js | 1 - test/peer-routing/peer-routing.node.js | 107 ------------------------- test/ts-use/src/main.ts | 7 -- test/utils/mockMultiaddrConn.js | 1 - 10 files changed, 2 insertions(+), 197 deletions(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 7d4523f76f..912e1afa7e 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -415,14 +415,7 @@ const node = await Libp2p.create({ contentRouting: [delegatedContentRouting], peerRouting: [delegatedPeerRouting], }, - peerId, - peerRouting: { // Peer routing configuration - refreshManager: { // Refresh known and connected closest peers - enabled: true, // Should find the closest peers. - interval: 6e5, // Interval for getting the new for closest peers of 10min - bootDelay: 10e3 // Delay for the initial query for closest peers - } - } + peerId }) ``` diff --git a/package.json b/package.json index 16423a60a9..405c58c2a3 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,6 @@ "dependencies": { "@motrix/nat-api": "^0.3.1", "@vascosantos/moving-average": "^1.1.0", - "abort-controller": "^3.0.0", "abortable-iterator": "^3.0.0", "aggregate-error": "^3.1.0", "any-signal": "^2.1.1", @@ -136,7 +135,6 @@ "@chainsafe/libp2p-noise": "^4.0.0", "@nodeutils/defaults-deep": "^1.1.0", "@types/es6-promisify": "^6.0.0", - "@types/node": "^16.0.1", "@types/node-forge": "^0.10.1", "@types/varint": "^6.0.0", "aegir": "^36.0.0", diff --git a/src/config.js b/src/config.js index 9542c70036..4640b2dd9f 100644 --- a/src/config.js +++ b/src/config.js @@ -49,24 +49,11 @@ const DefaultConfig = { persistence: false, threshold: 5 }, - peerRouting: { - refreshManager: { - enabled: true, - interval: 6e5, - bootDelay: 10e3 - } - }, config: { protocolPrefix: 'ipfs', dht: { enabled: false, - kBucketSize: 20, - randomWalk: { - enabled: false, // disabled waiting for https://github.com/libp2p/js-libp2p-kad-dht/issues/86 - queriesPerPeriod: 1, - interval: 300e3, - timeout: 10e3 - } + kBucketSize: 20 }, nat: { enabled: true, diff --git a/src/dialer/dial-request.js b/src/dialer/dial-request.js index 3f6fc18ae1..d912a53d4c 100644 --- a/src/dialer/dial-request.js +++ b/src/dialer/dial-request.js @@ -1,7 +1,6 @@ 'use strict' const errCode = require('err-code') -const AbortController = require('abort-controller').default const { anySignal } = require('any-signal') // @ts-ignore p-fifo does not export types const FIFO = require('p-fifo') diff --git a/src/index.js b/src/index.js index 2b42822312..2161f19d78 100644 --- a/src/index.js +++ b/src/index.js @@ -384,7 +384,6 @@ class Libp2p extends EventEmitter { this._isStarted = false this.relay && this.relay.stop() - this.peerRouting.stop() this._autodialler.stop() await (this._dht && this._dht.stop()) @@ -663,8 +662,6 @@ class Libp2p extends EventEmitter { // Relay this.relay && this.relay.start() - - this.peerRouting.start() } /** diff --git a/src/peer-routing.js b/src/peer-routing.js index 2ffa5cfd74..af2796b3c4 100644 --- a/src/peer-routing.js +++ b/src/peer-routing.js @@ -1,9 +1,5 @@ 'use strict' -const debug = require('debug') -const log = Object.assign(debug('libp2p:peer-routing'), { - error: debug('libp2p:peer-routing:err') -}) const errCode = require('err-code') const { storeAddresses, @@ -15,13 +11,7 @@ const { TimeoutController } = require('timeout-abort-controller') const merge = require('it-merge') const { pipe } = require('it-pipe') const first = require('it-first') -const drain = require('it-drain') const filter = require('it-filter') -const { - setDelayedInterval, - clearDelayedInterval -// @ts-ignore module with no types -} = require('set-delayed-interval') const { DHTPeerRouting } = require('./dht/dht-peer-routing') /** @@ -31,14 +21,7 @@ const { DHTPeerRouting } = require('./dht/dht-peer-routing') */ /** - * @typedef {Object} RefreshManagerOptions - * @property {boolean} [enabled = true] - Whether to enable the Refresh manager - * @property {number} [bootDelay = 6e5] - Boot delay to start the Refresh Manager (in ms) - * @property {number} [interval = 10e3] - Interval between each Refresh Manager run (in ms) - * @property {number} [timeout = 10e3] - How long to let each refresh run (in ms) - * * @typedef {Object} PeerRoutingOptions - * @property {RefreshManagerOptions} [refreshManager] */ class PeerRouting { @@ -56,42 +39,6 @@ class PeerRouting { if (libp2p._dht && libp2p._config.dht.enabled) { this._routers.push(new DHTPeerRouting(libp2p._dht)) } - - this._refreshManagerOptions = libp2p._options.peerRouting.refreshManager - - this._findClosestPeersTask = this._findClosestPeersTask.bind(this) - } - - /** - * Start peer routing service. - */ - start () { - if (!this._routers.length || this._timeoutId || !this._refreshManagerOptions.enabled) { - return - } - - this._timeoutId = setDelayedInterval( - this._findClosestPeersTask, this._refreshManagerOptions.interval, this._refreshManagerOptions.bootDelay - ) - } - - /** - * Recurrent task to find closest peers and add their addresses to the Address Book. - */ - async _findClosestPeersTask () { - try { - // nb getClosestPeers adds the addresses to the address book - await drain(this.getClosestPeers(this._peerId.id, { timeout: this._refreshManagerOptions.timeout || 10e3 })) - } catch (/** @type {any} */ err) { - log.error(err) - } - } - - /** - * Stop peer routing service. - */ - stop () { - clearDelayedInterval(this._timeoutId) } /** diff --git a/test/dialing/dial-request.spec.js b/test/dialing/dial-request.spec.js index 2c72fb7b87..3ec2eba8aa 100644 --- a/test/dialing/dial-request.spec.js +++ b/test/dialing/dial-request.spec.js @@ -5,7 +5,6 @@ const { expect } = require('aegir/utils/chai') const sinon = require('sinon') const { AbortError } = require('libp2p-interfaces/src/transport/errors') -const AbortController = require('abort-controller') const AggregateError = require('aggregate-error') const pDefer = require('p-defer') const delay = require('delay') diff --git a/test/peer-routing/peer-routing.node.js b/test/peer-routing/peer-routing.node.js index fd76ee792d..ac4027ff4b 100644 --- a/test/peer-routing/peer-routing.node.js +++ b/test/peer-routing/peer-routing.node.js @@ -6,9 +6,7 @@ const nock = require('nock') const sinon = require('sinon') const intoStream = require('into-stream') -const delay = require('delay') const pDefer = require('p-defer') -const pWaitFor = require('p-wait-for') const mergeOptions = require('merge-options') const drain = require('it-drain') const all = require('it-all') @@ -453,109 +451,4 @@ describe('peer-routing', () => { expect(peers).to.be.an('array').with.a.lengthOf(1).that.deep.equals(results) }) }) - - describe('peer routing refresh manager service', () => { - let node - let peerIds - - before(async () => { - peerIds = await peerUtils.createPeerId({ number: 2 }) - }) - - afterEach(() => { - sinon.restore() - - return node && node.stop() - }) - - it('should be enabled and start by default', async () => { - const results = [ - { id: peerIds[0], multiaddrs: [new Multiaddr('/ip4/30.0.0.1/tcp/2000')] }, - { id: peerIds[1], multiaddrs: [new Multiaddr('/ip4/32.0.0.1/tcp/2000')] } - ] - - ;[node] = await peerUtils.createPeer({ - config: mergeOptions(routingOptions, { - peerRouting: { - refreshManager: { - bootDelay: 100 - } - } - }), - started: false - }) - - sinon.spy(node.peerStore.addressBook, 'add') - sinon.stub(node._dht, 'getClosestPeers').callsFake(function * () { - yield results[0] - yield results[1] - }) - - await node.start() - - await pWaitFor(() => node._dht.getClosestPeers.callCount === 1) - await pWaitFor(() => node.peerStore.addressBook.add.callCount === results.length) - - const call0 = node.peerStore.addressBook.add.getCall(0) - expect(call0.args[0].equals(results[0].id)) - call0.args[1].forEach((m, index) => { - expect(m.equals(results[0].multiaddrs[index])) - }) - - const call1 = node.peerStore.addressBook.add.getCall(1) - expect(call1.args[0].equals(results[1].id)) - call0.args[1].forEach((m, index) => { - expect(m.equals(results[1].multiaddrs[index])) - }) - }) - - it('should support being disabled', async () => { - [node] = await peerUtils.createPeer({ - config: mergeOptions(routingOptions, { - peerRouting: { - refreshManager: { - bootDelay: 100, - enabled: false - } - } - }), - started: false - }) - - sinon.stub(node._dht, 'getClosestPeers').callsFake(function * () { - yield - throw new Error('should not be called') - }) - - await node.start() - await delay(100) - - expect(node._dht.getClosestPeers.callCount === 0) - }) - - it('should start and run recurrently on interval', async () => { - [node] = await peerUtils.createPeer({ - config: mergeOptions(routingOptions, { - peerRouting: { - refreshManager: { - interval: 500, - bootDelay: 200 - } - } - }), - started: false - }) - - sinon.stub(node._dht, 'getClosestPeers').callsFake(function * () { - yield { id: peerIds[0], multiaddrs: [new Multiaddr('/ip4/30.0.0.1/tcp/2000')] } - }) - - await node.start() - - await delay(300) - expect(node._dht.getClosestPeers.callCount).to.eql(1) - await delay(500) - expect(node._dht.getClosestPeers.callCount).to.eql(2) - }) - }) }) diff --git a/test/ts-use/src/main.ts b/test/ts-use/src/main.ts index d75a79f4ba..a3ff8fbc34 100644 --- a/test/ts-use/src/main.ts +++ b/test/ts-use/src/main.ts @@ -62,13 +62,6 @@ async function main() { contentRouting: [delegatedContentRouting], peerRouting: [delegatedPeerRouting] }, - peerRouting: { - refreshManager: { - enabled: true, - interval: 1000, - bootDelay: 11111 - } - }, dialer: { maxParallelDials: 100, maxDialsPerPeer: 4, diff --git a/test/utils/mockMultiaddrConn.js b/test/utils/mockMultiaddrConn.js index bb0cf6df7c..26482dd349 100644 --- a/test/utils/mockMultiaddrConn.js +++ b/test/utils/mockMultiaddrConn.js @@ -2,7 +2,6 @@ const duplexPair = require('it-pair/duplex') const abortable = require('abortable-iterator') -const AbortController = require('abort-controller') /** * Returns both sides of a mocked MultiaddrConnection