Support .nodeAddress() for /dnsaddr

[hacdias]

@jacobheun Are there any plans to support conversion to a node address from a dns addr? If so, I can take a stab at it and open a PR. If not, why? Should it be done in some other package?

I also wonder since we would need to check the DNS records if it would need to be async or not. Surely we could keep the API sync.

[jacobheun]

@hacdias we should add it, it looks like it just wasn't added to the nodeAddress list when it was added. Ideally I think we'd have some type of resolver in the protocol table instead of it being hard coded in the nodeAddress function so protocols are less likely to get missed in the future.

I also wonder since we would need to check the DNS records if it would need to be async or not. Surely we could keep the API sync.

What do we need to do this for? IIRC the existing transports are handling the dns resolution. .resolve does need to get implemented at some point, I'm just not sure on the priority, and that will need to be an async function. That should be the function that handles any lookup for protocols in the table that are marked as resolvable

[hacdias]

Hmm... as far as I understood, we need to resolve the /dnsaddr/example.com to be able to return something acceptable for nodeAddress since dnsaddr is a TXT record in the DNS on example.com domain. Please correct me if I'm wrong.

[jacobheun]

Ah yes, you're right, we need to resolve the TXT record.

We'll need to implement resolve so we can get the updated addresses and then perform nodeAddress on those.

It might be good to do some level of cacheing so we don't need to resolve the address frequently. We can't do a replace after the resolve as the TXT records are mutable, so we need to keep a record of the original address. But the cacheing is probably something that should be handled in PeerBook/PeerInfo anyway.

[richardschneider]

Since its DNS, the TXT record will provide a TTL. A good DNS client should do the caching for us.

[lidel]

The way I see it:

1. Add .resolve method to multiaddr instances (returns self by default)
2. Decide what whappens when .nodeAddress() is called on unresolved /dnsaddr (undefined ? throw?)

[jacobheun]

what to return when .nodeAddress() is called on /dnsaddr? undefined ?

I would expect this to throw an error, as it needs to be resolved. The addresses we get from .resolve() should be used to perform .nodeAddress().

We should really be performing resolution on any resolvable address we learn about before attempting to connect, and then using the resolved addresses to do the connecting.

[hacdias]

I could take a look at this effort. Is there anything else that should be done other than adding .resolve() on this package? I'll certain have a few questions during the process though.

[jacobheun]

Is there anything else that should be done other than adding .resolve() on this package?

That should be sufficient. Libp2p will need to get updated to do resolutions as part of the dialing and/or peer discovery pipeline.

While I don't think it's necessary for this iteration, we should keep in mind that in the future we may need different resolvers per address type.

[olizilla]

This is blocking js-ipfs from making use of the new bootstrappers see ipfs/js-ipfs#2581

@hacdias did you get anywhere with it? Will you pick it up again?

[hacdias]

@olizilla probably not and I haven't made any further progress on this one... :'(

[vasco-santos]

From my understanding after reading this and discuss a little bit with @jacobheun , we want the following:

• Support custom resolvers per codec
• Each resolver should be responsible for caching its data
• .nodeAddress() should throw if a dnsaddr multiaddr was not resolved (only case?)

This will unblock dnsaddr multiaddr to be resolved using a dns resolver, as well as other cases that might appear.

Moreover, we need a resolver for dnsaddr. The challenging part is resolving it in a browser context

[lidel]

WebUI/Desktop use js-multiaddr for validating input, which means it is impossible to connect to /dnsaddr/bootstrap.libp2p.io/p2p/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u19uLTa via GUI on the Peers screeen (@rafaelramalho19 is blocked in ipfs/ipfs-webui#1593).

On DNSAddr resolver for browser context

Resolving DNSAddr in browser context is tricky, because initial reaction is that we would like to avoid hard-coding/configuring third party DNS services, such as Cloudflare.

We have a nice solution for DNSLink in form of /api/v0/dns, which works on every IPFS node, and is also exposed on public gateways, for example: https://ipfs.io/api/v0/dns/en.wikipedia-on-ipfs.org

We could have similar endpoint for resolving DNSAddr, but it is introducing tight coupling between multiformats and IPFS API, while dnsaddr is an libp2p thing that does not care about IPFS at all, so entire scheme smells a bit. Even if we do this, there should be a configuration option to override IPFS resolver with a generic DoH endpoint, such as Cloudflare.

So.. supporting DoH is already a topic discussed in the context of js-ipfs (ipfs/helia-ipns#53) and it sounds like a safe, vendor-agnostic way of handling DNS resolution in browser context. If we add it to js-multiaddr, there should be an opt-in way to override the default resolver.

@vasco-santos @jacobheun is we wanted to unblock this, what would be the next step here?

[jacobheun]

There are 2 pieces to this as I see it:

1. Create a DoH resolver module that takes a multiaddr and resolves its dnsaddr. This could be multipurpose for DNS over http in general, but we could just start with dnsaddr TXT records for now, as normal dns addresses automaticall resolve in node address format. Ideally this module should do some cacheing of dns addresses.
2. Update multiaddr to support extending it with resolvers to avoid unnecessarily bloating the default module, something akin to the work being done at Dependency injection patterns for reducing bundle size ipld/js-block#10. I think supporting multiple resolvers per multiaddr protocol would be nice. Then in libp2p/IPFS we can register the resolvers for use by the libp2p Dialer.