fix: prevent auto-dialer from dialing self (#1104)

Co-authored-by: Robert Kiel <robert.kiel@hoprnet.io>
Co-authored-by: achingbrain <alex@achingbrain.net>

package.json

@@ -100,6 +100,7 @@
     "it-map": "^1.0.4",
     "it-merge": "^1.0.0",
     "it-pipe": "^1.1.0",
+    "it-sort": "^1.0.1",
     "it-take": "^1.0.0",
     "libp2p-crypto": "^0.21.2",
     "libp2p-interfaces": "^4.0.0",

src/connection-manager/auto-dialler.js

@@ -5,6 +5,9 @@ const mergeOptions = require('merge-options')
 // @ts-ignore retimer does not have types
 const retimer = require('retimer')
 const all = require('it-all')
+const { pipe } = require('it-pipe')
+const filter = require('it-filter')
+const sort = require('it-sort')
 
 const log = Object.assign(debug('libp2p:connection-manager:auto-dialler'), {
   error: debug('libp2p:connection-manager:auto-dialler:err')
@@ -90,21 +93,27 @@ class AutoDialler {
     // Sort peers on whether we know protocols of public keys for them
     // TODO: assuming the `peerStore.getPeers()` order is stable this will mean
     // we keep trying to connect to the same peers?
-    const peers = (await all(this._libp2p.peerStore.getPeers()))
-      .sort((a, b) => {
+    const peers = await pipe(
+      this._libp2p.peerStore.getPeers(),
+      (source) => filter(source, (peer) => !peer.id.equals(this._libp2p.peerId)),
+      (source) => sort(source, (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
-      })
+      }),
+      (source) => all(source)
+    )
 
     for (let i = 0; this._running && i < peers.length && this._libp2p.connections.size < minConnections; i++) {
-      if (!this._libp2p.connectionManager.get(peers[i].id)) {
-        log('connecting to a peerStore stored peer %s', peers[i].id.toB58String())
+      const peer = peers[i]
+
+      if (!this._libp2p.connectionManager.get(peer.id)) {
+        log('connecting to a peerStore stored peer %s', peer.id.toB58String())
         try {
-          await this._libp2p.dialer.connectToPeer(peers[i].id)
+          await this._libp2p.dialer.connectToPeer(peer.id)
         } catch (/** @type {any} */ err) {
           log.error('could not connect to peerStore stored peer', err)
         }

test/connection-manager/auto-dialler.spec.js

@@ -0,0 +1,64 @@
+'use strict'
+/* eslint-env mocha */
+
+const { expect } = require('aegir/utils/chai')
+const sinon = require('sinon')
+const AutoDialler = require('../../src/connection-manager/auto-dialler')
+const pWaitFor = require('p-wait-for')
+const PeerId = require('peer-id')
+const delay = require('delay')
+
+describe('Auto-dialler', () => {
+  let autoDialler
+  let libp2p
+  let options
+
+  beforeEach(async () => {
+    libp2p = {}
+    options = {}
+    autoDialler = new AutoDialler(libp2p, options)
+  })
+
+  afterEach(async () => {
+    sinon.restore()
+  })
+
+  it('should not dial self', async () => {
+    // peers with protocols are dialled before peers without protocols
+    const self = {
+      id: await PeerId.create(),
+      protocols: [
+        '/foo/bar'
+      ]
+    }
+    const other = {
+      id: await PeerId.create(),
+      protocols: []
+    }
+
+    autoDialler._options.minConnections = 10
+    libp2p.peerId = self.id
+    libp2p.connections = {
+      size: 1
+    }
+    libp2p.peerStore = {
+      getPeers: sinon.stub().returns([self, other])
+    }
+    libp2p.connectionManager = {
+      get: () => {}
+    }
+    libp2p.dialer = {
+      connectToPeer: sinon.stub().resolves()
+    }
+
+    await autoDialler.start()
+
+    await pWaitFor(() => libp2p.dialer.connectToPeer.callCount === 1)
+    await delay(1000)
+
+    await autoDialler.stop()
+
+    expect(libp2p.dialer.connectToPeer.callCount).to.equal(1)
+    expect(libp2p.dialer.connectToPeer.calledWith(self.id)).to.be.false()
+  })
+})