feat: ensure re-dials are sorted by success rate (libp2p#2150)

Author: maschad

packages/libp2p/src/connection-manager/dial-queue.ts

@@ -279,8 +279,7 @@ export class DialQueue {
     return signal
   }
 
-  // eslint-disable-next-line complexity
-  private async calculateMultiaddrs (peerId?: PeerId, addrs: Address[] = [], options: DialOptions = {}): Promise<Address[]> {
+  async calculateMultiaddrs (peerId?: PeerId, addrs: Address[] = [], options: DialOptions = {}): Promise<Address[]> {
     // if a peer id or multiaddr(s) with a peer id, make sure it isn't our peer id and that we are allowed to dial it
     if (peerId != null) {
       if (this.peerId.equals(peerId)) {
@@ -380,6 +379,8 @@ export class DialQueue {
         // append peer id to multiaddr if it is not already present
         if (addressPeerId !== peerId.toString()) {
           return {
+            lastFailure: addr.lastFailure,
+            lastSuccess: addr.lastSuccess,
             multiaddr: addr.multiaddr.encapsulate(peerIdMultiaddr),
             isCertified: addr.isCertified
           }
@@ -406,9 +407,47 @@ export class DialQueue {
       throw new CodeError('The connection gater denied all addresses in the dial request', codes.ERR_NO_VALID_ADDRESSES)
     }
 
+    sortedGatedAddrs.sort((a, b) => this.sortMultiaddrsByDialability(a, b))
+
     return sortedGatedAddrs
   }
 
+  private sortMultiaddrsByDialability (a: Address, b: Address): number {
+    if (a.lastSuccess !== undefined && b.lastSuccess !== undefined) {
+      if (a.lastSuccess > b.lastSuccess) {
+        return -1
+      } else {
+        return 1
+      }
+    }
+
+    if (a.lastFailure !== undefined && b.lastFailure !== undefined) {
+      if (a.lastFailure > b.lastFailure) {
+        return 1
+      } else {
+        return -1
+      }
+    }
+
+    if (a.lastSuccess !== undefined) {
+      return -1
+    }
+
+    if (b.lastSuccess !== undefined) {
+      return 1
+    }
+
+    if (a.lastFailure !== undefined) {
+      return 1
+    }
+
+    if (b.lastFailure !== undefined) {
+      return -1
+    }
+
+    return 0
+  }
+
   private async performDial (pendingDial: PendingDialInternal, options: DialOptions = {}): Promise<Connection> {
     const dialAbortControllers: Array<(AbortController | undefined)> = pendingDial.multiaddrs.map(() => new AbortController())
 

packages/libp2p/test/connection-manager/dial-queue.spec.ts

@@ -12,7 +12,7 @@ import { DialQueue } from '../../src/connection-manager/dial-queue.js'
 import type { Connection } from '@libp2p/interface/connection'
 import type { ConnectionGater } from '@libp2p/interface/connection-gater'
 import type { PeerId } from '@libp2p/interface/peer-id'
-import type { PeerStore } from '@libp2p/interface/peer-store'
+import type { Address, PeerStore } from '@libp2p/interface/peer-store'
 import type { Transport } from '@libp2p/interface/transport'
 import type { TransportManager } from '@libp2p/interface-internal/transport-manager'
 
@@ -330,4 +330,99 @@ describe('dial queue', () => {
 
     resolvers.delete('dnsaddr')
   })
+
+  const ensureDialOrder = async (peerId: PeerId, orderedAddresses: Address[], testType: 'lastFailure' | 'lastSuccess'): Promise<void> => {
+    components.transportManager.transportForMultiaddr.returns(stubInterface<Transport>())
+
+    const duplicatedOrderedAddresses = [...orderedAddresses]
+
+    const dialer = new DialQueue(components, {
+      maxParallelDials: 1
+    })
+
+    // reverse order of addresses
+    orderedAddresses.reverse()
+
+    const sortedAddresses = await dialer.calculateMultiaddrs(peerId, orderedAddresses)
+
+    expect(sortedAddresses.map(address => address[testType])).to.deep.equal(duplicatedOrderedAddresses.map(address => address[testType]))
+  }
+
+  it('should sort already succesfully dialled addresses by most recently sucessful', async () => {
+    const peerId = await createEd25519PeerId()
+    const orderedAddresses: Address[] = [
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1231'),
+        lastSuccess: Date.now()
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1232'),
+        lastSuccess: Date.now() - 1000
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1234'),
+        lastSuccess: Date.now() - 2000
+      }
+    ]
+
+    await ensureDialOrder(peerId, orderedAddresses, 'lastSuccess')
+  })
+
+  it('should sort addresses which failed to be dialled by least recent failure', async () => {
+    const peerId = await createEd25519PeerId()
+    const orderedAddresses: Address[] = [
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1231'),
+        lastFailure: Date.now() - 2000
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1232'),
+        lastFailure: Date.now() - 1000
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1234'),
+        lastFailure: Date.now()
+      }
+    ]
+
+    await ensureDialOrder(peerId, orderedAddresses, 'lastFailure')
+  })
+
+  it('should dial new addresses before addresses which have failed to be dialled', async () => {
+    const peerId = await createEd25519PeerId()
+    const orderedAddresses: Address[] = [
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1231')
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1232'),
+        lastFailure: Date.now() - 2000
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1234'),
+        lastFailure: Date.now() - 1000
+      }
+    ]
+
+    await ensureDialOrder(peerId, orderedAddresses, 'lastFailure')
+  })
+
+  it('should dial succesfull dialled addresses first, then new addresses, then failed addresses', async () => {
+    const peerId = await createEd25519PeerId()
+    const orderedAddresses: Address[] = [
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1231'),
+        lastSuccess: Date.now() - 5000
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1232')
+      },
+      {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1234'),
+        lastFailure: Date.now() - 6000
+      }
+    ]
+
+    await ensureDialOrder(peerId, orderedAddresses, 'lastSuccess')
+  })
 })