fix: address book should not emit peer event if no addresses are known

src/circuit/auto-relay.js

@@ -231,8 +231,7 @@ class AutoRelay {
 
     // 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())
+      await this._tryToListenOnRelay(peerId)
 
       // Check if already listening on enough relays
       if (this._listenRelays.size >= this.maxListeners) {
@@ -247,12 +246,11 @@ class AutoRelay {
         if (!provider.multiaddrs.length) {
           continue
         }
+
         const peerId = provider.id
-
         this._peerStore.addressBook.add(peerId, provider.multiaddrs)
-        const connection = await this._libp2p.dial(peerId)
 
-        await this._addListenRelay(connection, peerId.toB58String())
+        await this._tryToListenOnRelay(peerId)
 
         // Check if already listening on enough relays
         if (this._listenRelays.size >= this.maxListeners) {
@@ -263,6 +261,15 @@ class AutoRelay {
       log.error(err)
     }
   }
+
+  async _tryToListenOnRelay (peerId) {
+    try {
+      const connection = await this._libp2p.dial(peerId)
+      await this._addListenRelay(connection, peerId.toB58String())
+    } catch (err) {
+      log.error(`could not connect and listen on known hop relay ${peerId.toB58String()}`)
+    }
+  }
 }
 
 module.exports = AutoRelay

src/index.js

@@ -700,7 +700,7 @@ class Libp2p extends EventEmitter {
         try {
           await this.dialer.connectToPeer(peerId)
         } catch (err) {
-          log.error('could not connect to discovered peer', err)
+          log.error(`could not connect to discovered peer ${peerId.toB58String()} with ${err}`)
         }
       }
     }

src/peer-store/address-book.js

@@ -229,6 +229,11 @@ class AddressBook extends Book {
     const addresses = this._toAddresses(multiaddrs)
     const id = peerId.toB58String()
 
+    // Not add unavailable addresses
+    if (!addresses.length) {
+      return this
+    }
+
     const entry = this.data.get(id)
 
     if (entry && entry.addresses) {

test/peer-store/address-book.spec.js

@@ -186,6 +186,23 @@ describe('addressBook', () => {
       throw new Error('invalid multiaddr should throw error')
     })
 
+    it('does not emit event if no addresses are added', async () => {
+      const defer = pDefer()
+
+      peerStore.on('peer', () => {
+        defer.reject()
+      })
+
+      ab.add(peerId, [])
+
+      // Wait 50ms for incorrect second event
+      setTimeout(() => {
+        defer.resolve()
+      }, 50)
+
+      await defer.promise
+    })
+
     it('adds the new content and emits change event', () => {
       const defer = pDefer()
 

test/relay/auto-relay.node.js

@@ -3,6 +3,7 @@
 
 const { expect } = require('aegir/utils/chai')
 const delay = require('delay')
+const pDefer = require('p-defer')
 const pWaitFor = require('p-wait-for')
 const sinon = require('sinon')
 const nock = require('nock')
@@ -371,6 +372,50 @@ describe('auto-relay', () => {
       expect(autoRelay1._listenRelays.size).to.equal(1)
       expect(relayLibp2p1.connectionManager.size).to.eql(1)
     })
+
+    it('should not fail when trying to dial unreachable peers to add as hop relay and replaced removed ones', async () => {
+      const defer = pDefer()
+      // Spy if a connected peer is being added as listen relay
+      sinon.spy(autoRelay1, '_addListenRelay')
+      sinon.spy(relayLibp2p1.transportManager, 'listen')
+
+      // Discover one relay and connect
+      relayLibp2p1.peerStore.addressBook.add(relayLibp2p2.peerId, relayLibp2p2.multiaddrs)
+      await relayLibp2p1.dial(relayLibp2p2.peerId)
+
+      // Discover an extra relay and connect to gather its Hop support
+      relayLibp2p1.peerStore.addressBook.add(relayLibp2p3.peerId, relayLibp2p3.multiaddrs)
+      await relayLibp2p1.dial(relayLibp2p3.peerId)
+
+      // Wait for both peer to be attempted to added as listen relay
+      await pWaitFor(() => autoRelay1._addListenRelay.callCount === 2)
+      expect(autoRelay1._listenRelays.size).to.equal(1)
+      expect(relayLibp2p1.connectionManager.size).to.equal(2)
+
+      // Only one will be used for listeninng
+      expect(relayLibp2p1.transportManager.listen.callCount).to.equal(1)
+
+      // Disconnect not used listen relay
+      await relayLibp2p1.hangUp(relayLibp2p3.peerId)
+
+      expect(autoRelay1._listenRelays.size).to.equal(1)
+      expect(relayLibp2p1.connectionManager.size).to.equal(1)
+
+      // Stub dial
+      sinon.stub(relayLibp2p1, 'dial').callsFake(() => {
+        defer.resolve()
+        return Promise.reject(new Error('failed to dial'))
+      })
+
+      // Remove peer used as relay from peerStore and disconnect it
+      relayLibp2p1.peerStore.delete(relayLibp2p2.peerId)
+      await relayLibp2p1.hangUp(relayLibp2p2.peerId)
+      expect(autoRelay1._listenRelays.size).to.equal(0)
+      expect(relayLibp2p1.connectionManager.size).to.equal(0)
+
+      // Wait for failed dial
+      await defer.promise
+    })
   })
 
   describe('flows with 2 max listeners', () => {