chore: Removed pruning of the duplicate connections (#16461)

Removes pruning of "duplicate" peer connections.
Libp2p manages this, but more importantly, it's perfectly fine to have
multiple connections per peer.
([source](https://discuss.libp2p.io/t/random-walk-peer-discovery-adds-connections-to-same-peer/584))
Finally, choosing the most recent ones to remove seems somewhat
arbitrary (i.e., why not prune the oldest).

With this change, the preferred gossip test hasn't flaked for me using
`grind` at all.

Author: alexghr

yarn-project/p2p/src/services/peer-manager/peer_manager.test.ts

@@ -415,40 +415,6 @@ describe('PeerManager', () => {
       expect(mockLibP2PNode.hangUp).toHaveBeenCalledTimes(2);
     });
 
-    it('should disconnect from duplicate peers during heartbeat', async () => {
-      // Create a peer that will have duplicate connections
-      const peerId = await createSecp256k1PeerId();
-
-      // Create mock connections with different timestamps to simulate connections opened at different times
-      const oldestConnection = {
-        remotePeer: peerId,
-        timeline: { open: 1000 },
-        close: jest.fn(),
-      };
-      const duplicateConnection1 = {
-        remotePeer: peerId,
-        timeline: { open: 2000 },
-        close: jest.fn(),
-      };
-      const duplicateConnection2 = {
-        remotePeer: peerId,
-        timeline: { open: 3000 },
-        close: jest.fn(),
-      };
-      mockLibP2PNode.getConnections.mockReturnValue([duplicateConnection1, oldestConnection, duplicateConnection2]);
-
-      // Trigger heartbeat which should call pruneDuplicatePeers
-      await peerManager.heartbeat();
-
-      await sleep(100);
-
-      // Verify that the duplicate connections were closed
-      expect(duplicateConnection1.close).toHaveBeenCalled();
-      expect(duplicateConnection2.close).toHaveBeenCalled();
-      // Verify that the oldest connection was not closed
-      expect(oldestConnection.close).not.toHaveBeenCalled();
-    });
-
     it('should properly clean up peers on stop', async () => {
       mockLibP2PNode.getPeers.mockReturnValue([await createSecp256k1PeerId(), await createSecp256k1PeerId()]);
 
@@ -692,9 +658,6 @@ describe('PeerManager', () => {
       // This ensures prioritizePeers gets the correct connections
       jest.spyOn(peerManager as any, 'pruneUnhealthyPeers').mockImplementation(connections => connections);
 
-      // Mock the pruneDuplicatePeers method to return the connections unchanged
-      jest.spyOn(peerManager as any, 'pruneDuplicatePeers').mockImplementation(connections => connections);
-
       // Trigger heartbeat which should call prioritizePeers
       await peerManager.heartbeat();
 

yarn-project/p2p/src/services/peer-manager/peer_manager.ts

@@ -499,9 +499,7 @@ export class PeerManager implements PeerManagerInterface {
   private discover() {
     const connections = this.libP2PNode.getConnections();
 
-    const healthyConnections = this.prioritizePeers(
-      this.pruneUnhealthyPeers(this.getNonProtectedPeers(this.pruneDuplicatePeers(connections))),
-    );
+    const healthyConnections = this.prioritizePeers(this.pruneUnhealthyPeers(this.getNonProtectedPeers(connections)));
 
     // Calculate how many connections we're looking to make
     const protectedPeerCount = this.getProtectedPeerCount();
@@ -623,38 +621,6 @@ export class PeerManager implements PeerManagerInterface {
     }
   }
 
-  /**
-   * If multiple connections to the same peer are found, the oldest connection is kept and the duplicates are pruned.
-   *
-   * This is necessary to resolve a race condition where multiple connections to the same peer are established if
-   * they are discovered at the same time.
-   *
-   * @param connections - The list of connections to prune duplicate peers from.
-   * @returns The pruned list of connections.
-   */
-  private pruneDuplicatePeers(connections: Connection[]): Connection[] {
-    const peerConnections = new Map<string, Connection>();
-
-    for (const conn of connections) {
-      const peerId = conn.remotePeer.toString();
-      const existingConnection = peerConnections.get(peerId);
-      if (!existingConnection) {
-        peerConnections.set(peerId, conn);
-      } else {
-        // Keep the oldest connection for each peer
-        this.logger.debug(`Found duplicate connection to peer ${peerId}, keeping oldest connection`);
-        if (conn.timeline.open < existingConnection.timeline.open) {
-          peerConnections.set(peerId, conn);
-          void existingConnection.close();
-        } else {
-          void conn.close();
-        }
-      }
-    }
-
-    return [...peerConnections.values()];
-  }
-
   private async goodbyeAndDisconnectPeer(peer: PeerId, reason: GoodByeReason) {
     this.logger.debug(`Disconnecting peer ${peer.toString()} with reason ${prettyGoodbyeReason(reason)}`);