chore: improve _listenOnAvailableHopRelays logic

src/circuit/auto-relay.js

@@ -7,6 +7,7 @@ log.error = debug('libp2p:auto-relay:error')
 const uint8ArrayFromString = require('uint8arrays/from-string')
 const uint8ArrayToString = require('uint8arrays/to-string')
 const multiaddr = require('multiaddr')
+const PeerId = require('peer-id')
 
 const { relay: multicodec } = require('./multicodec')
 const { canHop } = require('./circuit/hop')
@@ -78,10 +79,12 @@ class AutoRelay {
         return
       }
 
-      await canHop({ connection })
+      const supportsHop = await canHop({ connection })
 
-      this._peerStore.metadataBook.set(peerId, hopMetadataKey, uint8ArrayFromString(hopMetadataValue))
-      await this._addListenRelay(connection, id)
+      if (supportsHop) {
+        this._peerStore.metadataBook.set(peerId, hopMetadataKey, uint8ArrayFromString(hopMetadataValue))
+        await this._addListenRelay(connection, id)
+      }
     } catch (err) {
       log.error(err)
     }
@@ -117,21 +120,30 @@ class AutoRelay {
       return
     }
 
-    this._listenRelays.add(id)
-
     // Create relay listen addr
-    const remoteMultiaddr = connection.remoteAddr
-    let listenAddr
+    let listenAddr, remoteMultiaddr
+
+    try {
+      const remoteAddrs = this._peerStore.addressBook.get(connection.remotePeer)
+      remoteMultiaddr = remoteAddrs.find(a => a.isCertified).multiaddr // Get first announced address certified
+    } catch (_) {
+      log.error(`${id} does not have announced certified multiaddrs`)
+      return
+    }
 
     if (!remoteMultiaddr.protoNames().includes('p2p')) {
-      listenAddr = `${remoteMultiaddr.toString()}/p2p/${connection.remotePeer.toB58String()}/p2p-circuit/p2p/${this._peerId.toB58String()}`
+      listenAddr = `${remoteMultiaddr.toString()}/p2p/${connection.remotePeer.toB58String()}/p2p-circuit`
     } else {
-      listenAddr = `${remoteMultiaddr.toString()}/p2p-circuit/p2p/${this._peerId.toB58String()}`
+      listenAddr = `${remoteMultiaddr.toString()}/p2p-circuit`
     }
 
     // Attempt to listen on relay
