From b4fb9b7bf266ba03c4462c0a41b1c2691e4e88d4 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Fri, 12 Feb 2021 11:22:11 +0100 Subject: [PATCH] fix: address book should not emit peer event if no addresses are known --- src/circuit/auto-relay.js | 17 +++++++---- src/index.js | 2 +- src/peer-store/address-book.js | 5 ++++ test/peer-store/address-book.spec.js | 17 +++++++++++ test/relay/auto-relay.node.js | 45 ++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/circuit/auto-relay.js b/src/circuit/auto-relay.js index a99bb14d32..c5c5d3e959 100644 --- a/src/circuit/auto-relay.js +++ b/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 + 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 diff --git a/src/index.js b/src/index.js index 02fa4f4ed5..4494ebac5d 100644 --- a/src/index.js +++ b/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}`) } } } diff --git a/src/peer-store/address-book.js b/src/peer-store/address-book.js index 595781c068..8b94acc584 100644 --- a/src/peer-store/address-book.js +++ b/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) { diff --git a/test/peer-store/address-book.spec.js b/test/peer-store/address-book.spec.js index 48eb7180bc..512fc8fd35 100644 --- a/test/peer-store/address-book.spec.js +++ b/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() diff --git a/test/relay/auto-relay.node.js b/test/relay/auto-relay.node.js index a254b0475d..4f0aa05cc7 100644 --- a/test/relay/auto-relay.node.js +++ b/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', () => {