Skip to content
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

chore(p2p): dnsaddr recursive resolution #2204

Merged
merged 16 commits into from
Sep 17, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2151](https://github.com/FuelLabs/fuel-core/pull/2151): Added limitations on gas used during dry_run in API.
- [2188](https://github.com/FuelLabs/fuel-core/pull/2188): Added the new variant `V2` for the `ConsensusParameters` which contains the new `block_transaction_size_limit` parameter.
- [2163](https://github.com/FuelLabs/fuel-core/pull/2163): Added runnable task for fetching block committer data.
- [2204](https://github.com/FuelLabs/fuel-core/pull/2204): Added `dnsaddr` resolution for TLD without suffixes.

### Changed

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions crates/fuel-core/src/p2p_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,13 @@ pub struct NamedNodes(pub HashMap<String, Node>);

impl Bootstrap {
/// Spawn a bootstrap node.
pub async fn new(node_config: &Config) -> Self {
pub async fn new(node_config: &Config) -> anyhow::Result<Self> {
let bootstrap_config = extract_p2p_config(node_config).await;
let codec = PostcardCodec::new(bootstrap_config.max_block_size);
let (sender, _) =
broadcast::channel(bootstrap_config.reserved_nodes.len().saturating_add(1));
let mut bootstrap = FuelP2PService::new(sender, bootstrap_config, codec);
bootstrap.start().await.unwrap();
let mut bootstrap = FuelP2PService::new(sender, bootstrap_config, codec).await?;
bootstrap.start().await?;

let listeners = bootstrap.multiaddrs();
let (kill, mut shutdown) = broadcast::channel(1);
Expand Down Expand Up @@ -169,7 +169,7 @@ impl Bootstrap {
}
});

Bootstrap { listeners, kill }
Ok(Bootstrap { listeners, kill })
}

pub fn listeners(&self) -> Vec<Multiaddr> {
Expand Down Expand Up @@ -269,7 +269,9 @@ pub async fn make_nodes(
if let Some(BootstrapSetup { pub_key, .. }) = boot {
update_signing_key(&mut node_config, pub_key);
}
Bootstrap::new(&node_config).await
Bootstrap::new(&node_config)
.await
.expect("Failed to create bootstrap node")
}
})
.collect()
Expand Down
1 change: 1 addition & 0 deletions crates/services/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fuel-core-storage = { workspace = true, features = ["std"] }
fuel-core-types = { workspace = true, features = ["std", "serde"] }
futures = { workspace = true }
hex = { workspace = true }
hickory-resolver = "0.24.1"
ip_network = "0.4"
libp2p = { version = "0.53.2", default-features = false, features = [
"dns",
Expand Down
11 changes: 6 additions & 5 deletions crates/services/p2p/src/behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub struct FuelBehaviour {
}

impl FuelBehaviour {
pub(crate) fn new(p2p_config: &Config, codec: PostcardCodec) -> Self {
pub(crate) fn new(p2p_config: &Config, codec: PostcardCodec) -> anyhow::Result<Self> {
let local_public_key = p2p_config.keypair.public();
let local_peer_id = PeerId::from_public_key(&local_public_key);

Expand Down Expand Up @@ -93,7 +93,7 @@ impl FuelBehaviour {

let gossipsub = build_gossipsub_behaviour(p2p_config);

let peer_report = peer_report::Behaviour::new(p2p_config);
let peer_report = peer_report::Behaviour::new(&p2p_config.reserved_nodes);

let identify = {
let identify_config = identify::Config::new(
Expand Down Expand Up @@ -125,15 +125,16 @@ impl FuelBehaviour {
req_res_config,
);

Self {
discovery: discovery_config.finish(),
let discovery = discovery_config.finish()?;
Ok(Self {
discovery,
gossipsub,
peer_report,
request_response,
blocked_peer: Default::default(),
identify,
heartbeat,
}
})
}

pub fn add_addresses_to_discovery(
Expand Down
3 changes: 2 additions & 1 deletion crates/services/p2p/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use std::{
use tracing::trace;
mod discovery_config;
mod mdns_wrapper;

pub use discovery_config::Config;

const SIXTY_SECONDS: Duration = Duration::from_secs(60);
Expand Down Expand Up @@ -265,7 +266,7 @@ mod tests {
.with_bootstrap_nodes(bootstrap_nodes)
.with_random_walk(Duration::from_millis(500));

config.finish()
config.finish().expect("Config should be valid")
}
}

Expand Down
11 changes: 5 additions & 6 deletions crates/services/p2p/src/discovery/discovery_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl Config {
self
}

pub fn finish(self) -> Behaviour {
pub fn finish(self) -> anyhow::Result<Behaviour> {
let Config {
local_peer_id,
bootstrap_nodes,
Expand All @@ -112,9 +112,8 @@ impl Config {
let memory_store = MemoryStore::new(local_peer_id.to_owned());
let mut kademlia_config = kad::Config::default();
let network = format!("/fuel/kad/{network_name}/kad/1.0.0");
kademlia_config.set_protocol_names(vec![
StreamProtocol::try_from_owned(network).expect("Invalid kad protocol")
]);
kademlia_config
.set_protocol_names(vec![StreamProtocol::try_from_owned(network)?]);

let mut kademlia =
kad::Behaviour::with_config(local_peer_id, memory_store, kademlia_config);
Expand Down Expand Up @@ -167,13 +166,13 @@ impl Config {
MdnsWrapper::disabled()
};

Behaviour {
Ok(Behaviour {
connected_peers: HashSet::new(),
kademlia,
next_kad_random_walk,
duration_to_next_kad: Duration::from_secs(1),
max_peers_connected,
mdns,
}
})
}
}
111 changes: 111 additions & 0 deletions crates/services/p2p/src/dnsaddr_resolution.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
use anyhow::anyhow;
use hickory_resolver::TokioAsyncResolver;
use libp2p::Multiaddr;
use std::pin::Pin;

/// The prefix for `dnsaddr` protocol TXT record lookups.
const DNSADDR_PREFIX: &str = "_dnsaddr.";
/// The maximum number of DNS lookups when dialing.
/// This limit is for preventing malicious or misconfigured DNS records from causing infinite recursion.
const MAX_DNS_LOOKUPS: usize = 10;

pub(crate) struct DnsResolver {
resolver: TokioAsyncResolver,
}

impl DnsResolver {
pub(crate) async fn lookup_dnsaddr(
&self,
addr: &str,
) -> anyhow::Result<Vec<Multiaddr>> {
self.resolve_recursive(addr, 0).await
}

pub(crate) async fn new() -> anyhow::Result<Box<Self>> {
let resolver = TokioAsyncResolver::tokio_from_system_conf()?;
Ok(Box::new(Self { resolver }))
}

/// Internal method to handle recursive DNS lookups.
fn resolve_recursive<'a>(
&'a self,
addr: &'a str,
depth: usize,
) -> Pin<
Box<dyn std::future::Future<Output = anyhow::Result<Vec<Multiaddr>>> + Send + 'a>,
> {
Box::pin(async move {
if depth >= MAX_DNS_LOOKUPS {
return Err(anyhow!("Maximum DNS lookup depth exceeded"));
}

let mut multiaddrs = vec![];
let dns_lookup_url = format!("{}{}", DNSADDR_PREFIX, addr);
let txt_records = self.resolver.txt_lookup(dns_lookup_url).await?;

for record in txt_records {
let parsed = record.to_string();
if !parsed.starts_with("dnsaddr") {
continue;
}

let dnsaddr_value = parsed
.split("dnsaddr=")
.nth(1)
.ok_or_else(|| anyhow!("Invalid DNS address: {:?}", parsed))?;

// Check if the parsed value is a multiaddress or another dnsaddr.
if dnsaddr_value.starts_with("/dnsaddr") {
let nested_dnsaddr = dnsaddr_value
.split('/')
.nth(2)
.ok_or_else(|| anyhow!("Invalid nested dnsaddr"))?;
// Recursively resolve the nested dnsaddr
#[allow(clippy::arithmetic_side_effects)]
let nested_addrs =
self.resolve_recursive(nested_dnsaddr, depth + 1).await?;
multiaddrs.extend(nested_addrs);
} else if let Ok(multiaddr) = dnsaddr_value.parse::<Multiaddr>() {
multiaddrs.push(multiaddr);
}
}

Ok(multiaddrs)
})
}
}

#[allow(non_snake_case)]
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashSet;

#[tokio::test]
async fn dns_resolver__parses_all_multiaddresses_from_mainnet_dnsaddr_entry() {
// given
let resolver = DnsResolver::new().await.unwrap();
let expected_multiaddrs: HashSet<Multiaddr> = [
"/dns/p2p-mainnet.fuel.network/tcp/30336/p2p/16Uiu2HAkxjhwNYtwawWUexYn84MsrA9ivFWkNHmiF4hSieoNP7Jd",
"/dns/p2p-mainnet.fuel.network/tcp/30337/p2p/16Uiu2HAmQunK6Dd81BXh3rW2ZsszgviPgGMuHw39vv2XxbkuCfaw",
"/dns/p2p-mainnet.fuel.network/tcp/30333/p2p/16Uiu2HAkuiLZNrfecgDYHJZV5LoEtCXqqRCqHY3yLBqs4LQk8jJg",
"/dns/p2p-mainnet.fuel.network/tcp/30334/p2p/16Uiu2HAkzYNa6yMykppS1ij69mKoKjrZEr11oHGiM5Mpc8nKjVDM",
"/dns/p2p-mainnet.fuel.network/tcp/30335/p2p/16Uiu2HAm5yqpTv1QVk3SepUYzeKXTWMuE2VqMWHq5qQLPR2Udg6s"
].iter().map(|s| s.parse().unwrap()).collect();

// when
// run a `dig +short txt _dnsaddr.mainnet.fuel.network` to get the TXT records
let multiaddrs = resolver
.lookup_dnsaddr("mainnet.fuel.network")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we switch this to core-test.fuellabs.net? The records on the mainnet.fuel.network record can and will change over time, but I just setup the core-test DNS record to be static.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for setting this up. I created this follow-up issue to use the core-test.fuellabs.net hostname in the test, since this PR got auto-merged after my approval.

.await
.unwrap();
// then
for multiaddr in multiaddrs.iter() {
assert!(
expected_multiaddrs.contains(multiaddr),
"Unexpected multiaddr: {:?}",
multiaddr
);
}
}
}
9 changes: 9 additions & 0 deletions crates/services/p2p/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub mod behavior;
pub mod codecs;
pub mod config;
pub mod discovery;
mod dnsaddr_resolution;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd move this declaration to a separate block than the public modules.

pub mod gossipsub;
pub mod heartbeat;
pub mod heavy_task_processor;
Expand All @@ -23,6 +24,7 @@ pub use libp2p::{
Multiaddr,
PeerId,
};
use tracing::warn;

#[cfg(feature = "test-helpers")]
pub mod network_service {
Expand All @@ -38,6 +40,13 @@ impl TryPeerId for Multiaddr {
fn try_to_peer_id(&self) -> Option<PeerId> {
self.iter().last().and_then(|p| match p {
Protocol::P2p(peer_id) => Some(peer_id),
Protocol::Dnsaddr(multiaddr) => {
warn!(
"synchronous recursive dnsaddr resolution is not yet supported: {:?}",
multiaddr
);
None
}
_ => None,
})
}
Expand Down
Loading
Loading