fix: do not auto-dial peers in the dial queue

src/connection-manager/auto-dial.ts

@@ -1,7 +1,7 @@
 import { logger } from '@libp2p/logger'
 import type { PeerStore } from '@libp2p/interface-peer-store'
 import type { ConnectionManager } from '@libp2p/interface-connection-manager'
-import { PeerMap } from '@libp2p/peer-collections'
+import { PeerMap, PeerSet } from '@libp2p/peer-collections'
 import PQueue from 'p-queue'
 import { AUTO_DIAL_CONCURRENCY, AUTO_DIAL_INTERVAL, AUTO_DIAL_PRIORITY, MIN_CONNECTIONS } from './constants.js'
 import type { Startable } from '@libp2p/interfaces/startable'
@@ -103,6 +103,12 @@ export class AutoDial implements Startable {
 
     const connections = this.connectionManager.getConnectionsMap()
     const numConnections = connections.size
+    const dialQueue = new PeerSet(
+      // @ts-expect-error boolean filter removes falsy peer IDs
+      this.connectionManager.getDialQueue()
+        .map(queue => queue.peerId)
+        .filter(Boolean)
+    )
 
     // Already has enough connections
     if (numConnections >= this.minConnections) {
@@ -127,6 +133,11 @@ export class AutoDial implements Startable {
         return false
       }
 
+      // remove peers we are already dialling
+      if (dialQueue.has(peer.id)) {
+        return false
+      }
+
       return true
     })
 

src/connection-manager/dial-queue.ts

@@ -232,7 +232,7 @@ export class DialQueue {
         signal.clear()
       })
       .catch(err => {
-        log.error('dial failed to %s', addrsToDial.map(({ multiaddr }) => multiaddr.toString()).join(', '), err)
+        log.error('dial failed to %s', pendingDial.multiaddrs.map(ma => ma.toString()).join(', '), err)
 
         // Error is a timeout
         if (signal.aborted) {
@@ -373,7 +373,14 @@ export class DialQueue {
       gatedAdrs.push(addr)
     }
 
-    return gatedAdrs.sort(this.addressSorter)
+    const sortedGatedAddrs = gatedAdrs.sort(this.addressSorter)
+
+    // make sure we actually have some addresses to dial
+    if (sortedGatedAddrs.length === 0) {
+      throw new CodeError('The connection gater denied all addresses in the dial request', codes.ERR_NO_VALID_ADDRESSES)
+    }
+
+    return sortedGatedAddrs
   }
 
   private async performDial (pendingDial: PendingDial, options: DialOptions = {}): Promise<Connection> {

test/connection-manager/auto-dial.spec.ts

@@ -11,6 +11,7 @@ import type { PeerStore, Peer } from '@libp2p/interface-peer-store'
 import { multiaddr } from '@multiformats/multiaddr'
 import { EventEmitter } from '@libp2p/interfaces/events'
 import { PeerMap } from '@libp2p/peer-collections'
+import type { Connection } from '@libp2p/interface-connection'
 
 describe('auto-dial', () => {
   let autoDialler: AutoDial
@@ -49,8 +50,10 @@ describe('auto-dial', () => {
       peerWithAddress, peerWithoutAddress
     ]))
 
-    const connectionManager = stubInterface<ConnectionManager>()
-    connectionManager.getConnectionsMap.returns(new PeerMap())
+    const connectionManager = stubInterface<ConnectionManager>({
+      getConnectionsMap: new PeerMap(),
+      getDialQueue: []
+    })
 
     autoDialler = new AutoDial({
       peerStore,
@@ -69,4 +72,115 @@ describe('auto-dial', () => {
     expect(connectionManager.openConnection.calledWith(peerWithAddress.id)).to.be.true()
     expect(connectionManager.openConnection.calledWith(peerWithoutAddress.id)).to.be.false()
   })
+
+  it('should not dial connected peers', async () => {
+    const connectedPeer: Peer = {
+      id: await createEd25519PeerId(),
+      protocols: [],
+      addresses: [{
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4001'),
+        isCertified: true
+      }],
+      metadata: new Map(),
+      tags: new Map()
+    }
+    const unConnectedPeer: Peer = {
+      id: await createEd25519PeerId(),
+      protocols: [],
+      addresses: [{
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4002'),
+        isCertified: true
+      }],
+      metadata: new Map(),
+      tags: new Map()
+    }
+
+    const peerStore = stubInterface<PeerStore>()
+
+    peerStore.all.returns(Promise.resolve([
+      connectedPeer, unConnectedPeer
+    ]))
+
+    const connectionMap = new PeerMap<Connection[]>()
+    connectionMap.set(connectedPeer.id, [stubInterface<Connection>()])
+
+    const connectionManager = stubInterface<ConnectionManager>({
+      getConnectionsMap: connectionMap,
+      getDialQueue: []
+    })
+
+    autoDialler = new AutoDial({
+      peerStore,
+      connectionManager,
+      events: new EventEmitter()
+    }, {
+      minConnections: 10
+    })
+    autoDialler.start()
+    await autoDialler.autoDial()
+
+    await pWaitFor(() => connectionManager.openConnection.callCount === 1)
+    await delay(1000)
+
+    expect(connectionManager.openConnection.callCount).to.equal(1)
+    expect(connectionManager.openConnection.calledWith(unConnectedPeer.id)).to.be.true()
+    expect(connectionManager.openConnection.calledWith(connectedPeer.id)).to.be.false()
+  })
+
+  it('should not dial peers already in the dial queue', async () => {
+    // peers with protocols are dialled before peers without protocols
+    const peerInDialQueue: Peer = {
+      id: await createEd25519PeerId(),
+      protocols: [],
+      addresses: [{
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4001'),
+        isCertified: true
+      }],
+      metadata: new Map(),
+      tags: new Map()
+    }
+    const peerNotInDialQueue: Peer = {
+      id: await createEd25519PeerId(),
+      protocols: [],
+      addresses: [{
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4002'),
+        isCertified: true
+      }],
+      metadata: new Map(),
+      tags: new Map()
+    }
+
+    const peerStore = stubInterface<PeerStore>()
+
+    peerStore.all.returns(Promise.resolve([
+      peerInDialQueue, peerNotInDialQueue
+    ]))
+
+    const connectionManager = stubInterface<ConnectionManager>({
+      getConnectionsMap: new PeerMap(),
+      getDialQueue: [{
+        id: 'foo',
+        peerId: peerInDialQueue.id,
+        multiaddrs: [],
+        status: 'queued'
+      }]
+    })
+
+    autoDialler = new AutoDial({
+      peerStore,
+      connectionManager,
+      events: new EventEmitter()
+    }, {
+      minConnections: 10
+    })
+    autoDialler.start()
+    await autoDialler.autoDial()
+
+    await pWaitFor(() => connectionManager.openConnection.callCount === 1)
+    await delay(1000)
+
+    expect(connectionManager.openConnection.callCount).to.equal(1)
+    expect(connectionManager.openConnection.calledWith(peerNotInDialQueue.id)).to.be.true()
+    expect(connectionManager.openConnection.calledWith(peerInDialQueue.id)).to.be.false()
+  })
 })