Better support for /dnsaddr/ (i.e. in swarm connect)

[hsanjuan]

Version information:

master

Type:

Enhancement

Description:

It should be possible to do ipfs swarm connect /dnsaddr/something. Right now it doesn't attempt to resolve and throws Error: invalid peer address: invalid IPFS address

[overbool]

@hsanjuan Error: invalid peer address: invalid IPFS address implies that the addr is invalid ipfs address, because it misses tcp and ipfs protocol. I try ipfs swarm connect /dnsaddr/xxx/tcp/4001/ipfs/Qmxxx in my bash, it print connect Qmxxx success.

[hsanjuan]

It should resolve the /dnsaddr/xxx before checking if the ipfs protocol is there (I think it checks too early now). The full libp2p address for a peer can be included in the _dnsaddr txt field of the domain, as pointed out in https://github.com/multiformats/go-multiaddr-dns

$ dig +short TXT _dnsaddr.bootstrap.libp2p.io
"dnsaddr=/ip6/2a03:b0c0:0:1010::23:1001/tcp/4001/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd"
"dnsaddr=/ip4/178.62.158.247/tcp/4001/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd"
$ madns /dnsaddr/bootstrap.libp2p.io/
/ip4/178.62.158.247/tcp/4001/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd
/ip6/2a03:b0c0:0:1010::23:1001/tcp/4001/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNA

[hsanjuan]

@overbool you are confusing /dns4 or /dns6 with /dnsaddr (common mistake), but they are different. /dns4 resolves domain to /ip4/<ip>. /dnsaddr resolves multiaddress from _dnsaddr TXT field associated to the domain.

[overbool]

@hsanjuan You are right. I've confused /dns4 with /dnsaddr.

[overbool]

It seems the same issue #5249.
Maybe we should resolve addr before connecting.

[Stebalien]

@overbool that one is for the --api flag (that's how we tell the IPFS client which daemon it should connect to).

@hsanjuan we'll have to think about a better way to do https://github.com/multiformats/go-multiaddr-dns/blob/78f39e8892d4f8c5c9f18679cc06d0b40ecab8cf/resolve.go#L167

Basically, when we resolve a dnsaddr, we match the final component to try to pull out the right ones. In theory, this allows us to have multiple distinct peers with the same hostname. In practice, this is kind of arbitrary. Any solution that tries to solve this issue will also have to solve that.

Note: there are also security concerns but I'm not that concerned. If the user doesn't specify a peer ID, they have no reason to believe we'll end up connecting to the right peer (they haven't told us which one to use).

[hsanjuan]

Ok, two problems here.

The first is the one triggering the issue. connect commands parses with https://github.com/ipfs/go-ipfs-addr/blob/master/ipfsaddr.go#L72 and this errors when the given address does not have an /ipfs/ part, before resolving it (it should resolve first or parse differently?).

The second is related domain resolution as mentioned. Thing works with empty trailers (no /ipfs/peer part in the original multiaddress, my usecase), or otherwise it seems the trailer needs to match the one defined in the dnslink.

That makes some current bootstrap peers (https://github.com/ipfs/go-ipfs-config/blob/master/bootstrap_peers.go#L20), problematic. Since the _dnsklink for bootstrap.libp2p.io contains /ipfs/Qm... parts, the bootstrap peer /dnsaddr/bootstrap.libp2p.io/ipfs/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN doesn't resolve at all because the peer ids don't match (resolves but returns an empty list of results).

But the second problem should not affect me if I just want to run ipfs swarm connect /dnsaddr/cluster.ipfs.io because this domain provides the full address, peer included (just like bootstrap.ipfs.io).

[overbool]

@Stebalien Did you mean that we should append an addr with new pid(ipfs/Qmnewpeerid) when the trailer doesn't match the resolved dnsaddr?

https://github.com/multiformats/go-multiaddr-dns/blob/78f39e8892d4f8c5c9f18679cc06d0b40ecab8cf/resolve.go#L167

[Stebalien]

Did you mean that we should append an addr with new pid(ipfs/Qmnewpeerid) when the trailer doesn't match the resolved dnsaddr?

@hsanjuan is right, we can probably solve this particular issue without delving into that. I'm not really sure the best way to improve this at the moment.

[overbool]

@Stebalien If we only solve this issue without considering resolveDnsaddr improvement currently, I had made a pr #5535. Could u help me review it?