fix support for dialing /p2p-circuit/p2p/<peer>

[jacobheun]

• Version: 0.29
• Platform: *
• Subsystem: Relay

Type: Bug

Severity: High

Description:

The following dial will throw an error Error: Invalid version, must be a number equal to 1 or 0.

const connection = await libp2p.dial('/p2p-circuit/p2p/<peer>')

This circuit dial behavior is currently broken. Providing this address should attempt to connect to the given peer over any connected relays.

[imestin]

Sometimes this.version will be null in /cids/src/index.js line 255.
Also in /cids/src/cid-util.js other.version is null.

Can be reproduced with this code:

'use strict'

const Libp2p = require('libp2p')
const TCP = require('libp2p-tcp')
const { NOISE } = require('libp2p-noise')

const createNode = async () => {
  const node = await Libp2p.create({
    addresses: {
      listen: ['/ip4/0.0.0.0/tcp/0']
    },
    modules: {
      transport: [ TCP ],
      connEncryption: [ NOISE ]
    }
  })

  await node.start()
  return node
}

(async () => {
    const node = await createNode();
    const connection = await node.dial('/p2p-circuit/p2p/Qmahcv1WK5W4oiSN6aUNd9YqB4affJZXWNWbwUXrGKGfJc')
    console.log('listening on:')
    node.multiaddrs.forEach((ma) => console.log(`${ma.toString()}/p2p/${node.peerId.toB58String()}`))
})();

[imestin]

In /cids/src/index.js at line 71, _CID.isCID(version) is giving false for Qmahcv1WK5W4oiSN6aUNd9YqB4affJZXWNWbwUXrGKGfJc

[jacobheun]

In /cids/src/index.js at line 71, _CID.isCID(version) is giving false for Qmahcv1WK5W4oiSN6aUNd9YqB4affJZXWNWbwUXrGKGfJc

This is due to how the Circuit is currently handling the dialing address https://github.com/libp2p/js-libp2p/blob/v0.29.1/src/circuit/index.js#L104-L108. There's no relay peer so it's not passing a valid PeerId when trying to create it.

Since there's no relay address it's failing. It should detect there is no relay peer in the multiaddr and then start dialing via the relays its connected to. If that all fails, we should likely just attempt to dial with the PeerId, in case we already have a valid address for the target peer.

[imestin]

I don't understand libp2p that much yet.

By PeerId, do you mean this.peerId (that is, the PeerID of the class) And how to access relays it's already connected to? I'm guessing something like this._connectionManager._libp2p.peerStore.addressBook but I can't really figure it out.

[mpetrunic]

@jacobheun @lidel I'm thinking of taking this on but wanted to check first. So to do this, when there is no relay specified, I'm supposed to send a CAN_HOP request to every connection in the connection manager to try to discover relay?

Should we prioritize this or #1029 ?

[jacobheun]

@mpetrunic I've been away from libp2p for a bit but I would say #1029 would be the priority. Circuit v2 will have more utility (although much more involved). The main reason this was a P1 when we created it was that it broke some existing behaviors folks were depending on with js-ipfs in browser being able to dial peers through a local go-ipfs node. This is handy if you have that local node, and it's configured to be an active relay (passive relays won't dial on your behalf, they only relay to already connected peers).

So to do this, when there is no relay specified, I'm supposed to send a CAN_HOP request to every connection in the connection manager to try to discover relay

I believe the old iteration just did the CAN_HOP check on each connect, and then kept track of the known relays so this didn't need to be done at time of dial.

Overall I would likely just prioritize Circuit V2. Then you can actually set up reservations and it will help unblock other hole punching work.

[tinytb]

It looks like #1029 is being worked on now; marking this as blocked for the time being.

[achingbrain]

Closing because the v2 implementation will be all that's supported in the future so traffic forwarding for remote hosts via the relay will not be supported.