fix: ensure relay listener is removed

Author: maschad

packages/libp2p/src/circuit-relay/transport/listener.ts

@@ -32,7 +32,6 @@ class CircuitRelayTransportListener extends EventEmitter<ListenerEvents> impleme
 
     // remove listening addrs when a relay is removed
     this.relayStore.addEventListener('relay:removed', this._onRemoveRelayPeer)
-    console.log('added event listener:', this.relayStore.listenerCount('relay:removed'))
   }
 
   _onRemoveRelayPeer = (evt: CustomEvent<PeerId>): void => {
@@ -92,11 +91,7 @@ class CircuitRelayTransportListener extends EventEmitter<ListenerEvents> impleme
   }
 
   async close (): Promise<void> {
-    // remove event listener
-    console.log('removing relay event listener for peer %s', Array.from(this.listeningAddrs.keys()).join(', '))
-    log.trace('removing relay event listener for peer %s', Array.from(this.listeningAddrs.keys()).join(', '))
-    this.relayStore.removeEventListener('relay:removed', this._onRemoveRelayPeer)
-    console.log('removing count:', this.relayStore.listenerCount('relay:removed'))
+
   }
 
   #removeRelayPeer (peerId: PeerId): void {
@@ -107,6 +102,8 @@ class CircuitRelayTransportListener extends EventEmitter<ListenerEvents> impleme
     this.listeningAddrs.delete(peerId)
 
     if (had) {
+      log.trace('removing relay event listener for peer %s', Array.from(this.listeningAddrs.keys()).join(', '))
+      this.relayStore.removeEventListener('relay:removed', this._onRemoveRelayPeer)
       // Announce listen addresses change
       this.safeDispatchEvent('close', {})
     }

packages/libp2p/src/circuit-relay/transport/reservation-store.ts

@@ -112,8 +112,9 @@ export class ReservationStore extends EventEmitter<ReservationStoreEvents> imple
     // remove the reservation and multiaddr and maybe trigger search
     // for new relays
     this.events.addEventListener('peer:disconnect', (evt) => {
-      console.log('peer:disconnect event listener', evt.detail)
-      this.#removeRelay(evt.detail)
+      this.#removeRelay(evt.detail).catch(err => {
+        log.error('error removing relay %p', evt.detail, err)
+      })
     })
   }
 
@@ -288,24 +289,43 @@ export class ReservationStore extends EventEmitter<ReservationStoreEvents> imple
     throw new Error(errMsg)
   }
 
+  async removeListeners (peerId: PeerId): Promise<void> {
+    const listenersToRemove: string[] = []
+
+    this.transportManager.getListeners().forEach((listeners) => {
+      listeners.getAddrs().forEach((addr) => {
+        if (addr.toString().includes(peerId.toString())) {
+          listenersToRemove.push(addr.toString())
+        }
+      })
+    })
+
+    for (const addr of listenersToRemove) {
+      await this.transportManager.remove(addr)
+      log('removed addr: %s', addr)
+    }
+  }
+
   /**
    * Remove listen relay
    */
-  #removeRelay (peerId: PeerId): void {
+  async #removeRelay (peerId: PeerId): Promise<void> {
     const existingReservation = this.reservations.get(peerId)
 
     if (existingReservation == null) {
       return
     }
 
-    log('connection to relay %p closed, removing reservation from local store', peerId)
+    log.trace('connection to relay %p closed, removing reservation from local store', peerId)
 
     clearTimeout(existingReservation.timeout)
     this.reservations.delete(peerId)
 
+    await this.removeListeners(peerId)
+
     this.safeDispatchEvent('relay:removed', { detail: peerId })
 
-    console.log('removing listener for relay peer %s', peerId)
+    log.trace('removing relay peer %s', peerId)
 
     if (this.reservations.size < this.maxDiscoveredRelays) {
       log('not enough relays %d/%d', this.reservations.size, this.maxDiscoveredRelays)

packages/libp2p/test/circuit-relay/relay.node.ts

@@ -685,12 +685,10 @@ describe('circuit-relay', () => {
       expect(events[1].detail.remotePeer.toString()).to.equal(relay1.peerId.toString())
     })
 
-    // Not sure why this test isn't passing
-    it.skip('should remove the relay event listener when the relay stops', async () => {
+    it('should remove the relay event listener when the relay stops', async () => {
       // discover relay and make reservation
       await local.dial(relay1.getMultiaddrs()[0])
       await local.dial(relay2.getMultiaddrs()[0])
-      await local.dial(relay3.getMultiaddrs()[0])
 
       await usingAsRelayCount(local, [relay1, relay2], 2)
 
@@ -708,8 +706,7 @@ describe('circuit-relay', () => {
       expect(circuitListener[0].relayStore.listenerCount('relay:removed')).to.equal(2)
 
       // remove one listener
-      await relay1.stop()
-      await relay2.stop()
+      await local.hangUp(relay1.peerId)
 
       await notUsingAsRelay(local, relay1)