fix: auto-confirm relay addresses

achingbrain · achingbrain · commit 4c30c5b3b308 · 2024-12-10T12:49:40.000Z
After we have created a reservation on a relay, automatically confirm that it is publicly dialable. Fixes #2883
diff --git a/packages/interface-internal/src/address-manager/index.ts b/packages/interface-internal/src/address-manager/index.ts
@@ -50,6 +50,11 @@ export interface ConfirmAddressOptions {
    * Override the TTL of the observed address verification
    */
   ttl?: number
+
+  /**
+   * Allows hinting which type of address this is
+   */
+  type?: AddressType
 }
 
 export interface AddressManager {
diff --git a/packages/libp2p/src/address-manager/index.ts b/packages/libp2p/src/address-manager/index.ts
@@ -249,19 +249,19 @@ export class AddressManager implements AddressManagerInterface {
     addr = stripPeerId(addr, this.components.peerId)
     let startingConfidence = true
 
-    if (this.observed.has(addr)) {
+    if (options?.type === 'observed' || this.observed.has(addr)) {
       startingConfidence = this.observed.confirm(addr, options?.ttl ?? this.addressVerificationTTL)
     }
 
-    if (this.transportAddresses.has(addr)) {
+    if (options?.type === 'transport' || this.transportAddresses.has(addr)) {
       startingConfidence = this.transportAddresses.confirm(addr, options?.ttl ?? this.addressVerificationTTL)
     }
 
-    if (this.dnsMappings.has(addr)) {
+    if (options?.type === 'dns-mapping' || this.dnsMappings.has(addr)) {
       startingConfidence = this.dnsMappings.confirm(addr, options?.ttl ?? this.addressVerificationTTL)
     }
 
-    if (this.ipMappings.has(addr)) {
+    if (options?.type === 'ip-mapping' || this.ipMappings.has(addr)) {
       startingConfidence = this.ipMappings.confirm(addr, options?.ttl ?? this.addressVerificationTTL)
     }
 
diff --git a/packages/libp2p/test/addresses/address-manager.spec.ts b/packages/libp2p/test/addresses/address-manager.spec.ts
@@ -680,4 +680,38 @@ describe('Address Manager', () => {
       multiaddr(`/ip4/${internalIp}/${protocol}/${internalPort}/p2p/${peerId.toString()}`)
     ])
   })
+
+  it('should confirm unknown observed addresses with hints', () => {
+    const transportManager = stubInterface<TransportManager>()
+    const am = new AddressManager({
+      peerId,
+      transportManager,
+      peerStore,
+      events,
+      logger: defaultLogger()
+    })
+
+    const internalIp = '192.168.1.123'
+    const internalPort = 4567
+    const externalIp = '2a00:23c6:14b1:7e00:28b8:30d:944e:27f3'
+    const externalPort = 8910
+    const protocol = 'tcp'
+
+    // confirm address before fetching addresses
+    am.confirmObservedAddr(multiaddr(`/ip6/${externalIp}/${protocol}/${externalPort}`), {
+      type: 'transport'
+    })
+
+    // one loopback, one LAN address
+    transportManager.getAddrs.returns([
+      multiaddr(`/ip4/${internalIp}/${protocol}/${internalPort}`),
+      multiaddr(`/ip6/${externalIp}/${protocol}/${externalPort}`)
+    ])
+
+    // should have changed the address list
+    expect(am.getAddresses()).to.deep.equal([
+      multiaddr(`/ip4/${internalIp}/${protocol}/${internalPort}/p2p/${peerId.toString()}`),
+      multiaddr(`/ip6/${externalIp}/${protocol}/${externalPort}/p2p/${peerId.toString()}`)
+    ])
+  })
 })
diff --git a/packages/transport-circuit-relay-v2/src/transport/listener.ts b/packages/transport-circuit-relay-v2/src/transport/listener.ts
@@ -2,15 +2,16 @@ import { ListenError, TypedEventEmitter, setMaxListeners } from '@libp2p/interfa
 import { multiaddr } from '@multiformats/multiaddr'
 import { DEFAULT_RESERVATION_COMPLETION_TIMEOUT } from '../constants.js'
 import { CircuitListen, CircuitSearch } from '../utils.js'
-import type { RelayDiscovery } from './discovery.js'
 import type { RelayReservation, ReservationStore } from './reservation-store.js'
 import type { ComponentLogger, Logger, Listener, ListenerEvents, PeerId } from '@libp2p/interface'
-import type { ConnectionManager } from '@libp2p/interface-internal'
+import type { AddressManager, ConnectionManager } from '@libp2p/interface-internal'
 import type { Multiaddr } from '@multiformats/multiaddr'
 
 export interface CircuitRelayTransportListenerComponents {
+  peerId: PeerId
   connectionManager: ConnectionManager
-  relayStore: ReservationStore
+  addressManager: AddressManager
+  reservationStore: ReservationStore
   logger: ComponentLogger
 }
 
@@ -19,9 +20,10 @@ export interface CircuitRelayTransportListenerInit {
 }
 
 class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> implements Listener {
+  private readonly peerId: PeerId
   private readonly connectionManager: ConnectionManager
+  private readonly addressManager: AddressManager
   private readonly reservationStore: ReservationStore
-  private readonly discovery?: RelayDiscovery
   private listeningAddrs: Multiaddr[]
   private readonly log: Logger
   private readonly listenTimeout: number
@@ -32,8 +34,10 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
     super()
 
     this.log = components.logger.forComponent('libp2p:circuit-relay:transport:listener')
+    this.peerId = components.peerId
     this.connectionManager = components.connectionManager
-    this.reservationStore = components.relayStore
+    this.addressManager = components.addressManager
+    this.reservationStore = components.reservationStore
     this.listeningAddrs = []
     this.listenTimeout = init.listenTimeout ?? DEFAULT_RESERVATION_COMPLETION_TIMEOUT
 
@@ -51,6 +55,11 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
 
     this.log('relay peer removed %p', evt.detail.relay)
 
+    this.listeningAddrs.forEach(ma => {
+      // mark as externally dialable
+      this.addressManager.removeObservedAddr(ma)
+    })
+
     this.listeningAddrs = []
 
     // announce listen addresses change
@@ -59,7 +68,7 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
 
   _onAddRelayPeer = (evt: CustomEvent<RelayReservation>): void => {
     const {
-      relay, details
+      details
     } = evt.detail
 
     if (details.type === 'configured') {
@@ -70,16 +79,7 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
       return
     }
 
-    this.log('relay peer added %p', relay)
-
-    this.relay = relay
-
-    // add all addresses from the relay reservation
-    this.listeningAddrs = details.reservation.addrs
-      .map(buf => multiaddr(buf).encapsulate('/p2p-circuit'))
-
-    // announce listen addresses change
-    this.safeDispatchEvent('listening')
+    this.addedRelay(evt.detail)
   }
 
   async listen (addr: Multiaddr): Promise<void> {
@@ -102,18 +102,7 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
       if (!this.reservationStore.hasReservation(relayConn.remotePeer)) {
         this.log('making reservation on peer %p', relayConn.remotePeer)
         const reservation = await this.reservationStore.addRelay(relayConn.remotePeer, 'configured')
-        this.log('made reservation on peer %p', relayConn.remotePeer)
-
-        this.relay = reservation.relay
-
-        // add all addresses from the relay reservation
-        this.listeningAddrs = reservation.details.reservation.addrs
-          .map(buf => multiaddr(buf).encapsulate('/p2p-circuit'))
-
-        // if that succeeded announce listen addresses change
-        queueMicrotask(() => {
-          this.safeDispatchEvent('listening')
-        })
+        this.addedRelay(reservation)
       }
     } else {
       throw new ListenError(`Could not listen on p2p-circuit address "${addr}"`)
@@ -136,6 +125,28 @@ class CircuitRelayTransportListener extends TypedEventEmitter<ListenerEvents> im
       this.safeDispatchEvent('close')
     })
   }
+
+  private addedRelay (reservation: RelayReservation): void {
+    this.log('relay peer added %p', reservation.relay)
+
+    this.relay = reservation.relay
+
+    // add all addresses from the relay reservation
+    this.listeningAddrs = reservation.details.reservation.addrs
+      .map(buf => multiaddr(buf).encapsulate('/p2p-circuit'))
+
+    this.listeningAddrs.forEach(ma => {
+      // mark as externally dialable
+      this.addressManager.confirmObservedAddr(ma, {
+        type: 'transport'
+      })
+    })
+
+    // if that succeeded announce listen addresses change
+    queueMicrotask(() => {
+      this.safeDispatchEvent('listening')
+    })
+  }
 }
 
 export function createListener (options: CircuitRelayTransportListenerComponents): Listener {
diff --git a/packages/transport-circuit-relay-v2/src/transport/transport.ts b/packages/transport-circuit-relay-v2/src/transport/transport.ts
@@ -246,8 +246,10 @@ export class CircuitRelayTransport implements Transport<CircuitRelayDialEvents>
    */
   createListener (options: CreateListenerOptions): Listener {
     return createListener({
+      peerId: this.peerId,
       connectionManager: this.connectionManager,
-      relayStore: this.reservationStore,
+      addressManager: this.addressManager,
+      reservationStore: this.reservationStore,
       logger: this.logger
     })
   }
diff --git a/packages/transport-circuit-relay-v2/test/listener.spec.ts b/packages/transport-circuit-relay-v2/test/listener.spec.ts
@@ -0,0 +1,101 @@
+import { generateKeyPair } from '@libp2p/crypto/keys'
+import { defaultLogger } from '@libp2p/logger'
+import { peerIdFromPrivateKey } from '@libp2p/peer-id'
+import { multiaddr } from '@multiformats/multiaddr'
+import { expect } from 'aegir/chai'
+import { stubInterface } from 'sinon-ts'
+import { createListener } from '../src/transport/listener.js'
+import { type ReservationStore } from '../src/transport/reservation-store.js'
+import type { ComponentLogger, Connection, Listener, PeerId } from '@libp2p/interface'
+import type { AddressManager, ConnectionManager } from '@libp2p/interface-internal'
+import type { StubbedInstance } from 'sinon-ts'
+
+export interface CircuitRelayTransportListenerComponents {
+  peerId: PeerId
+  connectionManager: StubbedInstance<ConnectionManager>
+  addressManager: StubbedInstance<AddressManager>
+  reservationStore: StubbedInstance<ReservationStore>
+  logger: ComponentLogger
+}
+
+describe('listener', () => {
+  let listener: Listener
+  let components: CircuitRelayTransportListenerComponents
+
+  beforeEach(async () => {
+    components = {
+      peerId: peerIdFromPrivateKey(await generateKeyPair('Ed25519')),
+      connectionManager: stubInterface(),
+      addressManager: stubInterface(),
+      reservationStore: stubInterface(),
+      logger: defaultLogger()
+    }
+
+    listener = createListener(components)
+  })
+
+  it('should auto-confirm discovered relay addresses', async () => {
+    await listener.listen(multiaddr('/p2p-circuit'))
+
+    expect(components.reservationStore.reserveRelay).to.have.property('called', true, 'did not begin relay search')
+
+    const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const relayAddr = multiaddr(`/ip4/123.123.123.123/tcp/1234/p2p/${relayPeer}`)
+
+    const createdReservationListener = components.reservationStore.addEventListener.getCall(1).args[1]
+
+    if (typeof createdReservationListener === 'function') {
+      createdReservationListener(
+        new CustomEvent('relay:created-reservation', {
+          detail: {
+            relay: relayPeer,
+            details: {
+              type: 'discovered',
+              reservation: {
+                addrs: [
+                  relayAddr
+                ]
+              }
+            }
+          }
+        })
+      )
+    }
+
+    expect(components.addressManager.confirmObservedAddr.calledWith(
+      relayAddr.encapsulate('/p2p-circuit')
+    )).to.be.true()
+  })
+
+  it('should auto-confirm configured relay addresses', async () => {
+    const relayPeer = peerIdFromPrivateKey(await generateKeyPair('Ed25519'))
+    const relayAddr = multiaddr(`/ip4/123.123.123.123/tcp/1234/p2p/${relayPeer}/p2p-circuit`)
+    const conn = stubInterface<Connection>({
+      id: 'connection-id-1234',
+      remotePeer: relayPeer
+    })
+
+    components.connectionManager.openConnection.withArgs(relayAddr.decapsulate('/p2p-circuit')).resolves(conn)
+
+    components.reservationStore.addRelay.withArgs(relayPeer).resolves({
+      relay: relayPeer,
+      details: {
+        type: 'configured',
+        reservation: {
+          addrs: [
+            relayAddr.bytes
+          ],
+          expire: 100n
+        },
+        timeout: 0 as any,
+        connection: conn.id
+      }
+    })
+
+    await listener.listen(relayAddr)
+
+    expect(components.addressManager.confirmObservedAddr.calledWith(
+      relayAddr.encapsulate('/p2p-circuit')
+    )).to.be.true()
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -249,19 +249,19 @@ export class AddressManager implements AddressManagerInterface {`
`249`	`249`	`addr = stripPeerId(addr, this.components.peerId)`
`250`	`250`	`let startingConfidence = true`
`251`	`251`
`252`		`- if (this.observed.has(addr)) {`
	`252`	`+ if (options?.type === 'observed' \|\| this.observed.has(addr)) {`
`253`	`253`	`startingConfidence = this.observed.confirm(addr, options?.ttl ?? this.addressVerificationTTL)`
`254`	`254`	`}`
`255`	`255`
`256`		`- if (this.transportAddresses.has(addr)) {`
	`256`	`+ if (options?.type === 'transport' \|\| this.transportAddresses.has(addr)) {`
`257`	`257`	`startingConfidence = this.transportAddresses.confirm(addr, options?.ttl ?? this.addressVerificationTTL)`
`258`	`258`	`}`
`259`	`259`
`260`		`- if (this.dnsMappings.has(addr)) {`
	`260`	`+ if (options?.type === 'dns-mapping' \|\| this.dnsMappings.has(addr)) {`
`261`	`261`	`startingConfidence = this.dnsMappings.confirm(addr, options?.ttl ?? this.addressVerificationTTL)`
`262`	`262`	`}`
`263`	`263`
`264`		`- if (this.ipMappings.has(addr)) {`
	`264`	`+ if (options?.type === 'ip-mapping' \|\| this.ipMappings.has(addr)) {`
`265`	`265`	`startingConfidence = this.ipMappings.confirm(addr, options?.ttl ?? this.addressVerificationTTL)`
`266`	`266`	`}`
`267`	`267`