From bcfa15993fd533c56c7523384e4b135c4930855b Mon Sep 17 00:00:00 2001
From: Alex Potsides <alex@achingbrain.net>
Date: Sat, 2 Dec 2023 11:59:08 +0000
Subject: [PATCH] fix: remove duplicate autodial from startup (#2289)

There's no need to set a timeout for autodialing during `.start`
since we autodial during `.afterStart` which will then set the
timeout for the next dial if one is required.

We guard on running twice so this isn't a problem, it's just
unnecessary.
---
 .../src/connection-manager/auto-dial.ts       | 46 +++++++++----------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/packages/libp2p/src/connection-manager/auto-dial.ts b/packages/libp2p/src/connection-manager/auto-dial.ts
index 7f65917441..49d60e4f97 100644
--- a/packages/libp2p/src/connection-manager/auto-dial.ts
+++ b/packages/libp2p/src/connection-manager/auto-dial.ts
@@ -101,12 +101,6 @@ export class AutoDial implements Startable {
   }
 
   start (): void {
-    this.autoDialInterval = setTimeout(() => {
-      this.autoDial()
-        .catch(err => {
-          this.log.error('error while autodialing', err)
-        })
-    }, this.autoDialIntervalMs)
     this.started = true
   }
 
@@ -126,28 +120,27 @@ export class AutoDial implements Startable {
   }
 
   async autoDial (): Promise<void> {
-    if (!this.started) {
+    if (!this.started || this.running) {
       return
     }
 
     const connections = this.connectionManager.getConnectionsMap()
     const numConnections = connections.size
 
-    // Already has enough connections
+    // already have enough connections
     if (numConnections >= this.minConnections) {
       if (this.minConnections > 0) {
         this.log.trace('have enough connections %d/%d', numConnections, this.minConnections)
       }
+
+      // no need to schedule next autodial as it will be run when on
+      // connection:close event
       return
     }
 
     if (this.queue.size > this.autoDialMaxQueueLength) {
       this.log('not enough connections %d/%d but auto dial queue is full', numConnections, this.minConnections)
-      return
-    }
-
-    if (this.running) {
-      this.log('not enough connections %d/%d - but skipping autodial as it is already running', numConnections, this.minConnections)
+      this.sheduleNextAutodial()
       return
     }
 
@@ -162,12 +155,12 @@ export class AutoDial implements Startable {
         .filter(Boolean)
     )
 
-    // Sort peers on whether we know protocols or public keys for them
+    // sort peers on whether we know protocols or public keys for them
     const peers = await this.peerStore.all({
       filters: [
-        // Remove some peers
+        // remove some peers
         (peer) => {
-          // Remove peers without addresses
+          // remove peers without addresses
           if (peer.addresses.length === 0) {
             this.log.trace('not autodialing %p because they have no addresses', peer.id)
             return false
@@ -200,7 +193,7 @@ export class AutoDial implements Startable {
     // dialled in a different order each time
     const shuffledPeers = peers.sort(() => Math.random() > 0.5 ? 1 : -1)
 
-    // Sort shuffled peers by tag value
+    // sort shuffled peers by tag value
     const peerValues = new PeerMap<number>()
     for (const peer of shuffledPeers) {
       if (peerValues.has(peer.id)) {
@@ -271,14 +264,19 @@ export class AutoDial implements Startable {
     }
 
     this.running = false
+    this.sheduleNextAutodial()
+  }
 
-    if (this.started) {
-      this.autoDialInterval = setTimeout(() => {
-        this.autoDial()
-          .catch(err => {
-            this.log.error('error while autodialing', err)
-          })
-      }, this.autoDialIntervalMs)
+  private sheduleNextAutodial (): void {
+    if (!this.started) {
+      return
     }
+
+    this.autoDialInterval = setTimeout(() => {
+      this.autoDial()
+        .catch(err => {
+          this.log.error('error while autodialing', err)
+        })
+    }, this.autoDialIntervalMs)
   }
 }