Skip to content

Commit 082308c

Browse files
paschal533paschal533
committed
fix(connection-manager): dial by peer id to correctly resolve relay addresses
1 parent b6ddda6 commit 082308c

File tree

3 files changed

+67
-218
lines changed

3 files changed

+67
-218
lines changed

‎packages/libp2p/src/libp2p.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { defaultLogger } from '@libp2p/logger'
44
import { PeerSet } from '@libp2p/peer-collections'
55
import { peerIdFromString } from '@libp2p/peer-id'
66
import { persistentPeerStore } from '@libp2p/peer-store'
7-
import { CODE_P2P, CODE_P2P_CIRCUIT, isMultiaddr, multiaddr } from '@multiformats/multiaddr'
7+
import { CODE_P2P, isMultiaddr } from '@multiformats/multiaddr'
88
import { MemoryDatastore } from 'datastore-core/memory'
99
import { TypedEventEmitter, setMaxListeners } from 'main-event'
1010
import { concat as uint8ArrayConcat } from 'uint8arrays/concat'
@@ -100,24 +100,9 @@ export class Libp2p<T extends ServiceMap = ServiceMap> extends TypedEventEmitter
100100
components.events.addEventListener('peer:update', evt => {
101101
// if there was no peer previously in the peer store this is a new peer
102102
if (evt.detail.previous == null) {
103-
const id = evt.detail.peer.id
104103
const peerInfo: PeerInfo = {
105-
id,
106-
multiaddrs: evt.detail.peer.addresses.map(a => {
107-
const ma = a.multiaddr
108-
const components = ma.getComponents()
109-
const circuitIdx = components.findIndex(c => c.code === CODE_P2P_CIRCUIT)
110-
111-
// For circuit relay addresses, ensure the target peer ID is appended
112-
// so callers can dial the address directly without having to add it manually.
113-
// Peers often announce relay addresses without their own peer ID (e.g. from
114-
// identify), so we add it here since it is known from the peer store.
115-
if (circuitIdx !== -1 && !components.slice(circuitIdx + 1).some(c => c.code === CODE_P2P)) {
116-
return ma.encapsulate(multiaddr(`/p2p/${id}`))
117-
}
118-
119-
return ma
120-
})
104+
id: evt.detail.peer.id,
105+
multiaddrs: evt.detail.peer.addresses.map(a => a.multiaddr)
121106
}
122107

123108
components.events.safeDispatchEvent('peer:discovery', { detail: peerInfo })

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,4 +472,67 @@ describe('dial queue', () => {
472472
await expect(dial1).to.eventually.equal(connection)
473473
await expect(dial2).to.eventually.equal(connection)
474474
})
475+
476+
it('should append peer id to circuit relay addresses that are missing it', async () => {
477+
const remotePeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
478+
const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
479+
480+
// relay address as stored in the peer store - no destination peer id
481+
// (PeerInfo multiaddrs intentionally omit the destination peer id for wire efficiency)
482+
const relayAddrWithoutPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit`)
483+
const relayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit/p2p/${remotePeer}`)
484+
const connection = stubInterface<Connection>({ remotePeer })
485+
486+
components.peerStore.get.withArgs(remotePeer).resolves({
487+
id: remotePeer,
488+
addresses: [{ multiaddr: relayAddrWithoutPeerId, isCertified: false }],
489+
protocols: [],
490+
metadata: new Map(),
491+
tags: new Map()
492+
})
493+
494+
components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
495+
components.transportManager.dial.callsFake(async (ma) => {
496+
if (ma.equals(relayAddrWithPeerId)) {
497+
return connection
498+
}
499+
throw new Error(`unexpected address: ${ma.toString()}`)
500+
})
501+
502+
dialer = new DialQueue(components)
503+
504+
await expect(dialer.dial(remotePeer)).to.eventually.equal(connection)
505+
506+
// the transport was called with the full relay address including the destination peer id
507+
expect(components.transportManager.dial.calledWith(relayAddrWithPeerId)).to.be.true()
508+
})
509+
510+
it('should not duplicate peer id in circuit relay addresses that already have it', async () => {
511+
const remotePeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
512+
const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
513+
514+
// relay address already has the destination peer id (e.g. from pubsub-peer-discovery)
515+
const relayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeer}/p2p-circuit/p2p/${remotePeer}`)
516+
const connection = stubInterface<Connection>({ remotePeer })
517+
518+
components.peerStore.get.withArgs(remotePeer).resolves({
519+
id: remotePeer,
520+
addresses: [{ multiaddr: relayAddrWithPeerId, isCertified: false }],
521+
protocols: [],
522+
metadata: new Map(),
523+
tags: new Map()
524+
})
525+
526+
components.transportManager.dialTransportForMultiaddr.returns(stubInterface<Transport>())
527+
components.transportManager.dial.resolves(connection)
528+
529+
dialer = new DialQueue(components)
530+
531+
await expect(dialer.dial(remotePeer)).to.eventually.equal(connection)
532+
533+
// the address passed to the transport must not have a double peer id
534+
const dialledAddr = components.transportManager.dial.getCall(0).args[0].toString()
535+
expect(dialledAddr).to.equal(relayAddrWithPeerId.toString())
536+
expect(dialledAddr).to.not.include(`/p2p/${remotePeer}/p2p/${remotePeer}`)
537+
})
475538
})

‎packages/libp2p/test/peer-discovery/peer-discovery.spec.ts‎

Lines changed: 1 addition & 200 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
1-
import { generateKeyPair } from '@libp2p/crypto/keys'
2-
import { peerIdFromPrivateKey } from '@libp2p/peer-id'
31
import { multiaddr } from '@multiformats/multiaddr'
42
import { expect } from 'aegir/chai'
53
import { TypedEventEmitter } from 'main-event'
6-
import { pEvent } from 'p-event'
74
import sinon from 'sinon'
85
import { stubInterface } from 'sinon-ts'
96
import { createLibp2p } from '../../src/index.js'
10-
import type { PeerDiscovery, PeerDiscoveryEvents, PeerInfo, Startable, Libp2p } from '@libp2p/interface'
7+
import type { PeerDiscovery, PeerDiscoveryEvents, Startable, Libp2p } from '@libp2p/interface'
118

129
describe('peer discovery', () => {
1310
let libp2p: Libp2p
@@ -36,202 +33,6 @@ describe('peer discovery', () => {
3633
expect(discovery.stop.calledOnce).to.be.true()
3734
})
3835

39-
it('should append peer id to circuit relay addresses in peer:discovery event', async () => {
40-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
41-
42-
libp2p = await createLibp2p({
43-
peerDiscovery: [() => discovery]
44-
})
45-
46-
await libp2p.start()
47-
48-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
49-
const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
50-
51-
// Address without target peer ID (e.g. as announced via identify by the remote peer)
52-
const relayAddr = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit`)
53-
54-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
55-
56-
discovery.safeDispatchEvent('peer', {
57-
detail: {
58-
id: remotePeerId,
59-
multiaddrs: [relayAddr]
60-
}
61-
})
62-
63-
const evt = await eventPromise
64-
65-
expect(evt.detail.id.toString()).to.equal(remotePeerId.toString())
66-
expect(evt.detail.multiaddrs).to.have.length(1)
67-
expect(evt.detail.multiaddrs[0].toString()).to.equal(
68-
`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`
69-
)
70-
})
71-
72-
it('should not duplicate peer id in circuit relay addresses that already have one', async () => {
73-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
74-
75-
libp2p = await createLibp2p({
76-
peerDiscovery: [() => discovery]
77-
})
78-
79-
await libp2p.start()
80-
81-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
82-
const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
83-
84-
// Address already has the target peer ID (e.g. as sent by pubsub-peer-discovery)
85-
const relayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`)
86-
87-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
88-
89-
discovery.safeDispatchEvent('peer', {
90-
detail: {
91-
id: remotePeerId,
92-
multiaddrs: [relayAddrWithPeerId]
93-
}
94-
})
95-
96-
const evt = await eventPromise
97-
98-
expect(evt.detail.id.toString()).to.equal(remotePeerId.toString())
99-
expect(evt.detail.multiaddrs).to.have.length(1)
100-
expect(evt.detail.multiaddrs[0].toString()).to.equal(
101-
`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`
102-
)
103-
})
104-
105-
it('should not modify direct (non-relay) addresses in peer:discovery event', async () => {
106-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
107-
108-
libp2p = await createLibp2p({
109-
peerDiscovery: [() => discovery]
110-
})
111-
112-
await libp2p.start()
113-
114-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
115-
const directAddr = multiaddr('/ip4/1.2.3.4/tcp/4001')
116-
117-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
118-
119-
discovery.safeDispatchEvent('peer', {
120-
detail: {
121-
id: remotePeerId,
122-
multiaddrs: [directAddr]
123-
}
124-
})
125-
126-
const evt = await eventPromise
127-
128-
expect(evt.detail.multiaddrs).to.have.length(1)
129-
expect(evt.detail.multiaddrs[0].toString()).to.equal('/ip4/1.2.3.4/tcp/4001')
130-
})
131-
132-
it('should append peer id to WebRTC circuit relay addresses missing one', async () => {
133-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
134-
135-
libp2p = await createLibp2p({
136-
peerDiscovery: [() => discovery]
137-
})
138-
139-
await libp2p.start()
140-
141-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
142-
const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
143-
144-
// WebRTC relay address without target peer ID
145-
const webrtcRelayAddr = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/webrtc`)
146-
147-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
148-
149-
discovery.safeDispatchEvent('peer', {
150-
detail: {
151-
id: remotePeerId,
152-
multiaddrs: [webrtcRelayAddr]
153-
}
154-
})
155-
156-
const evt = await eventPromise
157-
158-
expect(evt.detail.multiaddrs).to.have.length(1)
159-
expect(evt.detail.multiaddrs[0].toString()).to.equal(
160-
`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/webrtc/p2p/${remotePeerId}`
161-
)
162-
})
163-
164-
it('should not duplicate peer id in WebRTC circuit relay addresses that already have one', async () => {
165-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
166-
167-
libp2p = await createLibp2p({
168-
peerDiscovery: [() => discovery]
169-
})
170-
171-
await libp2p.start()
172-
173-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
174-
const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
175-
176-
// WebRTC relay address that already includes the target peer ID (e.g. from pubsub-peer-discovery)
177-
const webrtcRelayAddrWithPeerId = multiaddr(`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/webrtc/p2p/${remotePeerId}`)
178-
179-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
180-
181-
discovery.safeDispatchEvent('peer', {
182-
detail: {
183-
id: remotePeerId,
184-
multiaddrs: [webrtcRelayAddrWithPeerId]
185-
}
186-
})
187-
188-
const evt = await eventPromise
189-
190-
expect(evt.detail.multiaddrs).to.have.length(1)
191-
expect(evt.detail.multiaddrs[0].toString()).to.equal(
192-
`/ip4/1.2.3.4/tcp/1234/p2p/${relayPeerId}/p2p-circuit/webrtc/p2p/${remotePeerId}`
193-
)
194-
})
195-
196-
it('should handle mixed relay and direct addresses correctly in peer:discovery event', async () => {
197-
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
198-
199-
libp2p = await createLibp2p({
200-
peerDiscovery: [() => discovery]
201-
})
202-
203-
await libp2p.start()
204-
205-
const remotePeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
206-
const relayPeerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
207-
208-
const directAddr = multiaddr('/ip4/1.2.3.4/tcp/4001')
209-
const relayAddrNoId = multiaddr(`/ip4/5.6.7.8/tcp/1234/p2p/${relayPeerId}/p2p-circuit`)
210-
const relayAddrWithId = multiaddr(`/ip4/9.10.11.12/tcp/5678/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`)
211-
212-
const eventPromise = pEvent<'peer:discovery', CustomEvent<PeerInfo>>(libp2p, 'peer:discovery')
213-
214-
discovery.safeDispatchEvent('peer', {
215-
detail: {
216-
id: remotePeerId,
217-
multiaddrs: [directAddr, relayAddrNoId, relayAddrWithId]
218-
}
219-
})
220-
221-
const evt = await eventPromise
222-
223-
const addrStrings = evt.detail.multiaddrs.map(ma => ma.toString())
224-
225-
// Direct address unchanged
226-
expect(addrStrings).to.include('/ip4/1.2.3.4/tcp/4001')
227-
// Relay without peer ID gets it appended
228-
expect(addrStrings).to.include(`/ip4/5.6.7.8/tcp/1234/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`)
229-
// Relay that already has peer ID is not modified
230-
expect(addrStrings).to.include(`/ip4/9.10.11.12/tcp/5678/p2p/${relayPeerId}/p2p-circuit/p2p/${remotePeerId}`)
231-
// No address should have a double peer ID
232-
expect(addrStrings.every(a => !a.includes(`/p2p/${remotePeerId}/p2p/${remotePeerId}`))).to.be.true()
233-
})
234-
23536
it('should ignore self on discovery', async () => {
23637
const discovery = new TypedEventEmitter<PeerDiscoveryEvents>()
23738

0 commit comments

Comments
 (0)