-
Notifications
You must be signed in to change notification settings - Fork 963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libp2p-dns] Implement /dnsaddr
resolution.
#1931
Conversation
It looks like looking up TXT records from |
This seems odd to me. Any suspicion why that might be the case? As a data point, though I doubt a very helpful one, the |
I don't know yet. I may have to do some noisy debugging on this branch for CI, for which I apologise in advance. |
I changed the tests to be explicit about the name server(s) to use (quad9 in this case). Besides solving the problem with the request timeouts for TXT lookups with the system configuration, I think it is better to avoid the indirection via the opaque system configuration for the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes on top of #1927 look sound to me.
In my eyes your changes to Swarm::dial_addr
and Network::dial
prevent a lot of confusion (thinking back to my early days with rust-libp2p). Very much appreciated.
Still waiting on #1927. |
To that end, since resolving `/dnsaddr` addresses needs "fully qualified" multiaddresses when dialing, i.e. those that end with the `/p2p/...` protocol, we make sure that dialing always uses such fully qualified addresses by appending the `/p2p` protocol as necessary. As a side-effect, this adds support for dialing peers via "fully qualified" addresses, as an alternative to using a `PeerId` together with a `Multiaddr` with or without the `/p2p` protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New changes look good to me. Thanks for the additional assert_eq
s.
})) => { | ||
return Poll::Ready(Some(Ok(ListenerEvent::Upgrade { | ||
upgrade: RelayedListenerUpgrade::Relayed(Some(stream)), | ||
local_addr: relay_addr | ||
.with(Protocol::P2p(relay_peer_id.into())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Closes #1920, as well as, by necessity, #1462.
This PR is based on #1927 - a proper diff until that PR is merged can be seen here.
In order to correctly resolve
/dnsaddr
addresses, the DNS transport needs to be given "fully qualified" multiaddresses when dialing, i.e. those that end with the/p2p/...
protocol, since it chooses from the TXT records such addresses with a matching suffix of the address being dialed. So to pick the addresses for the correct peer, the/p2p
protocol needs to be at the end of the address being dialed. This essentially goes back to #1462. So to address #1462 as well as implement/dnsaddr
resolution, we now make sure that dialing always uses such fully qualified addresses by appending the/p2p
protocol as necessary in theNetwork
oflibp2p-core
before giving the address for dialing to the transport. This is mostly transparent for thePeerId
-based APIs, i.e. it is of course still possible to use aPeerId
together with an "unqualified" address for dialing. It is just that at the end of the day,Transport::dial()
always gets a fully-qualified address (when called from theNetwork
anyway) and hence transports now permit a trailing/p2p
suffix (and in the case of the DNS transport, even make essential use of it).As a side-effect,
Network::dial(addr)
andSwarm::dial_addr(addr)
now also handle "fully-qualified"/p2p
addresses, since theNetwork
will then take the peer ID from the end of the address.A related change as part of implementing
/dnsaddr
resolution is thatlibp2p-dns
now actually tries to use all resolved addresses (well, up to a fixed maximum of dialing attempts).The net result of these changes can be seen in the now cleaned up
ipfs-kad
example, which can finally make use of/dnsaddr/bootstrap.libp2p.io
.