chore: update tests

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

@@ -3,7 +3,7 @@ import { createLogger } from '@aztec/foundation/log';
 import { sleep } from '@aztec/foundation/sleep';
 import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
 
-import { ENR, SignableENR } from '@chainsafe/enr';
+import { type ENR, SignableENR } from '@chainsafe/enr';
 import { jest } from '@jest/globals';
 import { createSecp256k1PeerId } from '@libp2p/peer-id-factory';
 import { multiaddr } from '@multiformats/multiaddr';
@@ -37,9 +37,7 @@ describe('PeerManager', () => {
   // The function provided to the discovery servive callback will be run here
   let discoveredPeerCallback: (enr: ENR) => Promise<void>;
 
-  beforeEach(async () => {
-    jest.useFakeTimers();
-
+  beforeEach(() => {
     // Reset all mocks
     jest.clearAllMocks();
 
@@ -108,9 +106,8 @@ describe('PeerManager', () => {
 
       peerManager.penalizePeer(peerId, PeerErrorSeverity.LowToleranceError);
 
-      const peers = peerManager.getPeers();
-      const penalizedPeer = peers.find(p => p.id === peerId.toString());
-      //   expect(penalizedPeer?.score).toBeLessThan(0);
+      const score = peerManager.getPeerScore(peerId.toString());
+      expect(score).toBeLessThan(0);
     });
 
     it('should handle heartbeat', () => {
@@ -134,7 +131,6 @@ describe('PeerManager', () => {
     });
 
     it('should retry failed dials up to MAX_DIAL_ATTEMPTS', async () => {
-      jest.useRealTimers();
       const enr = await createMockENR();
       mockLibP2PNode.dial.mockRejectedValue(new Error('Connection failed'));
 
@@ -149,26 +145,44 @@ describe('PeerManager', () => {
       await (peerManager as any).discover();
       expect(mockLibP2PNode.dial).toHaveBeenCalledTimes(2);
 
+      // dial peer happens asynchronously, so we need to wait
+      await sleep(100);
+
       // Third attempt
       await (peerManager as any).discover();
       expect(mockLibP2PNode.dial).toHaveBeenCalledTimes(3);
 
+      // dial peer happens asynchronously, so we need to wait
+      await sleep(100);
+
       // After the third attempt, the peer should be removed from
       // the cache, and placed in timeout
       await discoveredPeerCallback(enr);
       expect(mockLibP2PNode.dial).toHaveBeenCalledTimes(3);
     });
 
+    const triggerTimeout = async (enr: ENR) => {
+      // First attempt - adds it to the cache
+      await discoveredPeerCallback(enr);
+      await sleep(100);
+      // Second attempt - on heartbeat
+      await (peerManager as any).discover();
+      await sleep(100);
+      // Third attempt - on heartbeat
+      await (peerManager as any).discover();
+    };
+
     it('should timeout a peer after max dial attempts and ignore it for the timeout period', async () => {
       const enr = await createMockENR();
       mockLibP2PNode.dial.mockRejectedValue(new Error('Connection failed'));
 
       // Fail three times to trigger timeout
-      for (let i = 0; i < 3; i++) {
-        await discoveredPeerCallback(enr);
-      }
+      await triggerTimeout(enr);
+
       expect(mockLibP2PNode.dial).toHaveBeenCalledTimes(3);
 
+      jest.useFakeTimers();
+
       // Try to dial immediately after timeout - should be ignored
       mockLibP2PNode.dial.mockClear();
       await discoveredPeerCallback(enr);
@@ -190,15 +204,15 @@ describe('PeerManager', () => {
       mockLibP2PNode.dial.mockRejectedValue(new Error('Connection failed'));
 
       // Fail three times to trigger timeout
-      for (let i = 0; i < 3; i++) {
-        await discoveredPeerCallback(enr);
-      }
+      await triggerTimeout(enr);
 
       // Verify peer is timed out
       mockLibP2PNode.dial.mockClear();
       await discoveredPeerCallback(enr);
       expect(mockLibP2PNode.dial).not.toHaveBeenCalled();
 
+      jest.useFakeTimers();
+
       // Advance time past timeout period and trigger heartbeat
       jest.advanceTimersByTime(5 * 60 * 1000);
       peerManager.heartbeat();
@@ -238,6 +252,7 @@ describe('PeerManager', () => {
 
       // Try peer2 once
       await discoveredPeerCallback(enr2);
+      await sleep(100);
 
       const peers = peerManager.getPeers(true);
       expect(peers).toHaveLength(2);

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

@@ -1,9 +1,9 @@
 import { type PeerErrorSeverity, type PeerInfo } from '@aztec/circuit-types';
 import { createLogger } from '@aztec/foundation/log';
-import { TelemetryClient, type Traceable, type Tracer, WithTracer, trackSpan } from '@aztec/telemetry-client';
+import { type TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client';
 
 import { type ENR } from '@chainsafe/enr';
-import { Connection, type PeerId } from '@libp2p/interface';
+import { type Connection, type PeerId } from '@libp2p/interface';
 import { type Multiaddr } from '@multiformats/multiaddr';
 import { inspect } from 'util';
 
@@ -42,7 +42,7 @@ export class PeerManager extends WithTracer {
     private libP2PNode: PubSubLibp2p,
     private peerDiscoveryService: PeerDiscoveryService,
     private config: P2PConfig,
-    private telemetryClient: TelemetryClient,
+    telemetryClient: TelemetryClient,
     private logger = createLogger('p2p:peer-manager'),
   ) {
     super(telemetryClient, 'PeerManager');
@@ -91,7 +91,7 @@ export class PeerManager extends WithTracer {
    * Simply logs the type of connected peer.
    * @param e - The connected peer event.
    */
-  private async handleConnectedPeerEvent(e: CustomEvent<PeerId>) {
+  private handleConnectedPeerEvent(e: CustomEvent<PeerId>) {
     const peerId = e.detail;
     if (this.peerDiscoveryService.isBootstrapPeer(peerId)) {
       this.logger.verbose(`Connected to bootstrap peer ${peerId.toString()}`);
@@ -104,7 +104,7 @@ export class PeerManager extends WithTracer {
    * Simply logs the type of disconnected peer.
    * @param e - The disconnected peer event.
    */
-  private async handleDisconnectedPeerEvent(e: CustomEvent<PeerId>) {
+  private handleDisconnectedPeerEvent(e: CustomEvent<PeerId>) {
     const peerId = e.detail;
     if (this.peerDiscoveryService.isBootstrapPeer(peerId)) {
       this.logger.verbose(`Disconnected from bootstrap peer ${peerId.toString()}`);
@@ -183,8 +183,6 @@ export class PeerManager extends WithTracer {
 
     const cachedPeersToDial: CachedPeer[] = [];
 
-    console.log('cachedPeers', this.cachedPeers);
-
     const pendingDials = new Set(
       this.libP2PNode
         .getDialQueue()
@@ -233,6 +231,7 @@ export class PeerManager extends WithTracer {
         case PeerScoreState.Banned:
         case PeerScoreState.Disconnect:
           void this.disconnectPeer(peer.remotePeer);
+          break;
         case PeerScoreState.Healthy:
           connectedHealthyPeers.push(peer);
       }
@@ -314,20 +313,18 @@ export class PeerManager extends WithTracer {
 
   private async dialPeer(peer: CachedPeer) {
     const id = peer.peerId.toString();
+
+    // Add to the address book before dialing
     await this.libP2PNode.peerStore.merge(peer.peerId, { multiaddrs: [peer.multiaddrTcp] });
 
     this.logger.trace(`Dialing peer ${id}`);
     try {
-      console.log('dialing peer', peer.multiaddrTcp);
       await this.libP2PNode.dial(peer.multiaddrTcp);
     } catch (error) {
-      console.log('error', error);
       peer.dialAttempts++;
       if (peer.dialAttempts < MAX_DIAL_ATTEMPTS) {
         this.logger.trace(`Failed to dial peer ${id} (attempt ${peer.dialAttempts})`, { error: inspect(error) });
-        console.log('failed to dial peer, adding to cache');
         this.cachedPeers.set(id, peer);
-        console.log('cachedPeers', this.cachedPeers);
       } else {
         this.logger.debug(`Failed to dial peer ${id} (dropping)`, { error: inspect(error) });
         this.cachedPeers.delete(id);
@@ -337,8 +334,6 @@ export class PeerManager extends WithTracer {
           timeoutUntilMs: Date.now() + FAILED_PEER_BAN_TIME_MS,
         });
       }
-    } finally {
-      console.log('dialed peer', peer.multiaddrTcp);
     }
   }