fix: handle dialing /p2p/Qmfoo-style addresses (#3064)

Dialing `/p2p/Qmfoo` address should load addresses from the peer
store and/or query the routing.

Author: achingbrain

packages/libp2p/package.json

@@ -103,7 +103,7 @@
     "@libp2p/utils": "^6.6.0",
     "@multiformats/dns": "^1.0.6",
     "@multiformats/multiaddr": "^12.3.5",
-    "@multiformats/multiaddr-matcher": "^1.6.0",
+    "@multiformats/multiaddr-matcher": "^1.7.0",
     "any-signal": "^4.1.1",
     "datastore-core": "^10.0.2",
     "interface-datastore": "^8.3.1",

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

@@ -385,8 +385,9 @@ export class DialQueue {
         }
       }
 
-      // if we still don't have any addresses for this peer, try a lookup
-      // using the peer routing
+      // if we still don't have any addresses for this peer, or the only
+      // addresses we have are without any routing information (e.g.
+      // `/p2p/Qmfoo`), try a lookup using the peer routing
       if (addrs.length === 0) {
         this.log('looking up multiaddrs for %p in the peer routing', peerId)
 

packages/libp2p/src/get-peer.ts

@@ -1,6 +1,7 @@
 import { InvalidMultiaddrError, InvalidParametersError, isPeerId } from '@libp2p/interface'
 import { peerIdFromString } from '@libp2p/peer-id'
 import { isMultiaddr } from '@multiformats/multiaddr'
+import { PEER_ID } from '@multiformats/multiaddr-matcher'
 import type { PeerId } from '@libp2p/interface'
 import type { Multiaddr } from '@multiformats/multiaddr'
 
@@ -10,25 +11,24 @@ export interface PeerAddress {
 }
 
 /**
- * Extracts a PeerId and/or multiaddr from the passed PeerId or Multiaddr or an array of Multiaddrs
+ * Extracts a PeerId and/or multiaddr from the passed PeerId or Multiaddr or an
+ * array of Multiaddrs
  */
 export function getPeerAddress (peer: PeerId | Multiaddr | Multiaddr[]): PeerAddress {
   if (isPeerId(peer)) {
     return { peerId: peer, multiaddrs: [] }
   }
 
-  if (!Array.isArray(peer)) {
-    peer = [peer]
-  }
+  let multiaddrs = Array.isArray(peer) ? peer : [peer]
 
   let peerId: PeerId | undefined
 
-  if (peer.length > 0) {
-    const peerIdStr = peer[0].getPeerId()
+  if (multiaddrs.length > 0) {
+    const peerIdStr = multiaddrs[0].getPeerId()
     peerId = peerIdStr == null ? undefined : peerIdFromString(peerIdStr)
 
     // ensure PeerId is either not set or is consistent
-    peer.forEach(ma => {
+    multiaddrs.forEach(ma => {
       if (!isMultiaddr(ma)) {
         throw new InvalidMultiaddrError('Invalid multiaddr')
       }
@@ -49,8 +49,14 @@ export function getPeerAddress (peer: PeerId | Multiaddr | Multiaddr[]): PeerAdd
     })
   }
 
+  // ignore any `/p2p/Qmfoo`-style addresses as we will include the peer id in
+  // the returned value of this function
+  multiaddrs = multiaddrs.filter(ma => {
+    return !PEER_ID.exactMatch(ma)
+  })
+
   return {
     peerId,
-    multiaddrs: peer
+    multiaddrs
   }
 }

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

@@ -82,6 +82,33 @@ describe('dial queue', () => {
     await expect(deferredConn.promise).to.eventually.be.undefined()
   })
 
+  it('should load addresses from the peer store when dialing a multiaddr that only contains a peer id', async () => {
+    const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const connection = stubInterface<Connection>()
+    const foundAddress = multiaddr('/ip4/127.0.0.1/tcp/4001')
+    const ma = multiaddr(`/p2p/${peerId}`)
+
+    components.peerStore.get.withArgs(peerId).resolves({
+      id: peerId,
+      addresses: [{
+        multiaddr: foundAddress,
+        isCertified: false
+      }],
+      protocols: [],
+      metadata: new Map(),
+      tags: new Map()
+    })
+
+    components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
+    components.transportManager.dial.withArgs(foundAddress.encapsulate(`/p2p/${peerId}`)).resolves(connection)
+
+    dialer = new DialQueue(components)
+
+    await expect(dialer.dial(ma)).to.eventually.equal(connection)
+
+    expect(components.peerRouting.findPeer).to.have.property('called', false)
+  })
+
   it('should load addresses from the peer routing when peer id is not in the peer store', async () => {
     const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
     const connection = stubInterface<Connection>()
@@ -103,6 +130,28 @@ describe('dial queue', () => {
     await expect(dialer.dial(peerId)).to.eventually.equal(connection)
   })
 
+  it('should load addresses from the peer routing when dialing a multiaddr that only contains a peer id', async () => {
+    const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const connection = stubInterface<Connection>()
+    const foundAddress = multiaddr('/ip4/127.0.0.1/tcp/4001')
+    const ma = multiaddr(`/p2p/${peerId}`)
+
+    components.peerStore.get.withArgs(peerId).rejects(new NotFoundError('Not found'))
+    components.peerRouting.findPeer.withArgs(peerId).resolves({
+      id: peerId,
+      multiaddrs: [
+        foundAddress
+      ]
+    })
+
+    components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
+    components.transportManager.dial.withArgs(foundAddress.encapsulate(`/p2p/${peerId}`)).resolves(connection)
+
+    dialer = new DialQueue(components)
+
+    await expect(dialer.dial(ma)).to.eventually.equal(connection)
+  })
+
   it('should load addresses from the peer routing when none are present in the peer store', async () => {
     const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
     const connection = stubInterface<Connection>()