From ab2c1f6731ccfe21a39482bdab217a8abd3f027b Mon Sep 17 00:00:00 2001
From: Chad Nehemiah <chad.nehemiah94@gmail.com>
Date: Wed, 25 Oct 2023 02:58:52 -0500
Subject: [PATCH] fix: remove relay:removed event listener after relay is
 removed (#1998)

When a relay is removed, also remove the event listener

Closes #1944

Closes https://github.com/libp2p/js-libp2p/issues/2106

---------

Co-authored-by: Alex Potsides <alex@achingbrain.net>
---
 .../src/circuit-relay/transport/listener.ts   | 10 ++++--
 .../libp2p/test/circuit-relay/relay.node.ts   | 33 +++++++++++++++++--
 packages/libp2p/test/circuit-relay/utils.ts   | 17 ++++++++++
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/packages/libp2p/src/circuit-relay/transport/listener.ts b/packages/libp2p/src/circuit-relay/transport/listener.ts
index dea961b5fd..bb67007db9 100644
--- a/packages/libp2p/src/circuit-relay/transport/listener.ts
+++ b/packages/libp2p/src/circuit-relay/transport/listener.ts
@@ -31,9 +31,11 @@ class CircuitRelayTransportListener extends EventEmitter<ListenerEvents> impleme
     this.listeningAddrs = new PeerMap()
 
     // remove listening addrs when a relay is removed
-    this.relayStore.addEventListener('relay:removed', (evt) => {
-      this.#removeRelayPeer(evt.detail)
-    })
+    this.relayStore.addEventListener('relay:removed', this._onRemoveRelayPeer)
+  }
+
+  _onRemoveRelayPeer = (evt: CustomEvent<PeerId>): void => {
+    this.#removeRelayPeer(evt.detail)
   }
 
   async listen (addr: Multiaddr): Promise<void> {
@@ -100,6 +102,8 @@ class CircuitRelayTransportListener extends EventEmitter<ListenerEvents> impleme
     this.listeningAddrs.delete(peerId)
 
     if (had) {
+      log.trace('removing relay event listener for peer %p', peerId)
+      this.relayStore.removeEventListener('relay:removed', this._onRemoveRelayPeer)
       // Announce listen addresses change
       this.safeDispatchEvent('close', {})
     }
diff --git a/packages/libp2p/test/circuit-relay/relay.node.ts b/packages/libp2p/test/circuit-relay/relay.node.ts
index 936dfbb869..bcf608a10d 100644
--- a/packages/libp2p/test/circuit-relay/relay.node.ts
+++ b/packages/libp2p/test/circuit-relay/relay.node.ts
@@ -21,7 +21,7 @@ import { HopMessage, Status } from '../../src/circuit-relay/pb/index.js'
 import { identifyService } from '../../src/identify/index.js'
 import { createLibp2p, type Libp2pOptions } from '../../src/index.js'
 import { plaintext } from '../../src/insecure/index.js'
-import { discoveredRelayConfig, doesNotHaveRelay, getRelayAddress, hasRelay, usingAsRelay } from './utils.js'
+import { discoveredRelayConfig, doesNotHaveRelay, getRelayAddress, hasRelay, notUsingAsRelay, usingAsRelay, usingAsRelayCount } from './utils.js'
 import type { Components } from '../../src/components.js'
 import type { Libp2p } from '@libp2p/interface'
 import type { Connection } from '@libp2p/interface/connection'
@@ -354,7 +354,7 @@ describe('circuit-relay', () => {
           transports: [
             tcp(),
             circuitRelayTransport({
-              discoverRelays: 1
+              discoverRelays: 3
             })
           ]
         }),
@@ -603,6 +603,35 @@ describe('circuit-relay', () => {
       expect(events[1].detail.remotePeer.toString()).to.equal(relay1.peerId.toString())
     })
 
+    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 usingAsRelayCount(local, [relay1, relay2], 2)
+
+      // expect 2 listeners
+      // @ts-expect-error these are private fields
+      const listeners = local.components.transportManager.getListeners()
+
+      // @ts-expect-error as a result these will have any types
+      const circuitListener = listeners.filter(listener => {
+        // @ts-expect-error as a result these will have any types
+        const circuitMultiaddrs = listener.getAddrs().filter(ma => Circuit.matches(ma))
+        return circuitMultiaddrs.length > 0
+      })
+
+      expect(circuitListener[0].relayStore.listenerCount('relay:removed')).to.equal(2)
+
+      // remove one listener
+      await local.hangUp(relay1.peerId)
+
+      await notUsingAsRelay(local, relay1)
+
+      // expect 1 listener
+      expect(circuitListener[0].relayStore.listenerCount('relay:removed')).to.equal(1)
+    })
+
     it('should mark a relayed connection as transient', async () => {
       // discover relay and make reservation
       const connectionToRelay = await remote.dial(relay1.getMultiaddrs()[0])
diff --git a/packages/libp2p/test/circuit-relay/utils.ts b/packages/libp2p/test/circuit-relay/utils.ts
index e11bd708f6..aa3cde6e15 100644
--- a/packages/libp2p/test/circuit-relay/utils.ts
+++ b/packages/libp2p/test/circuit-relay/utils.ts
@@ -30,6 +30,23 @@ export async function usingAsRelay (node: Libp2p, relay: Libp2p, opts?: PWaitFor
   }, opts)
 }
 
+export async function usingAsRelayCount (node: Libp2p, relays: Libp2p[], count: number): Promise<void> {
+  // Wait for peer to be used as a relay
+  await pWaitFor(async () => {
+    let relayCount = 0
+
+    for (const relay of relays) {
+      for (const addr of node.getMultiaddrs()) {
+        const search = `${relay.peerId.toString()}/p2p-circuit`
+        if (addr.toString().includes(search)) {
+          relayCount++
+        }
+      }
+    }
+    return relayCount === count
+  })
+}
+
 export async function notUsingAsRelay (node: Libp2p, relay: Libp2p, opts?: PWaitForOptions<boolean>): Promise<void> {
   // Wait for peer to be used as a relay
   await pWaitFor(() => {