Add transport support for dnsaddr #1450
Conversation
different generator has their own types
transports/dns/src/lib.rs
Outdated
| @@ -170,7 +176,56 @@ where | |||
| }) | |||
| .next() | |||
| .ok_or_else(|| DnsErr::ResolveFail(name)) | |||
| }.left_future() | |||
| }.boxed().left_future() | |||
There was a problem hiding this comment.
You normally don't have to box the future. Just put an async block around the entire code instead of having two distinct async blocks.
Since this is a lot of code, it might be worth moving it to a separate freestanding function.
| assert_eq!(addr.len(), 3); | ||
| // TODO: ipfs | ||
| // match addr[2] { | ||
| // Protocol::Ipfs(_) => (), |
There was a problem hiding this comment.
I'll file another PR to add the ipfs support.
transports/dns/src/lib.rs
Outdated
| use trust_dns_client::udp::UdpClientConnection; | ||
| use trust_dns_client::client::{SyncClient, Client}; | ||
| use trust_dns_proto::rr::domain::Name; | ||
| use futures::stream::FuturesOrdered; |
There was a problem hiding this comment.
Please merge with the use futures:: above.
transports/dns/src/lib.rs
Outdated
| @@ -238,6 +225,90 @@ where TErr: error::Error + 'static | |||
| } | |||
| } | |||
|
|
|||
| async fn resolve_dns<T>(p: Protocol<'_>, suffix: Multiaddr) -> Result<Multiaddr, DnsErr<T::Error>> | |||
| where | |||
There was a problem hiding this comment.
These where bounds shouldn't be necessary.
| where |
There was a problem hiding this comment.
Updated at fcc3d6a .
If T: Transport is removed we'll get the error:
error[E0220]: associated type `Error` not found for `T`
--> transports/dns/src/lib.rs:228:89
|
228 | async fn resolve_dns<T>(p: Protocol<'_>, suffix: Multiaddr) -> Result<Multiaddr, DnsErr<T::Error>>
| ^^^^^^^^ associated type `Error` not found
transports/dns/src/lib.rs
Outdated
| { | ||
| match p { | ||
| Protocol::Dns4(ref n) | Protocol::Dns6(ref n) => { | ||
| let name = n.to_string(); |
There was a problem hiding this comment.
| let name = n.to_string(); |
transports/dns/src/lib.rs
Outdated
| T::Dial: Send | ||
| { | ||
| match p { | ||
| Protocol::Dns4(ref n) | Protocol::Dns6(ref n) => { |
There was a problem hiding this comment.
| Protocol::Dns4(ref n) | Protocol::Dns6(ref n) => { | |
| Protocol::Dns4(ref name) | Protocol::Dns6(ref name) => { |
transports/dns/src/lib.rs
Outdated
| Err(e) => Err(e) | ||
| }.map_err(|_| { | ||
| error!("DNS resolver crashed"); | ||
| DnsErr::ResolveFail(name.clone()) |
There was a problem hiding this comment.
| DnsErr::ResolveFail(name.clone()) | |
| DnsErr::ResolveFail(name.to_string()) |
transports/dns/src/lib.rs
Outdated
| if suffix.len() == 0 { | ||
| Ok(resolved_addrs[0].clone()) | ||
| } else { | ||
| // TODO: suffix matching |
There was a problem hiding this comment.
I'd that this is out of scope of this function.
| } | ||
| Protocol::Dnsaddr(ref n) => { | ||
| let conn = UdpClientConnection::new("8.8.8.8:53".parse().unwrap()).unwrap(); // TODO: error handling | ||
| let client = SyncClient::new(conn); // TODO: should use async client? |
There was a problem hiding this comment.
// TODO: should use async client?
Yes!
There was a problem hiding this comment.
Hmm...
I've added trust-dns in order to query TXT records. I'm trying to switch the SyncClient to AsyncClient but I think AsyncClient can not be used for rust-libp2p because a background future is required for the client. 🤔
https://docs.rs/trust-dns-client/0.19.3/trust_dns_client/#async-usage
// Create a new client, the bg is a background future which handles
// the multiplexing of the DNS requests to the server.
// the client is a handle to an unbounded queue for sending requests via the
// background. The background must be scheduled to run before the client can
// send any dns requests
let client = AsyncClient::connect(stream);
// await the connection to be established
let (mut client, bg) = runtime.block_on(client).expect("connection failed");
// make sure to run the background task
runtime.spawn(bg);======
Do you know other library what can query TXT records asynchronously? 💦
There was a problem hiding this comment.
I think AsyncClient can not be used for rust-libp2p because a background future is required for the client.
That makes it more difficult to implement, but normally not impossible.
There was a problem hiding this comment.
The idea is not to run the background future in the background, but to run it alongside with the "front" one. I don't have more precise guidelines without looking deeply into details (which I don't really have the time right now to do) though.
There was a problem hiding this comment.
I've thought for a while about this but I can not come up anything to address the issue... hmm... 🤔💦
Co-Authored-By: Pierre Krieger <pierre.krieger1708@gmail.com>
|
Superseded by #1931. |
Related issue: #967
Related PR: #1356
https://github.com/multiformats/multiaddr/blob/master/protocols/DNSADDR.md