fix: append peer id to circuit relay addresses in peer:discovery event

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

@@ -472,4 +472,67 @@ describe('dial queue', () => {
     await expect(dial1).to.eventually.equal(connection)
     await expect(dial2).to.eventually.equal(connection)
   })
+
+  it('should append peer id to circuit relay addresses that are missing it', async () => {
+    const remotePeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    // relay address as stored in the peer store - no destination peer id
+    // (PeerInfo multiaddrs intentionally omit the destination peer id for wire efficiency)
+    const relayAddrWithoutPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit`)
+    const relayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit/p2p/${remotePeer}`)
+    const connection = stubInterface<Connection>({ remotePeer })
+
+    components.peerStore.get.withArgs(remotePeer).resolves({
+      id: remotePeer,
+      addresses: [{ multiaddr: relayAddrWithoutPeerId, isCertified: false }],
+      protocols: [],
+      metadata: new Map(),
+      tags: new Map()
+    })
+
+    components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
+    components.transportManager.dial.callsFake(async (ma) => {
+      if (ma.equals(relayAddrWithPeerId)) {
+        return connection
+      }
+      throw new Error(`unexpected address: ${ma.toString()}`)
+    })
+
+    dialer = new DialQueue(components)
+
+    await expect(dialer.dial(remotePeer)).to.eventually.equal(connection)
+
+    // the transport was called with the full relay address including the destination peer id
+    expect(components.transportManager.dial.calledWith(relayAddrWithPeerId)).to.be.true()
+  })
+
+  it('should not duplicate peer id in circuit relay addresses that already have it', async () => {
+    const remotePeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    // relay address already has the destination peer id (e.g. from pubsub-peer-discovery)
+    const relayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit/p2p/${remotePeer}`)
+    const connection = stubInterface<Connection>({ remotePeer })
+
+    components.peerStore.get.withArgs(remotePeer).resolves({
+      id: remotePeer,
+      addresses: [{ multiaddr: relayAddrWithPeerId, isCertified: false }],
+      protocols: [],
+      metadata: new Map(),
+      tags: new Map()
+    })
+
+    components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
+    components.transportManager.dial.resolves(connection)
+
+    dialer = new DialQueue(components)
+
+    await expect(dialer.dial(remotePeer)).to.eventually.equal(connection)
+
+    // the address passed to the transport must not have a double peer id
+    const dialledAddr = components.transportManager.dial.getCall(0).args[0].toString()
+    expect(dialledAddr).to.equal(relayAddrWithPeerId.toString())
+    expect(dialledAddr).to.not.include(`/p2p/${remotePeer}/p2p/${remotePeer}`)
+  })
 })

packages/peer-store/test/utils/dedupe-addresses.spec.ts

@@ -66,6 +66,56 @@ describe('dedupe-addresses', () => {
     }])
   })
 
+  it('should preserve target peer id in circuit relay addresses', async () => {
+    const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const targetPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    // Address that includes the target peer ID after /p2p-circuit (e.g. from pubsub-peer-discovery)
+    const relayAddr = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/p2p/${targetPeerId}`)
+
+    const result = await dedupeFilterAndSortAddresses(targetPeerId, async () => true, [{
+      multiaddr: relayAddr,
+      isCertified: false
+    }])
+
+    expect(result).to.have.length(1)
+    // The trailing /p2p/TARGET_ID must not be stripped - it is needed for dialling via relay
+    expect(multiaddr(result[0].multiaddr).toString()).to.equal(relayAddr.toString())
+  })
+
+  it('should preserve target peer id in WebRTC circuit relay addresses', async () => {
+    const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const targetPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    // WebRTC browser-to-browser relay address includes /webrtc before the target peer ID
+    const webrtcRelayAddr = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/webrtc/p2p/${targetPeerId}`)
+
+    const result = await dedupeFilterAndSortAddresses(targetPeerId, async () => true, [{
+      multiaddr: webrtcRelayAddr,
+      isCertified: false
+    }])
+
+    expect(result).to.have.length(1)
+    expect(multiaddr(result[0].multiaddr).toString()).to.equal(webrtcRelayAddr.toString())
+  })
+
+  it('should strip peer id from direct addresses', async () => {
+    const targetPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+
+    // Direct address with redundant peer ID appended (common from identify / pubsub-peer-discovery)
+    const directAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/4001/p2p/${targetPeerId}`)
+    const directAddr = multiaddr('/ip4/1.2.3.4/tcp/4001')
+
+    const result = await dedupeFilterAndSortAddresses(targetPeerId, async () => true, [{
+      multiaddr: directAddrWithPeerId,
+      isCertified: false
+    }])
+
+    expect(result).to.have.length(1)
+    // Peer ID is stripped from direct addresses in storage (it is redundant - known from peer store key)
+    expect(multiaddr(result[0].multiaddr).toString()).to.equal(directAddr.toString())
+  })
+
   it('should filter addresses', async () => {
     expect(await dedupeFilterAndSortAddresses(peerId, async () => false, [{
       multiaddr: addr1,