Skip to content

Commit

Permalink
fix: try all peer addresses when dialing a relay (#1140)
Browse files Browse the repository at this point in the history
The order of the addresses can affect our success rate in dialing a
relay - if it's a loopback address or similar it won't work.

Instead try dialing every address.
  • Loading branch information
achingbrain authored Jan 21, 2022
1 parent b7e8706 commit 63aa480
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 16 deletions.
25 changes: 16 additions & 9 deletions src/circuit/auto-relay.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,24 @@ class AutoRelay {
connection.remotePeer, this._addressSorter
)

if (!remoteAddrs || !remoteAddrs.length) {
return
}

const listenAddr = `${remoteAddrs[0].toString()}/p2p-circuit`
this._listenRelays.add(id)

// Attempt to listen on relay
await this._transportManager.listen([new Multiaddr(listenAddr)])
const result = await Promise.all(
remoteAddrs.map(async addr => {
try {
// Announce multiaddrs will update on listen success by TransportManager event being triggered
await this._transportManager.listen([new Multiaddr(`${addr.toString()}/p2p-circuit`)])
return true
} catch (/** @type {any} */ err) {
this._onError(err)
}

return false
})
)

// Announce multiaddrs will update on listen success by TransportManager event being triggered
if (result.includes(true)) {
this._listenRelays.add(id)
}
} catch (/** @type {any} */ err) {
this._onError(err)
this._listenRelays.delete(id)
Expand Down
3 changes: 3 additions & 0 deletions src/connection-manager/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ class ConnectionManager extends EventEmitter {
latencyCheckIntervalMs: this._options.pollInterval,
dataEmitIntervalMs: this._options.pollInterval
})

// This emitter gets listened to a lot
this.setMaxListeners(Infinity)
}

/**
Expand Down
13 changes: 6 additions & 7 deletions test/relay/auto-relay.node.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('auto-relay', () => {
expect(knownProtocols3).to.include(relayMulticodec)
})

it('should not listen on a relayed address if peer disconnects', async () => {
it('should not listen on a relayed address we disconnect from peer', async () => {
// Spy if identify push is fired on adding/removing listen addr
sinon.spy(relayLibp2p1.identifyService, 'pushToPeerStore')

Expand All @@ -236,19 +236,13 @@ describe('auto-relay', () => {
// Wait for listening on the relay
await usingAsRelay(relayLibp2p1, relayLibp2p2)

// Identify push for adding listen relay multiaddr
expect(relayLibp2p1.identifyService.pushToPeerStore.callCount).to.equal(1)

// Disconnect from peer used for relay
await relayLibp2p1.hangUp(relayLibp2p2.peerId)

// Wait for removed listening on the relay
await expect(usingAsRelay(relayLibp2p1, relayLibp2p2, {
timeout: 1000
})).to.eventually.be.rejected()

// Identify push for removing listen relay multiaddr
await pWaitFor(() => relayLibp2p1.identifyService.pushToPeerStore.callCount === 2)
})

it('should try to listen on other connected peers relayed address if one used relay disconnects', async () => {
Expand All @@ -271,6 +265,11 @@ describe('auto-relay', () => {
// Disconnect from peer used for relay
await relayLibp2p2.stop()

// Should not be using the relay any more
await expect(usingAsRelay(relayLibp2p1, relayLibp2p2, {
timeout: 1000
})).to.eventually.be.rejected()

// Wait for other peer connected to be added as listen addr
await usingAsRelay(relayLibp2p1, relayLibp2p3)
})
Expand Down

0 comments on commit 63aa480

Please sign in to comment.