chore: address review

Author: vasco-santos

package.json

@@ -64,7 +64,7 @@
     "mafmt": "^8.0.0",
     "merge-options": "^2.0.0",
     "moving-average": "^1.0.0",
-    "multiaddr": "multiformats/js-multiaddr#feat/resolve-multiaddrs",
+    "multiaddr": "^8.1.0",
     "multicodec": "^2.0.0",
     "multistream-select": "^1.0.0",
     "mutable-proxy": "^1.0.0",

src/dialer/index.js

@@ -18,9 +18,6 @@ const {
   MAX_PER_PEER_DIALS
 } = require('../constants')
 
-const dns4Code = 54
-const dns6Code = 55
-
 class Dialer {
   /**
    * @class
@@ -45,9 +42,12 @@ class Dialer {
     this.concurrency = concurrency
     this.timeout = timeout
     this.perPeerLimit = perPeerLimit
-    this.resolvers = resolvers
     this.tokens = [...new Array(concurrency)].map((_, index) => index)
     this._pendingDials = new Map()
+
+    for (const [key, value] of Object.entries(resolvers)) {
+      multiaddr.resolvers.set(key, value)
+    }
   }
 
   /**
@@ -211,10 +211,11 @@ class Dialer {
    */
   async _resolve (ma) {
     // TODO: recursive logic should live in multiaddr once dns4/dns6 support is in place
-    const resolvableProto = ma.protos().find((p) => p.resolvable)
+    // Now only supporting resolve for dnsaddr
+    const resolvableProto = ma.protoNames().includes('dnsaddr')
 
-    // Multiaddr is not resolvable (including exception for dns4/dns6)? End recursion!
-    if (!resolvableProto || resolvableProto.code === dns4Code || resolvableProto === dns6Code) {
+    // Multiaddr is not resolvable? End recursion!
+    if (!resolvableProto) {
       return [ma]
     }
 
@@ -232,17 +233,19 @@ class Dialer {
   }
 
   /**
-   * Add dialer resolvers to multiaddr and resolve multiaddr.
+   * Resolve a given multiaddr. If this fails, an empty array will be returned
    *
    * @param {Multiaddr} ma
    * @returns {Promise<Array<Multiaddr>>}
    */
-  _resolveRecord (ma) {
-    for (const [key, value] of Object.entries(this.resolvers)) {
-      ma.resolvers.set(key, value)
+  async _resolveRecord (ma) {
+    try {
+      const multiaddrs = await ma.resolve()
+      return multiaddrs
+    } catch (_) {
+      log.error(`multiaddr ${ma} could not be resolved`)
+      return []
     }
-
-    return ma.resolve()
   }
 }
 

test/dialing/resolver.spec.js

@@ -7,6 +7,8 @@ const sinon = require('sinon')
 const multiaddr = require('multiaddr')
 const { Resolver } = require('multiaddr/src/resolvers/dns')
 
+const { codes: ErrorCodes } = require('../../src/errors')
+
 const peerUtils = require('../utils/creators/peer')
 const baseOptions = require('../utils/base-options.browser')
 
@@ -88,10 +90,10 @@ describe('Dialing (resolvable addresses)', () => {
 
     // Resolver stub
     const stub = sinon.stub(Resolver.prototype, 'resolveTxt')
-    let alreadyCalled = false
+    let firstCall = false
     stub.callsFake(() => {
-      if (!alreadyCalled) {
-        alreadyCalled = true
+      if (!firstCall) {
+        firstCall = true
         // Return an array of dnsaddr
         return Promise.resolve(getDnsaddrStub(remoteId))
       }
@@ -108,6 +110,7 @@ describe('Dialing (resolvable addresses)', () => {
   })
 
   // TODO: Temporary solution does not resolve dns4/dns6
+  // Resolver just returns the received multiaddrs
   it('stops recursive resolve if finds dns4/dns6 and dials it', async () => {
     const remoteId = remoteLibp2p.peerId.toB58String()
     const dialAddr = multiaddr(`/dnsaddr/remote.libp2p.io/p2p/${remoteId}`)
@@ -128,4 +131,58 @@ describe('Dialing (resolvable addresses)', () => {
 
     await libp2p.dial(dialAddr)
   })
+
+  it('resolves a dnsaddr recursively not failing if one address fails to resolve', async () => {
+    const remoteId = remoteLibp2p.peerId.toB58String()
+    const dialAddr = multiaddr(`/dnsaddr/remote.libp2p.io/p2p/${remoteId}`)
+    const relayedAddrFetched = multiaddr(relayedAddr(remoteId))
+
+    // Transport spy
+    const transport = libp2p.transportManager._transports.get('Circuit')
+    sinon.spy(transport, 'dial')
+
+    // Resolver stub
+    let firstCall = false
+    let secondCall = false
+
+    const stub = sinon.stub(Resolver.prototype, 'resolveTxt')
+    stub.callsFake(() => {
+      if (!firstCall) {
+        firstCall = true
+        // Return an array of dnsaddr
+        return Promise.resolve(getDnsaddrStub(remoteId))
+      } else if (!secondCall) {
+        secondCall = true
+        // Address failed to resolve
+        return Promise.reject(new Error())
+      }
+      return Promise.resolve(getDnsRelayedAddrStub(remoteId))
+    })
+
+    // Dial with address resolve
+    const connection = await libp2p.dial(dialAddr)
+    expect(connection).to.exist()
+    expect(connection.remoteAddr.equals(relayedAddrFetched))
+
+    const dialArgs = transport.dial.firstCall.args
+    expect(dialArgs[0].equals(relayedAddrFetched)).to.eql(true)
+  })
+
+  it('fails to dial if resolve fails and there are no addresses to dial', async () => {
+    const remoteId = remoteLibp2p.peerId.toB58String()
+    const dialAddr = multiaddr(`/dnsaddr/remote.libp2p.io/p2p/${remoteId}`)
+
+    // Stub resolver
+    const stubResolve = sinon.stub(Resolver.prototype, 'resolveTxt')
+    stubResolve.returns(Promise.reject(new Error()))
+
+    // Stub transport
+    const transport = libp2p.transportManager._transports.get('WebSockets')
+    const spy = sinon.spy(transport, 'dial')
+
+    await expect(libp2p.dial(dialAddr))
+      .to.eventually.be.rejectedWith(Error)
+      .and.to.have.nested.property('.code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
+    expect(spy.callCount).to.eql(0)
+  })
 })