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

Add retries to DNS lookup for enode hostnames #8358

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ quote = "1.0"
tokio-stream = "0.1.11"
tokio = { version = "1.21", default-features = false }
tokio-util = { version = "0.7.4", features = ["codec"] }
tokio-retry = { version = "0.3.0" }

# async
async-stream = "0.3"
Expand Down
4 changes: 2 additions & 2 deletions bin/reth/src/commands/p2p/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ impl Command {

let mut config: Config = confy::load_path(&config_path).unwrap_or_default();

for &peer in &self.network.trusted_peers {
config.peers.trusted_nodes.insert(peer);
for peer in &self.network.trusted_peers {
config.peers.trusted_nodes.insert(peer.resolve(None).await?);
}

if config.peers.trusted_nodes.is_empty() && self.network.trusted_only {
Expand Down
3 changes: 1 addition & 2 deletions bin/reth/src/commands/stage/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use reth_config::{
use reth_db::init_db;
use reth_downloaders::bodies::bodies::BodiesDownloaderBuilder;
use reth_exex::ExExManagerHandle;
use reth_net_common::dns_node_record_resolve::resolve_dns_node_record;
use reth_primitives::ChainSpec;
use reth_provider::{
ProviderFactory, StageCheckpointReader, StageCheckpointWriter, StaticFileProviderFactory,
Expand Down Expand Up @@ -180,7 +179,7 @@ impl Command {
config.peers.trusted_nodes_only = self.network.trusted_only;
if !self.network.trusted_peers.is_empty() {
for peer in &self.network.trusted_peers {
let peer = resolve_dns_node_record(peer.clone()).await?;
let peer = peer.resolve(None).await?;
config.peers.trusted_nodes.insert(peer);
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/net/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ workspace = true

[dependencies]
# ethereum
alloy-primitives.workspace = true
alloy-primitives = { workspace = true, features = ["rand"] }

# async
pin-project.workspace = true
url.workspace = true
tokio = { workspace = true, features = ["full"] }
41 changes: 0 additions & 41 deletions crates/net/common/src/dns_node_record_resolve.rs

This file was deleted.

5 changes: 3 additions & 2 deletions crates/net/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
#![cfg_attr(not(test), warn(unused_crate_dependencies))]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

// PeerId::random() trait
use alloy_primitives as _;

pub mod ban_list;
pub mod bandwidth_meter;

/// Traits related to tokio streams
pub mod stream;

pub mod dns_node_record_resolve;

pub mod ratelimit;
6 changes: 2 additions & 4 deletions crates/net/network/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ use reth_eth_wire::{
DisconnectReason, EthVersion, Status,
};
use reth_metrics::common::mpsc::UnboundedMeteredSender;
use reth_net_common::{
bandwidth_meter::BandwidthMeter, dns_node_record_resolve::resolve_dns_node_record,
};
use reth_net_common::bandwidth_meter::BandwidthMeter;
use reth_network_api::ReputationChangeKind;
use reth_network_types::PeerId;
use reth_primitives::{ForkId, NodeRecord};
Expand Down Expand Up @@ -211,7 +209,7 @@ where
// resolve boot nodes
let mut resolved_boot_nodes = vec![];
for record in boot_nodes.iter() {
let resolved = resolve_dns_node_record(record.clone()).await?;
let resolved = record.resolve(None).await?;
resolved_boot_nodes.push(resolved);
}

Expand Down
2 changes: 2 additions & 0 deletions crates/net/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ secp256k1.workspace = true
serde_with.workspace = true
thiserror.workspace = true
url.workspace = true
tokio = { workspace = true, features = ["full"] }
tokio-retry = { workspace = true }

[dev-dependencies]
alloy-primitives = { workspace = true, features = ["rand"] }
Expand Down
107 changes: 107 additions & 0 deletions crates/net/types/src/dns_node_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::{
fmt::{self, Write},
io::Error,
net::IpAddr,
num::ParseIntError,
str::FromStr,
Expand All @@ -10,6 +11,8 @@ use std::{
use crate::{NodeRecord, PeerId};
use secp256k1::{SecretKey, SECP256K1};
use serde_with::{DeserializeFromStr, SerializeDisplay};
use std::time::Duration;
use tokio_retry::{strategy::FixedInterval, Retry};
use url::Host;

/// Represents a ENR in discovery.
Expand All @@ -27,6 +30,22 @@ pub struct DNSNodeRecord {
pub id: PeerId,
}

/// Retry strategy for DNS lookups.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct RetryStrategy {
/// The amount of time between attempts.
pub interval: Duration,
/// The number of attempts to make before failing.
pub attempts: usize,
}

impl RetryStrategy {
/// Create a new retry strategy.
pub fn new(interval: Duration, attempts: usize) -> Self {
Self { interval, attempts }
}
}

impl DNSNodeRecord {
/// Derive the [`NodeRecord`] from the secret key and addr
pub fn from_secret_key(host: Host, port: u16, sk: &SecretKey) -> Self {
Expand All @@ -39,6 +58,54 @@ impl DNSNodeRecord {
pub fn new(host: Host, port: u16, id: PeerId) -> Self {
Self { host, tcp_port: port, udp_port: port, id }
}

/// Resolves the host in a [DNSNodeRecord] to an IP address, returning a [NodeRecord].
pub async fn resolve(
&self,
retry_strategy: Option<RetryStrategy>,
) -> Result<NodeRecord, Error> {
let domain = match self.host.to_owned() {
Host::Ipv4(ip) => {
let id = self.id;
let tcp_port = self.tcp_port;
let udp_port = self.udp_port;

return Ok(NodeRecord { address: ip.into(), id, tcp_port, udp_port })
}
Host::Ipv6(ip) => {
let id = self.id;
let tcp_port = self.tcp_port;
let udp_port = self.udp_port;

return Ok(NodeRecord { address: ip.into(), id, tcp_port, udp_port })
}
Host::Domain(domain) => domain,
};

// Use provided retry strategy or use strategy which does not retry
let strategy = match retry_strategy {
Some(rs) => rs,
None => RetryStrategy::new(Duration::from_millis(0), 0),
};
// Execute the lookup
let lookup = || async { Self::lookup_host(&domain).await };
let retry = FixedInterval::new(strategy.interval).take(strategy.attempts);
let ip = Retry::spawn(retry, lookup).await?;
Ok(NodeRecord {
address: ip,
id: self.id,
tcp_port: self.tcp_port,
udp_port: self.udp_port,
})
}

async fn lookup_host(domain: &str) -> Result<std::net::IpAddr, Error> {
let mut ips = tokio::net::lookup_host(format!("{domain}:0")).await?;
let ip = ips
.next()
.ok_or_else(|| Error::new(std::io::ErrorKind::AddrNotAvailable, "No IP found"))?;
Ok(ip.ip())
}
}

impl fmt::Display for DNSNodeRecord {
Expand Down Expand Up @@ -234,4 +301,44 @@ mod tests {
assert_eq!(node, expected);
}
}

#[tokio::test]
async fn test_resolve_dns_node_record() {
use super::*;

// Set up tests
let tests = vec![
(
"localhost",
Some(RetryStrategy { interval: Duration::from_millis(100), attempts: 0 }),
),
(
"localhost",
Some(RetryStrategy { interval: Duration::from_millis(100), attempts: 3 }),
),
("localhost", None),
(
"localhost",
Some(RetryStrategy { interval: Duration::from_millis(100), attempts: 0 }),
),
];

// Run tests
for (domain, retry_strategy) in tests {
// Construct record
let rec =
DNSNodeRecord::new(url::Host::Domain(domain.to_owned()), 30300, PeerId::random());

// Resolve domain and validate
let rec = rec.resolve(retry_strategy).await.unwrap();
match rec.address {
std::net::IpAddr::V4(addr) => {
assert_eq!(addr, std::net::Ipv4Addr::new(127, 0, 0, 1))
}
std::net::IpAddr::V6(addr) => {
assert_eq!(addr, std::net::Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1))
}
}
}
}
}
2 changes: 1 addition & 1 deletion crates/net/types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub mod node_record;
pub use node_record::{NodeRecord, NodeRecordParseError};

pub mod dns_node_record;
pub use dns_node_record::DNSNodeRecord;
pub use dns_node_record::{DNSNodeRecord, RetryStrategy};

/// This tag should be set to indicate to libsecp256k1 that the following bytes denote an
/// uncompressed pubkey.
Expand Down
1 change: 1 addition & 0 deletions crates/node-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ reth-db = { workspace = true, features = ["mdbx"] }
reth-interfaces = { workspace = true, features = ["clap"] }
reth-provider.workspace = true
reth-network = { workspace = true, features = ["serde"] }
reth-network-types.workspace = true
reth-rpc-engine-api.workspace = true
reth-rpc-builder.workspace = true
reth-rpc.workspace = true
Expand Down
Loading