Add retries to DNS lookup for enode hostnames (#8358)

Cargo.toml

@@ -385,6 +385,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"

bin/reth/src/commands/p2p/mod.rs

@@ -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 {

bin/reth/src/commands/stage/run.rs

@@ -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,
@@ -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);
                         }
                     }

crates/net/common/Cargo.toml

@@ -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"] }

crates/net/common/src/lib.rs

@@ -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;

crates/net/network/src/manager.rs

@@ -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};
@@ -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);
         }
 

crates/net/types/Cargo.toml

@@ -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"] }

crates/net/types/src/dns_node_record.rs

@@ -2,6 +2,7 @@
 
 use std::{
     fmt::{self, Write},
+    io::Error,
     net::IpAddr,
     num::ParseIntError,
     str::FromStr,
@@ -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.
@@ -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 {
@@ -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 {
@@ -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))
+                }
+            }
+        }
+    }
 }

crates/net/types/src/lib.rs

@@ -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.