fix(libp2p): sort addresses to dial as public, then relay (#2031)

achingbrain · web-flow · commit 5294f14caa31 · 2023-09-08T17:22:29.000+01:00
Adds a new default address sorter that sorts by:

1. public addresses
2. public relay addresess
3. private addresses
4. private relay addresses

Where they are equal, certified addresses take priority, otherwise
the sort order is stable.
diff --git a/packages/libp2p/src/config.ts b/packages/libp2p/src/config.ts
@@ -1,6 +1,6 @@
 import { CodeError } from '@libp2p/interface/errors'
 import { FaultTolerance } from '@libp2p/interface/transport'
-import { publicAddressesFirst } from '@libp2p/utils/address-sort'
+import { defaultAddressSort } from '@libp2p/utils/address-sort'
 import { dnsaddrResolver } from '@multiformats/multiaddr/resolvers'
 import mergeOptions from 'merge-options'
 import { codes, messages } from './errors.js'
@@ -19,7 +19,7 @@ const DefaultConfig: Partial<Libp2pInit> = {
     resolvers: {
       dnsaddr: dnsaddrResolver
     },
-    addressSorter: publicAddressesFirst
+    addressSorter: defaultAddressSort
   },
   transportManager: {
     faultTolerance: FaultTolerance.FATAL_ALL
diff --git a/packages/libp2p/src/connection-manager/dial-queue.ts b/packages/libp2p/src/connection-manager/dial-queue.ts
@@ -1,7 +1,7 @@
 import { setMaxListeners } from 'events'
 import { AbortError, CodeError } from '@libp2p/interface/errors'
 import { logger } from '@libp2p/logger'
-import { publicAddressesFirst } from '@libp2p/utils/address-sort'
+import { defaultAddressSort } from '@libp2p/utils/address-sort'
 import { type Multiaddr, type Resolver, resolvers } from '@multiformats/multiaddr'
 import { dnsaddrResolver } from '@multiformats/multiaddr/resolvers'
 import { type ClearableSignal, anySignal } from 'any-signal'
@@ -51,7 +51,7 @@ interface DialerInit {
 }
 
 const defaultOptions = {
-  addressSorter: publicAddressesFirst,
+  addressSorter: defaultAddressSort,
   maxParallelDials: MAX_PARALLEL_DIALS,
   maxPeerAddrsToDial: MAX_PEER_ADDRS_TO_DIAL,
   maxParallelDialsPerPeer: MAX_PARALLEL_DIALS_PER_PEER,
diff --git a/packages/libp2p/src/connection-manager/index.ts b/packages/libp2p/src/connection-manager/index.ts
@@ -2,7 +2,7 @@ import { CodeError } from '@libp2p/interface/errors'
 import { KEEP_ALIVE } from '@libp2p/interface/peer-store/tags'
 import { logger } from '@libp2p/logger'
 import { PeerMap } from '@libp2p/peer-collections'
-import { publicAddressesFirst } from '@libp2p/utils/address-sort'
+import { defaultAddressSort } from '@libp2p/utils/address-sort'
 import { type Multiaddr, type Resolver, multiaddr } from '@multiformats/multiaddr'
 import { dnsaddrResolver } from '@multiformats/multiaddr/resolvers'
 import { RateLimiterMemory } from 'rate-limiter-flexible'
@@ -254,7 +254,7 @@ export class DefaultConnectionManager implements ConnectionManager, Startable {
       transportManager: components.transportManager,
       connectionGater: components.connectionGater
     }, {
-      addressSorter: init.addressSorter ?? publicAddressesFirst,
+      addressSorter: init.addressSorter ?? defaultAddressSort,
       maxParallelDials: init.maxParallelDials ?? MAX_PARALLEL_DIALS,
       maxPeerAddrsToDial: init.maxPeerAddrsToDial ?? MAX_PEER_ADDRS_TO_DIAL,
       maxParallelDialsPerPeer: init.maxParallelDialsPerPeer ?? MAX_PARALLEL_DIALS_PER_PEER,
diff --git a/packages/libp2p/test/connection-manager/direct.spec.ts b/packages/libp2p/test/connection-manager/direct.spec.ts
@@ -8,7 +8,7 @@ import { mplex } from '@libp2p/mplex'
 import { peerIdFromString } from '@libp2p/peer-id'
 import { createEd25519PeerId } from '@libp2p/peer-id-factory'
 import { PersistentPeerStore } from '@libp2p/peer-store'
-import { publicAddressesFirst } from '@libp2p/utils/address-sort'
+import { defaultAddressSort } from '@libp2p/utils/address-sort'
 import { webSockets } from '@libp2p/websockets'
 import * as filters from '@libp2p/websockets/filters'
 import { multiaddr } from '@multiformats/multiaddr'
@@ -193,11 +193,11 @@ describe('dialing (direct, WebSockets)', () => {
       multiaddr('/ip4/30.0.0.1/tcp/15001/ws')
     ]
 
-    const publicAddressesFirstSpy = sinon.spy(publicAddressesFirst)
+    const addressesSorttSpy = sinon.spy(defaultAddressSort)
     const localTMDialStub = sinon.stub(localTM, 'dial').callsFake(async (ma) => mockConnection(mockMultiaddrConnection(mockDuplex(), remoteComponents.peerId)))
 
     connectionManager = new DefaultConnectionManager(localComponents, {
-      addressSorter: publicAddressesFirstSpy,
+      addressSorter: addressesSorttSpy,
       maxParallelDials: 3,
       maxParallelDialsPerPeer: 3
     })
@@ -213,7 +213,7 @@ describe('dialing (direct, WebSockets)', () => {
 
     const sortedAddresses = peerMultiaddrs
       .map((m) => ({ multiaddr: m, isCertified: false }))
-      .sort(publicAddressesFirst)
+      .sort(defaultAddressSort)
 
     expect(localTMDialStub.getCall(0).args[0].equals(sortedAddresses[0].multiaddr))
     expect(localTMDialStub.getCall(1).args[0].equals(sortedAddresses[1].multiaddr))
diff --git a/packages/utils/package.json b/packages/utils/package.json
@@ -89,6 +89,7 @@
     "@libp2p/interface": "^0.1.2",
     "@libp2p/logger": "^3.0.2",
     "@multiformats/multiaddr": "^12.1.5",
+    "@multiformats/multiaddr-matcher": "^1.0.1",
     "is-loopback-addr": "^2.0.1",
     "it-stream-types": "^2.0.1",
     "private-ip": "^3.0.0",
diff --git a/packages/utils/src/address-sort.ts b/packages/utils/src/address-sort.ts
@@ -20,13 +20,13 @@
  * ```
  */
 
+import { Circuit } from '@multiformats/multiaddr-matcher'
 import { isPrivate } from './multiaddr/is-private.js'
 import type { Address } from '@libp2p/interface/peer-store'
 
 /**
- * Compare function for array.sort().
- * This sort aims to move the private addresses to the end of the array.
- * In case of equality, a certified address will come first.
+ * Compare function for array.sort() that moves public addresses to the start
+ * of the array.
  */
 export function publicAddressesFirst (a: Address, b: Address): -1 | 0 | 1 {
   const isAPrivate = isPrivate(a.multiaddr)
@@ -37,7 +37,15 @@ export function publicAddressesFirst (a: Address, b: Address): -1 | 0 | 1 {
   } else if (!isAPrivate && isBPrivate) {
     return -1
   }
-  // Check certified?
+
+  return 0
+}
+
+/**
+ * Compare function for array.sort() that moves certified addresses to the start
+ * of the array.
+ */
+export function certifiedAddressesFirst (a: Address, b: Address): -1 | 0 | 1 {
   if (a.isCertified && !b.isCertified) {
     return -1
   } else if (!a.isCertified && b.isCertified) {
@@ -48,8 +56,36 @@ export function publicAddressesFirst (a: Address, b: Address): -1 | 0 | 1 {
 }
 
 /**
- * A test thing
+ * Compare function for array.sort() that moves circuit relay addresses to the
+ * start of the array.
  */
-export async function something (): Promise<Uint8Array> {
-  return Uint8Array.from([0, 1, 2])
+export function circuitRelayAddressesLast (a: Address, b: Address): -1 | 0 | 1 {
+  const isACircuit = Circuit.exactMatch(a.multiaddr)
+  const isBCircuit = Circuit.exactMatch(b.multiaddr)
+
+  if (isACircuit && !isBCircuit) {
+    return 1
+  } else if (!isACircuit && isBCircuit) {
+    return -1
+  }
+
+  return 0
+}
+
+export function defaultAddressSort (a: Address, b: Address): -1 | 0 | 1 {
+  const publicResult = publicAddressesFirst(a, b)
+
+  if (publicResult !== 0) {
+    return publicResult
+  }
+
+  const relayResult = circuitRelayAddressesLast(a, b)
+
+  if (relayResult !== 0) {
+    return relayResult
+  }
+
+  const certifiedResult = certifiedAddressesFirst(a, b)
+
+  return certifiedResult
 }
diff --git a/packages/utils/test/address-sort.spec.ts b/packages/utils/test/address-sort.spec.ts
@@ -2,50 +2,176 @@
 
 import { multiaddr } from '@multiformats/multiaddr'
 import { expect } from 'aegir/chai'
-import { publicAddressesFirst } from '../src/address-sort.js'
+import { publicAddressesFirst, certifiedAddressesFirst, circuitRelayAddressesLast, defaultAddressSort } from '../src/address-sort.js'
 
 describe('address-sort', () => {
-  it('should sort public addresses first', () => {
-    const addresses = [
-      {
+  describe('public addresses first', () => {
+    it('should sort public addresses first', () => {
+      const publicAddress = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000'),
+        isCertified: false
+      }
+      const privateAddress = {
         multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4000'),
         isCertified: false
-      },
-      {
+      }
+
+      const addresses = [
+        privateAddress,
+        publicAddress
+      ]
+
+      const sortedAddresses = addresses.sort(publicAddressesFirst)
+      expect(sortedAddresses).to.deep.equal([
+        publicAddress,
+        privateAddress
+      ])
+    })
+  })
+
+  describe('certified addresses first', () => {
+    it('should sort certified addresses first', () => {
+      const certifiedPublicAddress = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4001'),
+        isCertified: true
+      }
+      const publicAddress = {
         multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000'),
         isCertified: false
-      },
-      {
-        multiaddr: multiaddr('/ip4/31.0.0.1/tcp/4000'),
+      }
+      const certifiedPrivateAddress = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4001'),
+        isCertified: true
+      }
+      const privateAddress = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4000'),
         isCertified: false
       }
-    ]
 
-    const sortedAddresses = addresses.sort(publicAddressesFirst)
-    expect(sortedAddresses[0].multiaddr.equals(multiaddr('/ip4/30.0.0.1/tcp/4000'))).to.eql(true)
-    expect(sortedAddresses[1].multiaddr.equals(multiaddr('/ip4/31.0.0.1/tcp/4000'))).to.eql(true)
-    expect(sortedAddresses[2].multiaddr.equals(multiaddr('/ip4/127.0.0.1/tcp/4000'))).to.eql(true)
+      const addresses = [
+        publicAddress,
+        certifiedPublicAddress,
+        certifiedPrivateAddress,
+        privateAddress
+      ]
+
+      const sortedAddresses = addresses.sort(certifiedAddressesFirst)
+      expect(sortedAddresses).to.deep.equal([
+        certifiedPublicAddress,
+        certifiedPrivateAddress,
+        publicAddress,
+        privateAddress
+      ])
+    })
   })
 
-  it('should sort public certified addresses first', () => {
-    const addresses = [
-      {
-        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4000'),
+  describe('circuit relay addresses last', () => {
+    it('should sort circuit relay addresses last', () => {
+      const publicAddress = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000'),
         isCertified: false
-      },
-      {
+      }
+      const publicRelay = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: false
+      }
+
+      const addresses = [
+        publicRelay,
+        publicAddress
+      ]
+
+      const sortedAddresses = addresses.sort(circuitRelayAddressesLast)
+      expect(sortedAddresses).to.deep.equal([
+        publicAddress,
+        publicRelay
+      ])
+    })
+  })
+
+  describe('default address sort', () => {
+    it('should sort public, then public relay, then private, then private relay with certified addresses taking priority', () => {
+      const certifiedPublicAddress = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4001'),
+        isCertified: true
+      }
+      const publicAddress = {
         multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000'),
         isCertified: false
-      },
-      {
-        multiaddr: multiaddr('/ip4/31.0.0.1/tcp/4000'),
+      }
+      const certifiedPublicRelay = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4001/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: true
+      }
+      const publicRelay = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: false
+      }
+      const certifiedPrivateAddress = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4001'),
         isCertified: true
       }
-    ]
+      const privateAddress = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4000'),
+        isCertified: false
+      }
+      const certifiedPrivateRelay = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4001/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: true
+      }
+      const privateRelay = {
+        multiaddr: multiaddr('/ip4/127.0.0.1/tcp/4000/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: false
+      }
+
+      const addresses = [
+        privateAddress,
+        certifiedPrivateAddress,
+        publicRelay,
+        certifiedPublicRelay,
+        privateRelay,
+        publicAddress,
+        certifiedPublicAddress,
+        certifiedPrivateRelay
+      ].sort(() => {
+        return Math.random() > 0.5 ? -1 : 1
+      })
+
+      const sortedAddresses = addresses.sort(defaultAddressSort)
+      expect(sortedAddresses).to.deep.equal([
+        certifiedPublicAddress,
+        publicAddress,
+        certifiedPublicRelay,
+        publicRelay,
+        certifiedPrivateAddress,
+        privateAddress,
+        certifiedPrivateRelay,
+        privateRelay
+      ])
+    })
+
+    it('should sort WebRTC over relay addresses before relay addresses', () => {
+      const publicRelay = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm'),
+        isCertified: false
+      }
+      const webRTCOverRelay = {
+        multiaddr: multiaddr('/ip4/30.0.0.1/tcp/4000/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN/p2p-circuit/p2p/QmcrQZ6RJdpYuGvZqD5QEHAv6qX4BrQLJLQPQUrTrzdcgm/webrtc'),
+        isCertified: false
+      }
+
+      const addresses = [
+        publicRelay,
+        webRTCOverRelay
+      ].sort(() => {
+        return Math.random() > 0.5 ? -1 : 1
+      })
 
-    const sortedAddresses = addresses.sort(publicAddressesFirst)
-    expect(sortedAddresses[0].multiaddr.equals(multiaddr('/ip4/31.0.0.1/tcp/4000'))).to.eql(true)
-    expect(sortedAddresses[1].multiaddr.equals(multiaddr('/ip4/30.0.0.1/tcp/4000'))).to.eql(true)
-    expect(sortedAddresses[2].multiaddr.equals(multiaddr('/ip4/127.0.0.1/tcp/4000'))).to.eql(true)
+      const sortedAddresses = addresses.sort(defaultAddressSort)
+      expect(sortedAddresses).to.deep.equal([
+        webRTCOverRelay,
+        publicRelay
+      ])
+    })
   })
 })
