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

[paschal533]

Problem

The peer:discovery event emits circuit relay multiaddrs without the
target peer ID, making them un-dialable when used directly:

// This fails for WebRTC/circuit-relay peers:
await libp2p.dial(evt.detail.multiaddrs)

// Users were forced to manually re-add the peer ID:
const maddrs = evt.detail.multiaddrs.map(ma => ma.encapsulate(`/p2p/${evt.detail.id}`))
await libp2p.dial(maddrs)

Confirmed regression between js-libp2p 2.7.4 → 2.9.0, introduced in
commit a5a33af (feat: add getInfo function to peerstore).

Root cause

dedupeFilterAndSortAddresses called ma.getPeerId() which returns
the last peer ID in a multiaddr. For a circuit relay address like
/p2p/RELAY_ID/p2p-circuit/p2p/TARGET_ID, this returned TARGET_ID
and stripped it. The function is correct to strip peer IDs from
direct addresses (it's redundant in storage), but must preserve
them in relay addresses since they are needed for routing.

This was silently partially fixed in b8ecade as a side effect of the
multiaddr v13 upgrade (which removed getPeerId()), but no test was
added. Case 1 below was never fixed:

• Case 1: Peers advertise relay addresses via identify as
  /p2p/RELAY/p2p-circuit (no target peer ID). Always stored and
  emitted without the target peer ID.
• Case 2: pubsub-peer-discovery sends /p2p/RELAY/p2p-circuit/p2p/TARGET.
  Was stripped by a5a33af, silently fixed by b8ecade.

Fix

In the peer:update → peer:discovery pipeline, append /p2p/{peerId}
to any circuit relay address that does not already have a target peer ID
after /p2p-circuit. This covers plain relay and WebRTC relay addresses
(/p2p-circuit/webrtc/p2p/TARGET), and works for both cases above.

Tests

• Relay address without peer ID → peer ID appended ✓
• Relay address with peer ID → not duplicated ✓
• WebRTC relay address without peer ID → peer ID appended ✓
• WebRTC relay address with peer ID → not duplicated ✓
• Direct (non-relay) addresses → unchanged ✓
• Mixed relay + direct in same event → each handled correctly ✓
• Regression: dedupeFilterAndSortAddresses preserves target peer ID
  in plain and WebRTC circuit relay addresses ✓
• Regression: dedupeFilterAndSortAddresses still strips peer ID from
  direct addresses (intended behaviour) ✓

Fixes #3239

[Faolain]

This one is also a big one that's been annoying @dozyio if you get a chance to review 🙏

[achingbrain]

Some context here: the peer:discovery event detail is a PeerInfo object - this is an interface that roughly aligns with the similarly named struct in go-libp2p and appears in several APIs/protocols.

This object gets serialized in a few places so doesn't include the PeerId in the multiaddrs as it causes redundant bytes to be written onto the wire.

If dialling the multiaddrs from the PeerInfo object the PeerId would need to be appended to all addresses, not just relay addresses, otherwise libp2p cannot verify that the remote peer is the one you are trying to dial.

Instead you would typically use the PeerId to to dial the peer, and not the addresses in the PeerInfo during the discovery event, e.g.:

await libp2p.dial(evt.detail.id)

This also allows libp2p to select and order multiaddrs based on the current context and possibly apply heuristics - maybe IPv6 support is missing, maybe addresses that have failed recently can be omitted, maybe there are other addresses for the peer in the peerstore already from previous discovery events emitted by other mechanisms, or maybe all addresses fail and a routing lookup can be performed to find new addresses for the peer etc.

[paschal533]

Some context here: the peer:discovery event detail is a PeerInfo object - this is an interface that roughly aligns with the similarly named struct in go-libp2p and appears in several APIs/protocols.

This object gets serialized in a few places so doesn't include the PeerId in the multiaddrs as it causes redundant bytes to be written onto the wire.

If dialling the multiaddrs from the PeerInfo object the PeerId would need to be appended to all addresses, not just relay addresses, otherwise libp2p cannot verify that the remote peer is the one you are trying to dial.

Instead you would typically use the PeerId to to dial the peer, and not the addresses in the PeerInfo during the discovery event, e.g.:

await libp2p.dial(evt.detail.id)

This also allows libp2p to select and order multiaddrs based on the current context and possibly apply heuristics - maybe IPv6 support is missing, maybe addresses that have failed recently can be omitted, maybe there are other addresses for the peer in the peerstore already from previous discovery events emitted by other mechanisms, or maybe all addresses fail and a routing lookup can be performed to find new addresses for the peer etc.

Thank you for the detailed context, @achingbrain That makes sense, I was patching relay addresses specifically because those were failing, but I missed that the real invariant is that all PeerInfo multiaddrs omit the PeerId by design for wire efficiency. I'll update the PR to use this approach.

Btw, should the fix live in the example/documentation that triggered the issue, or is there also defensive code needed inside the library itself to guard against callers who do try to dial the raw PeerInfo multiaddrs directly?