From 06e8f3dd42432e4b37ab7904b02abde7d1cadda3 Mon Sep 17 00:00:00 2001
From: Vasco Santos <vasco.santos@moxy.studio>
Date: Tue, 20 Apr 2021 09:31:16 +0200
Subject: [PATCH] fix: do not add abort signals to useless addresses (#913)

---
 src/dialer/index.js         |  8 ++++++--
 test/core/listening.node.js |  2 +-
 test/dialing/direct.node.js | 28 ++++++++++++++++++++++++----
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/dialer/index.js b/src/dialer/index.js
index 1225e1cc28..31919a1f44 100644
--- a/src/dialer/index.js
+++ b/src/dialer/index.js
@@ -110,7 +110,7 @@ class Dialer {
     const dialTarget = await this._createDialTarget(peer)
 
     if (!dialTarget.addrs.length) {
-      throw errCode(new Error('The dial request has no addresses'), codes.ERR_NO_VALID_ADDRESSES)
+      throw errCode(new Error('The dial request has no valid addresses'), codes.ERR_NO_VALID_ADDRESSES)
     }
     const pendingDial = this._pendingDials.get(dialTarget.id) || this._createPendingDial(dialTarget, options)
 
@@ -134,6 +134,7 @@ class Dialer {
    * Creates a DialTarget. The DialTarget is used to create and track
    * the DialRequest to a given peer.
    * If a multiaddr is received it should be the first address attempted.
+   * Multiaddrs not supported by the available transports will be filtered out.
    *
    * @private
    * @param {PeerId|Multiaddr|string} peer - A PeerId or Multiaddr
@@ -162,9 +163,12 @@ class Dialer {
       resolvedAddrs.forEach(ra => addrs.push(ra))
     }
 
+    // Multiaddrs not supported by the available transports will be filtered out.
+    const supportedAddrs = addrs.filter(a => this.transportManager.transportForMultiaddr(a))
+
     return {
       id: id.toB58String(),
-      addrs
+      addrs: supportedAddrs
     }
   }
 
diff --git a/test/core/listening.node.js b/test/core/listening.node.js
index fe202b1ab1..7e948ca869 100644
--- a/test/core/listening.node.js
+++ b/test/core/listening.node.js
@@ -45,7 +45,7 @@ describe('Listening', () => {
     expect(addrs.length).to.be.at.least(2)
     for (const addr of addrs) {
       const opts = addr.toOptions()
-      expect(opts.family).to.equal('ipv4')
+      expect(opts.family).to.equal(4)
       expect(opts.transport).to.equal('tcp')
       expect(opts.host).to.match(/[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/)
       expect(opts.port).to.be.gt(0)
diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js
index 1a1b508929..7750a35773 100644
--- a/test/dialing/direct.node.js
+++ b/test/dialing/direct.node.js
@@ -98,8 +98,8 @@ describe('Dialing (direct, TCP)', () => {
     const dialer = new Dialer({ transportManager: localTM, peerStore })
 
     await expect(dialer.connectToPeer(unsupportedAddr))
-      .to.eventually.be.rejectedWith(AggregateError)
-      .and.to.have.nested.property('._errors[0].code', ErrorCodes.ERR_TRANSPORT_UNAVAILABLE)
+      .to.eventually.be.rejectedWith(Error)
+      .and.to.have.nested.property('.code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
   })
 
   it('should fail to connect if peer has no known addresses', async () => {
@@ -139,8 +139,28 @@ describe('Dialing (direct, TCP)', () => {
     const peerId = await PeerId.createFromJSON(Peers[0])
 
     await expect(dialer.connectToPeer(peerId))
-      .to.eventually.be.rejectedWith(AggregateError)
-      .and.to.have.nested.property('._errors[0].code', ErrorCodes.ERR_TRANSPORT_UNAVAILABLE)
+      .to.eventually.be.rejectedWith(Error)
+      .and.to.have.nested.property('.code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
+  })
+
+  it('should only try to connect to addresses supported by the transports configured', async () => {
+    const remoteAddrs = remoteTM.getAddrs()
+    const dialer = new Dialer({
+      transportManager: localTM,
+      peerStore: {
+        addressBook: {
+          add: () => { },
+          getMultiaddrsForPeer: () => [...remoteAddrs, unsupportedAddr]
+        }
+      }
+    })
+    const peerId = await PeerId.createFromJSON(Peers[0])
+
+    sinon.spy(localTM, 'dial')
+    const connection = await dialer.connectToPeer(peerId)
+    expect(localTM.dial.callCount).to.equal(remoteAddrs.length)
+    expect(connection).to.exist()
+    await connection.close()
   })
 
   it('should abort dials on queue task timeout', async () => {