+    this._listenRelays.add(id)
+
     try {
       await this._transportManager.listen([multiaddr(listenAddr)])
+      // TODO: push announce multiaddrs update
+      // await this._libp2p.identifyService.pushToPeerStore()
     } catch (err) {
       log.error(err)
       this._listenRelays.delete(id)
@@ -145,45 +157,73 @@ class AutoRelay {
    * @return {void}
    */
   _removeListenRelay (id) {
-    this._listenRelays.delete(id)
-    // TODO: this should be responsibility of the connMgr
-    this._listenOnAvailableHopRelays()
+    if (this._listenRelays.delete(id)) {
+      // TODO: this should be responsibility of the connMgr
+      this._listenOnAvailableHopRelays([id])
+    }
   }
 
   /**
    * Try to listen on available hop relay connections.
+   * The following order will happen while we do not have enough relays.
+   * 1. Check the metadata store for known relays, try to listen on the ones we are already connected.
+   * 2. Dial and try to listen on the peers we know that support hop but are not connected.
+   * 3. Search the network.
+   * @param {Array<string>} [peersToIgnore]
    * @return {Promise<void>}
    */
-  async _listenOnAvailableHopRelays () {
+  async _listenOnAvailableHopRelays (peersToIgnore = []) {
+    // TODO: The peer redial issue on disconnect should be handled by connection gating
     // Check if already listening on enough relays
     if (this._listenRelays.size >= this.maxListeners) {
       return
     }
 
-    // Verify if there are available connections to hop
-    for (const [connection] of this._connectionManager.connections.values()) {
-      const peerId = connection.remotePeer
-      const id = peerId.toB58String()
+    const knownHopsToDial = []
 
-      // Continue to next if listening on this
-      if (this._listenRelays.has(id)) {
+    // Check if we have known hop peers to use and attempt to listen on the already connected
+    for (const [id, metadataMap] of this._peerStore.metadataBook.data.entries()) {
+      // Continue to next if listening on this or peer to ignore
+      if (this._listenRelays.has(id) || peersToIgnore.includes(id)) {
         continue
       }
 
-      const supportsHop = this._peerStore.metadataBook.getValue(peerId, hopMetadataKey)
+      const supportsHop = metadataMap.get(hopMetadataKey)
 
       // Continue to next if it does not support Hop
       if (!supportsHop || uint8ArrayToString(supportsHop) !== hopMetadataValue) {
         continue
       }
 
+      const peerId = PeerId.createFromCID(id)
+      const connection = this._connectionManager.get(peerId)
+
+      // If not connected, store for possible later use.
+      if (!connection) {
+        knownHopsToDial.push(peerId)
+        continue
+      }
+
       await this._addListenRelay(connection, id)
 
       // Check if already listening on enough relays
       if (this._listenRelays.size >= this.maxListeners) {
-        break
+        return
+      }
+    }
+
+    // Try to listen on known peers that are not connected
+    for (const peerId of knownHopsToDial) {
+      const connection = await this._libp2p.dial(peerId)
+      await this._addListenRelay(connection, peerId.toB58String())
+
+      // Check if already listening on enough relays
+      if (this._listenRelays.size >= this.maxListeners) {
+        return
       }
     }
+
+    // TODO: Try to find relays to hop on the network
   }
 }
 

src/circuit/circuit/hop.js

@@ -121,7 +121,7 @@ module.exports.hop = async function hop ({
  * Performs a CAN_HOP request to a relay peer, in order to understand its capabilities.
  * @param {object} options
  * @param {Connection} options.connection Connection to the relay
- * @returns {Promise<void>}
+ * @returns {Promise<boolean>}
  */
 module.exports.canHop = async function canHop ({
   connection
@@ -138,8 +138,10 @@ module.exports.canHop = async function canHop ({
   await streamHandler.close()
 
   if (response.code !== CircuitPB.Status.SUCCESS) {
-    throw errCode(new Error('Relay is not able to hop'), Errors.ERR_CANNOT_HOP)
+    return false
   }
+
+  return true
 }
 
 /**

src/circuit/listener.js

@@ -17,7 +17,12 @@ module.exports = (libp2p) => {
 
   // Remove listeningAddrs when a peer disconnects
   libp2p.connectionManager.on('peer:disconnect', (connection) => {
-    listeningAddrs.delete(connection.remotePeer.toB58String())
+    const deleted = listeningAddrs.delete(connection.remotePeer.toB58String())
+
+    if (deleted) {
+      // TODO push announce multiaddrs update
+      // libp2p.identifyService.pushToPeerStore()
+    }
   })
 
   /**

src/errors.js

@@ -19,7 +19,6 @@ exports.codes = {
   ERR_DISCOVERED_SELF: 'ERR_DISCOVERED_SELF',
   ERR_DUPLICATE_TRANSPORT: 'ERR_DUPLICATE_TRANSPORT',
   ERR_ENCRYPTION_FAILED: 'ERR_ENCRYPTION_FAILED',
-  ERR_CANNOT_HOP: 'ERR_CANNOT_HOP',
   ERR_HOP_REQUEST_FAILED: 'ERR_HOP_REQUEST_FAILED',
   ERR_INVALID_KEY: 'ERR_INVALID_KEY',
   ERR_INVALID_MESSAGE: 'ERR_INVALID_MESSAGE',

src/identify/index.js

@@ -119,12 +119,12 @@ class IdentifyService {
 
   /**
    * Calls `push` for all peers in the `peerStore` that are connected
-   * @param {PeerStore} peerStore
+   * @returns {void}
    */
-  pushToPeerStore (peerStore) {
+  pushToPeerStore () {
     const connections = []
     let connection
-    for (const peer of peerStore.peers.values()) {
+    for (const peer of this.peerStore.peers.values()) {
       if (peer.protocols.includes(MULTICODEC_IDENTIFY_PUSH) && (connection = this.connectionManager.get(peer.id))) {
         connections.push(connection)
       }

src/index.js

@@ -424,7 +424,7 @@ class Libp2p extends EventEmitter {
 
     // Only push if libp2p is running
     if (this.isStarted() && this.identifyService) {
-      this.identifyService.pushToPeerStore(this.peerStore)
+      this.identifyService.pushToPeerStore()
     }
   }
 
@@ -441,7 +441,7 @@ class Libp2p extends EventEmitter {
 
     // Only push if libp2p is running
     if (this.isStarted() && this.identifyService) {
-      this.identifyService.pushToPeerStore(this.peerStore)
+      this.identifyService.pushToPeerStore()
     }
   }
 

src/peer-store/address-book.js

@@ -260,7 +260,7 @@ class AddressBook extends Book {
    * Get the known data of a provided peer.
    * @override
    * @param {PeerId} peerId
-   * @returns {Array<data>}
+   * @returns {Array<Address>|undefined}
    */
   get (peerId) {
     if (!PeerId.isPeerId(peerId)) {