fix: append peer id to dial addresses before filtering

packages/libp2p/src/connection-manager/dial-queue.ts

@@ -348,7 +348,7 @@ export class DialQueue {
     }
 
     // resolve addresses - this can result in a one-to-many translation when dnsaddrs are resolved
-    const resolvedAddresses = (await Promise.all(
+    let resolvedAddresses = (await Promise.all(
       addrs.map(async addr => {
         const result = await resolveMultiaddrs(addr.multiaddr, options)
 
@@ -364,6 +364,29 @@ export class DialQueue {
     ))
       .flat()
 
+    // ensure the peer id is appended to the multiaddr
+    if (peerId != null) {
+      const peerIdMultiaddr = `/p2p/${peerId.toString()}`
+      resolvedAddresses = resolvedAddresses.map(addr => {
+        const lastProto = addr.multiaddr.protos().pop()
+
+        // do not append peer id to path multiaddrs
+        if (lastProto?.path === true) {
+          return addr
+        }
+
+        // append peer id to multiaddr if it is not already present
+        if (addr.multiaddr.getPeerId() == null) {
+          return {
+            multiaddr: addr.multiaddr.encapsulate(peerIdMultiaddr),
+            isCertified: addr.isCertified
+          }
+        }
+
+        return addr
+      })
+    }
+
     const filteredAddrs = resolvedAddresses.filter(addr => {
       // filter out any multiaddrs that we do not have transports for
       if (this.transportManager.transportForMultiaddr(addr.multiaddr) == null) {
@@ -396,7 +419,7 @@ export class DialQueue {
       dedupedAddrs.set(maStr, addr)
     }
 
-    let dedupedMultiaddrs = [...dedupedAddrs.values()]
+    const dedupedMultiaddrs = [...dedupedAddrs.values()]
 
     if (dedupedMultiaddrs.length === 0 || dedupedMultiaddrs.length > this.maxPeerAddrsToDial) {
       log('addresses for %p before filtering', peerId ?? 'unknown peer', resolvedAddresses.map(({ multiaddr }) => multiaddr.toString()))
@@ -413,30 +436,6 @@ export class DialQueue {
       throw new CodeError('dial with more addresses than allowed', codes.ERR_TOO_MANY_ADDRESSES)
     }
 
-    // ensure the peer id is appended to the multiaddr
-    if (peerId != null) {
-      const peerIdMultiaddr = `/p2p/${peerId.toString()}`
-      dedupedMultiaddrs = dedupedMultiaddrs.map(addr => {
-        const addressPeerId = addr.multiaddr.getPeerId()
-        const lastProto = addr.multiaddr.protos().pop()
-
-        // do not append peer id to path multiaddrs
-        if (lastProto?.path === true) {
-          return addr
-        }
-
-        // append peer id to multiaddr if it is not already present
-        if (addressPeerId !== peerId.toString()) {
-          return {
-            multiaddr: addr.multiaddr.encapsulate(peerIdMultiaddr),
-            isCertified: addr.isCertified
-          }
-        }
-
-        return addr
-      })
-    }
-
     const gatedAdrs: Address[] = []
 
     for (const addr of dedupedMultiaddrs) {

packages/libp2p/test/connection-manager/dial-queue.spec.ts

@@ -3,6 +3,7 @@
 import { mockConnection, mockDuplex, mockMultiaddrConnection } from '@libp2p/interface-compliance-tests/mocks'
 import { createEd25519PeerId } from '@libp2p/peer-id-factory'
 import { multiaddr, resolvers } from '@multiformats/multiaddr'
+import { WebRTC } from '@multiformats/multiaddr-matcher'
 import { expect } from 'aegir/chai'
 import delay from 'delay'
 import pDefer from 'p-defer'
@@ -330,4 +331,41 @@
 
     resolvers.delete('dnsaddr')
   })
+
+  it('should dial WebRTC address with peer id appended', async () => {
+    const remotePeer = await createEd25519PeerId()
+    const relayPeer = await createEd25519PeerId()
+    const ma = multiaddr(`/ip4/123.123.123.123/tcp/123/ws/p2p/${relayPeer}/p2p-circuit/webrtc`)
+    const maWithPeer = `${ma}/p2p/${remotePeer}`
+
+    components.transportManager.transportForMultiaddr.callsFake(ma => {
+      if (WebRTC.exactMatch(ma)) {
+        return stubInterface<Transport>()
+      }
+    })
+    components.peerStore.get.withArgs(remotePeer).resolves({
+      id: remotePeer,
+      protocols: [],
+      metadata: new Map(),
+      tags: new Map(),
+      addresses: [{
+        multiaddr: ma,
+        isCertified: true
+      }]
+    })
+
+    const connection = mockConnection(mockMultiaddrConnection(mockDuplex(), remotePeer))
+
+    components.transportManager.dial.callsFake(async (ma, opts = {}) => {
+      if (ma.toString() === maWithPeer) {
+        await delay(100)
+        return connection
+      }
+
+      throw new Error('Could not dial address')
+    })
+
+    dialer = new DialQueue(components)
+    await expect(dialer.dial(remotePeer)).to.eventually.equal(connection)
+  })
 })