diff --git a/crates/cli/commands/src/p2p/bootnode.rs b/crates/cli/commands/src/p2p/bootnode.rs index cacc6f041ed..9589ae4da41 100644 --- a/crates/cli/commands/src/p2p/bootnode.rs +++ b/crates/cli/commands/src/p2p/bootnode.rs @@ -5,10 +5,11 @@ use reth_cli_util::{get_secret_key, load_secret_key::rng_secret_key}; use reth_discv4::{DiscoveryUpdate, Discv4, Discv4Config}; use reth_discv5::{ discv5::{self, Event, ListenConfig}, + enr::EnrCombinedKeyWrapper, Config, Discv5, }; use reth_net_nat::NatResolver; -use reth_network_peers::NodeRecord; +use reth_network_peers::{id2pk, pk2id, AnyNode, Enr, NodeRecord}; use secp256k1::SecretKey; use std::{ net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, @@ -17,7 +18,7 @@ use std::{ }; use tokio::{net::UdpSocket, select}; use tokio_stream::StreamExt; -use tracing::info; +use tracing::{info, warn}; /// Start a discovery only bootnode. #[derive(Parser, Debug)] @@ -37,14 +38,23 @@ pub struct Command { /// NAT resolution method (any|none|upnp|publicip|extip:\). /// - /// Can be repeated with one IPv4 and one IPv6 `extip:` to advertise a dual-stack discv5 - /// ENR (discv4 binds a single socket and always advertises only the `--addr` family). + /// Can be repeated with one IPv4 and one IPv6 `extip:` to advertise a dual-stack node. + /// The enode advertises only the `--addr` family; the discv5 and EIP-868 ENRs carry both + /// families, and with `--v5` discv4 also answers the opposite family on the shared port. #[arg(long, default_value = "any")] pub nat: Vec, /// Also run discv5, sharing the discv4 UDP port (`--addr`). + /// + /// The opposite-family sibling socket is only bound when an `extip:` of that family is + /// advertised via `--nat`; the default `--nat any` serves the `--addr` family only. #[arg(long)] pub v5: bool, + + /// Comma-separated nodes to seed discovery: `enode://` URLs with an IP host and/or signed + /// `enr:` records. Each entry seeds discv4 and (with `--v5`) discv5. + #[arg(long, value_delimiter = ',')] + pub bootnodes: Vec, } impl Command { @@ -57,8 +67,18 @@ impl Command { // (`enode://…@:0?discport=`). let local_enr = NodeRecord::from_secret_key(self.addr, &sk).with_tcp_port(0); let nat = self.resolved_nat()?; + let seeds = self.seed_nodes(&nat)?; + + for record in seeds.discv4_records() { + if !self.serves_family(record.address, &nat) { + warn!( + "No socket will be bound for the IP family of --bootnodes entry {record}; \ + it will not be contacted (advertise a matching --nat extip with --v5)" + ); + } + } - let discv4_config = self.discv4_config(&nat); + let discv4_config = self.discv4_config(&nat, &seeds); // In v5 mode discv4 and discv5 share a single UDP socket: discv5 owns the read loop and // discv4 packets surface as `UnrecognizedFrame`s forwarded to its ingress. The discv5 @@ -68,30 +88,32 @@ impl Command { // Actual port, so an ephemeral `--addr` port (`:0`) still yields one shared port. let shared_port = shared_socket.local_addr()?.port(); - let (discv4, discv4_service, mut ingress) = - Discv4::bind_shared(shared_socket.clone(), local_enr, sk, discv4_config)?; - info!("Started discv4 (shared port) at address: {local_enr:?}"); - // Hand discv5 the shared socket and bind the opposite family (if advertised) on the - // same port. - let mut discv5_config = self.discv5_config(&nat); + // same port. That sibling socket doubles as discv4's secondary-family egress, so + // discv4 answers both families on the shared port. + let mut discv5_config = self.discv5_config(&nat, &seeds); let discv5_cfg = discv5_config.discv5_config_mut(); let (mut ipv4, mut ipv6) = (None, None); if self.addr.is_ipv4() { - ipv4 = Some(shared_socket); + ipv4 = Some(shared_socket.clone()); if let Some(mut addr) = reth_discv5::config::ipv6(&discv5_cfg.listen_config) { addr.set_port(shared_port); ipv6 = Some(bind_socket(SocketAddr::V6(addr)).await?); } } else { - ipv6 = Some(shared_socket); + ipv6 = Some(shared_socket.clone()); if let Some(mut addr) = reth_discv5::config::ipv4(&discv5_cfg.listen_config) { addr.set_port(shared_port); ipv4 = Some(bind_socket(SocketAddr::V4(addr)).await?); } } + let secondary_socket = if self.addr.is_ipv4() { ipv6.clone() } else { ipv4.clone() }; discv5_cfg.listen_config = ListenConfig::FromSockets { ipv4, ipv6 }; + let (discv4, discv4_service, mut ingress) = + Discv4::bind_shared(shared_socket, secondary_socket, local_enr, sk, discv4_config)?; + info!("Started discv4 (shared port) at address: {local_enr:?}"); + info!("Starting discv5 (shared port)"); let (discv5, mut updates) = Discv5::start(&sk, discv5_config).await?; log_discv5_enr(&discv5); @@ -167,8 +189,8 @@ impl Command { /// /// A single value behaves like the rest of the node: the resolver drives discv4's external IP /// resolution, and any fixed IP is also advertised in the discv5 ENR. Two `extip:` values - /// (one per family) advertise a dual-stack discv5 record; discv4 advertises only the family - /// matching `--addr`, since it binds a single socket. + /// (one per family) advertise a dual-stack record; the enode advertises only the family + /// matching `--addr`, since discv4's own record carries a single address. fn resolved_nat(&self) -> eyre::Result { match self.nat.as_slice() { [] => Ok(BootnodeNat::single(NatResolver::Any, self.addr.port())), @@ -208,11 +230,92 @@ impl Command { } } - fn discv4_config(&self, nat: &BootnodeNat) -> Discv4Config { - Discv4Config::builder().external_ip_resolver(Some(nat.resolver.clone())).build() + fn discv4_config(&self, nat: &BootnodeNat, seeds: &BootnodeSeeds) -> Discv4Config { + let mut builder = Discv4Config::builder(); + builder + .external_ip_resolver(Some(nat.resolver.clone())) + .add_boot_nodes(seeds.discv4_records()); + // the opposite-family address is only served with the `--v5` sibling socket + if self.v5 && + let Some(ip) = + nat.advertised_ips.iter().find(|ip| ip.is_ipv4() != self.addr.is_ipv4()) + { + builder.secondary_advertised_ip(*ip); + } + builder.build() } - fn discv5_config(&self, nat: &BootnodeNat) -> Config { + /// Parses `--bootnodes`, keeping enode-sourced records separate from signed ENRs so an ENR + /// entry is not seeded into discv5 twice (signed and again as an unsigned enode). + fn seed_nodes(&self, nat: &BootnodeNat) -> eyre::Result { + let mut seeds = BootnodeSeeds::default(); + for node in &self.bootnodes { + let record = match node { + AnyNode::NodeRecord(record) => { + // v4-mapped hosts parse as V6; normalize so family routing is semantic + let record = record.into_ipv4_mapped(); + id2pk(record.id).map_err(|err| { + eyre::eyre!("--bootnodes entry has an invalid node id ({err}): {node}") + })?; + seeds.enodes.push(record); + record + } + AnyNode::Enr(enr) => { + let record = self.enr_endpoint(enr, nat)?; + seeds.enr_records.push(record); + seeds.enrs.push(EnrCombinedKeyWrapper::from(enr.clone()).0); + record + } + node => eyre::bail!( + "--bootnodes entries must be enode URLs with an IP host or signed ENRs: {node}" + ), + }; + if record.udp_port == 0 { + eyre::bail!("--bootnodes entry has no discovery port: {node}"); + } + } + Ok(seeds) + } + + /// Derives an ENR seed's discv4 endpoint, preferring a family this bootnode serves over the + /// default order (IPv4 first). + /// + /// Endpoints are derived per IP family so a partial dual-stack ENR (e.g. `ip4` without + /// `udp4`, plus `ip6`/`udp6`) cannot yield a mixed-family socket. + fn enr_endpoint(&self, enr: &Enr, nat: &BootnodeNat) -> eyre::Result { + let id = pk2id(&enr.public_key()); + let v4 = enr.ip4().zip(enr.udp4()).map(|(ip, udp_port)| NodeRecord { + address: ip.into(), + udp_port, + tcp_port: enr.tcp4().unwrap_or(0), + id, + }); + let v6 = enr.ip6().zip(enr.udp6()).map(|(ip, udp_port)| NodeRecord { + address: ip.into(), + udp_port, + tcp_port: enr.tcp6().unwrap_or(0), + id, + }); + + // an unservable endpoint is kept as a last resort; the startup warning covers it + v4.into_iter() + .chain(v6) + .find(|record| self.serves_family(record.address, nat)) + .or(v4) + .or(v6) + .ok_or_else(|| { + eyre::eyre!("unusable --bootnodes ENR (no ip+udp endpoint for one family): {enr}") + }) + } + + /// Whether a socket of `ip`'s family will be bound: the `--addr` family always, the opposite + /// family only when `--v5` binds its sibling socket (an address of that family is advertised). + fn serves_family(&self, ip: IpAddr, nat: &BootnodeNat) -> bool { + ip.is_ipv4() == self.addr.is_ipv4() || + (self.v5 && nat.advertised_ips.iter().any(|a| a.is_ipv4() == ip.is_ipv4())) + } + + fn discv5_config(&self, nat: &BootnodeNat, seeds: &BootnodeSeeds) -> Config { // Discovery-only bootnode: no RLPx, so the ENR carries `tcp=0`; the discovery listen // port is set from `--addr` below. let mut builder = Config::builder(SocketAddr::new(self.addr.ip(), 0)); @@ -243,6 +346,27 @@ impl Command { builder = builder.advertised_ip(*ip); } + // discv5's bootstrap aborts on an ENR it cannot contact (AddNodeFailed) and wastes a + // request_enr round-trip on an unreachable enode: seed only what the listen config serves + let enrs = seeds.enrs.iter().filter(|enr| { + let contactable = (bind_ipv4 && enr.udp4_socket().is_some()) || + (bind_ipv6 && enr.udp6_socket().is_some()); + if !contactable { + warn!("--bootnodes ENR has no endpoint for a served IP family; discv5 will not dial it: {enr}"); + } + contactable + }); + let enodes = seeds.enodes.iter().filter(|record| { + if record.address.is_ipv4() { + bind_ipv4 + } else { + bind_ipv6 + } + }); + + builder = + builder.add_signed_boot_nodes(enrs.cloned()).add_unsigned_boot_nodes(enodes.copied()); + builder.build() } } @@ -263,6 +387,24 @@ impl BootnodeNat { } } +/// Parsed `--bootnodes`, split by record kind. +#[derive(Debug, Default)] +struct BootnodeSeeds { + /// `enode://` entries: seed discv4 directly and discv5 as unsigned boot nodes. + enodes: Vec, + /// Signed `enr:` entries: seed discv5. + enrs: Vec, + /// Endpoints derived from `enrs`: seed discv4. + enr_records: Vec, +} + +impl BootnodeSeeds { + /// All discv4 bootstrap records (enode entries + ENR-derived endpoints). + fn discv4_records(&self) -> impl Iterator + '_ { + self.enodes.iter().chain(&self.enr_records).copied() + } +} + /// Binds a UDP socket for shared discv4/discv5 use. /// /// IPv6 sockets are bound with `IPV6_V6ONLY=true` so an IPv4 sibling socket on the same port @@ -371,7 +513,7 @@ mod tests { "extip:2001:db8::1", ]); let nat = command.resolved_nat().unwrap(); - let config = command.discv5_config(&nat); + let config = command.discv5_config(&nat, &command.seed_nodes(&nat).unwrap()); let sk = SecretKey::from_byte_array(&[1u8; 32]).unwrap(); let (enr, _, _, _) = build_local_enr(&sk, &config); @@ -393,7 +535,7 @@ mod tests { "extip:1.2.3.4", ]); let nat = command.resolved_nat().unwrap(); - let config = command.discv5_config(&nat); + let config = command.discv5_config(&nat, &command.seed_nodes(&nat).unwrap()); let sk = SecretKey::from_byte_array(&[1u8; 32]).unwrap(); let (enr, _, _, _) = build_local_enr(&sk, &config); @@ -415,7 +557,7 @@ mod tests { "extip:2001:db8::1", ]); let nat = command.resolved_nat().unwrap(); - let config = command.discv5_config(&nat); + let config = command.discv5_config(&nat, &command.seed_nodes(&nat).unwrap()); let sk = SecretKey::from_byte_array(&[1u8; 32]).unwrap(); let (enr, _, _, _) = build_local_enr(&sk, &config); @@ -462,10 +604,10 @@ mod tests { } #[test] - fn discv4_config_advertises_only_the_addr_family() { - // discv4 binds a single socket, so it must not advertise the secondary (IPv6) family it - // cannot serve; the IPv4 resolver drives discv4 and only discv5 carries both families. - let command = Command::parse_from([ + fn discv4_config_serves_secondary_family_only_with_v5() { + // The IPv4 resolver drives discv4's own record either way; the v6 address is only + // advertised (EIP-868 ENR) when the `--v5` sibling socket exists to serve it. + let args = [ "reth", "--addr", "0.0.0.0:30303", @@ -473,12 +615,156 @@ mod tests { "extip:1.2.3.4", "--nat", "extip:2001:db8::1", - ]); + ]; + + let command = Command::parse_from(args); let nat = command.resolved_nat().unwrap(); - let config = command.discv4_config(&nat); + let config = command.discv4_config(&nat, &command.seed_nodes(&nat).unwrap()); + assert_eq!(nat.resolver, NatResolver::ExternalIp("1.2.3.4".parse().unwrap())); + assert_eq!(config.secondary_advertised_ip, None); + let command = Command::parse_from(args.iter().chain(&["--v5"])); + let nat = command.resolved_nat().unwrap(); + let config = command.discv4_config(&nat, &command.seed_nodes(&nat).unwrap()); assert_eq!(nat.resolver, NatResolver::ExternalIp("1.2.3.4".parse().unwrap())); - assert!(config.additional_eip868_rlp_pairs.is_empty()); + assert_eq!(config.secondary_advertised_ip, Some("2001:db8::1".parse().unwrap())); + } + + #[test] + fn bootnodes_seed_discv4_and_discv5() { + // The EIP-778 example record: ip 127.0.0.1, udp 30303, no tcp key (discovery-only). + let enr = "enr:-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8"; + let enode = "enode://d860a01f9722d78051619d1e2351aba3f43f943f6f00718d1b9baa4101932a1f5011f16bb2b1bb35db20d6fe28fa0bf09636d26a87d31de9ec6203eeedb1f666@1.2.3.4:0?discport=30303"; + + let command = Command::parse_from(["reth", "--bootnodes", &format!("{enode},{enr}")]); + let nat = command.resolved_nat().unwrap(); + let seeds = command.seed_nodes(&nat).unwrap(); + + assert_eq!(seeds.enodes.len(), 1); + assert_eq!(seeds.enodes[0].address, "1.2.3.4".parse::().unwrap()); + assert_eq!(seeds.enodes[0].udp_port, 30303); + assert_eq!(seeds.enodes[0].tcp_port, 0); + // the ENR seeds discv4 too, with an absent tcp key meaning no RLPx — but stays out of + // `enodes` so discv5 doesn't bootstrap it twice (signed + unsigned) + assert_eq!(seeds.enr_records.len(), 1); + assert_eq!(seeds.enr_records[0].address, "127.0.0.1".parse::().unwrap()); + assert_eq!(seeds.enr_records[0].udp_port, 30303); + assert_eq!(seeds.enr_records[0].tcp_port, 0); + assert_eq!(seeds.enrs.len(), 1); + assert_eq!(seeds.enrs[0].udp4(), Some(30303)); + + let config = command.discv4_config(&command.resolved_nat().unwrap(), &seeds); + assert_eq!(config.bootstrap_nodes.len(), 2); + } + + #[test] + fn bootnodes_normalize_and_validate_enodes() { + let peer_id = "d860a01f9722d78051619d1e2351aba3f43f943f6f00718d1b9baa4101932a1f5011f16bb2b1bb35db20d6fe28fa0bf09636d26a87d31de9ec6203eeedb1f666"; + + // a v4-mapped host normalizes to its semantic IPv4 family + let command = Command::parse_from([ + "reth", + "--bootnodes", + &format!("enode://{peer_id}@[::ffff:9.9.9.9]:0?discport=30303"), + ]); + let seeds = command.seed_nodes(&command.resolved_nat().unwrap()).unwrap(); + assert_eq!(seeds.enodes[0].address, "9.9.9.9".parse::().unwrap()); + + // a zero discovery port can never be pinged + let command = + Command::parse_from(["reth", "--bootnodes", &format!("enode://{peer_id}@1.2.3.4:0")]); + assert!(command.seed_nodes(&command.resolved_nat().unwrap()).is_err()); + + // an id off the secp256k1 curve would be silently dropped by discv5's bootstrap + let bad_id = "ff".repeat(64); + let command = Command::parse_from([ + "reth", + "--bootnodes", + &format!("enode://{bad_id}@1.2.3.4:30303"), + ]); + assert!(command.seed_nodes(&command.resolved_nat().unwrap()).is_err()); + } + + #[test] + fn enr_seed_prefers_served_family() { + // Deterministic record (key 0xaa..aa): ip 9.9.9.9/udp 30303 + ip6 2001:db8::9/udp6 30304. + let dual = "enr:-KG4QF9o9jc3N31d-QdtzGq_HRKxncK1dmwMPQv8upyE43wzaMXsUrFdXobKuBTEwRaxToiampTdT9gw0sbWMjrE8SoBgmlkgnY0gmlwhAkJCQmDaXA2kCABDbgAAAAAAAAAAAAAAAmJc2VjcDI1NmsxoQJqBKuY2eR3StgG4wLd3rY76ha1y18iPud0eOhhu1g-s4N1ZHCCdl-EdWRwNoJ2YA"; + + // v6-primary: the discv4 seed must be the ENR's IPv6 endpoint, not the ip4-first default + let command = Command::parse_from([ + "reth", + "--addr", + "[::]:30303", + "--nat", + "extip:2001:db8::1", + "--bootnodes", + dual, + ]); + let seeds = command.seed_nodes(&command.resolved_nat().unwrap()).unwrap(); + assert_eq!(seeds.enr_records[0].address, "2001:db8::9".parse::().unwrap()); + assert_eq!(seeds.enr_records[0].udp_port, 30304); + + // v4-primary keeps the IPv4 endpoint + let command = Command::parse_from([ + "reth", + "--addr", + "0.0.0.0:30303", + "--nat", + "extip:1.2.3.4", + "--bootnodes", + dual, + ]); + let seeds = command.seed_nodes(&command.resolved_nat().unwrap()).unwrap(); + assert_eq!(seeds.enr_records[0].address, "9.9.9.9".parse::().unwrap()); + assert_eq!(seeds.enr_records[0].udp_port, 30303); + } + + #[test] + fn enr_seed_never_mixes_families() { + let sk = rng_secret_key(); + let enr = Enr::builder() + .ip4("9.9.9.9".parse().unwrap()) + .ip6("2001:db8::9".parse().unwrap()) + .udp6(30304) + .build(&sk) + .unwrap(); + + // v4-primary bootnode: the lone ip4 key is not an endpoint; the discv4 seed must be the + // complete v6 endpoint, not a fabricated 9.9.9.9:30304 + let command = Command::parse_from([ + "reth", + "--addr", + "0.0.0.0:30303", + "--nat", + "extip:1.2.3.4", + "--bootnodes", + &enr.to_base64(), + ]); + let seeds = command.seed_nodes(&command.resolved_nat().unwrap()).unwrap(); + assert_eq!(seeds.enr_records[0].address, "2001:db8::9".parse::().unwrap()); + assert_eq!(seeds.enr_records[0].udp_port, 30304); + + // no complete endpoint for any family is an error + let enr = Enr::builder().ip4("9.9.9.9".parse().unwrap()).build(&sk).unwrap(); + let command = Command::parse_from([ + "reth", + "--addr", + "0.0.0.0:30303", + "--nat", + "extip:1.2.3.4", + "--bootnodes", + &enr.to_base64(), + ]); + let err = command.seed_nodes(&command.resolved_nat().unwrap()).unwrap_err(); + assert!(err.to_string().contains("no ip+udp endpoint"), "{err}"); + } + + #[test] + fn bootnodes_reject_endpointless_entries() { + let peer_id = "d860a01f9722d78051619d1e2351aba3f43f943f6f00718d1b9baa4101932a1f5011f16bb2b1bb35db20d6fe28fa0bf09636d26a87d31de9ec6203eeedb1f666"; + + let command = Command::parse_from(["reth", "--bootnodes", &format!("enode://{peer_id}")]); + assert!(command.seed_nodes(&command.resolved_nat().unwrap()).is_err()); } #[test] @@ -494,7 +780,7 @@ mod tests { "extip:2001:db8::1", ]); let nat = command.resolved_nat().unwrap(); - let config = command.discv5_config(&nat); + let config = command.discv5_config(&nat, &command.seed_nodes(&nat).unwrap()); let sk = SecretKey::from_byte_array(&[1u8; 32]).unwrap(); let (enr, _, _, _) = build_local_enr(&sk, &config); diff --git a/crates/net/discv4/src/config.rs b/crates/net/discv4/src/config.rs index c2b30d3daf1..007d85ea794 100644 --- a/crates/net/discv4/src/config.rs +++ b/crates/net/discv4/src/config.rs @@ -10,6 +10,7 @@ use reth_net_nat::{NatResolver, ResolveNatInterval}; use reth_network_peers::NodeRecord; use std::{ collections::{HashMap, HashSet}, + net::IpAddr, time::Duration, }; @@ -62,6 +63,10 @@ pub struct Discv4Config { pub additional_eip868_rlp_pairs: HashMap, Bytes>, /// If configured, try to resolve public ip pub external_ip_resolver: Option, + /// External IP of the opposite family to the local [`NodeRecord`] address, advertised in the + /// local EIP-868 ENR next to it (`ip` + `ip6`). Serving that family requires a secondary + /// socket of that family (shared-port mode). + pub secondary_advertised_ip: Option, /// If configured and a `external_ip_resolver` is configured, try to resolve the external ip /// using this interval. pub resolve_external_ip_interval: Option, @@ -134,6 +139,7 @@ impl Default for Discv4Config { enforce_expiration_timestamps: true, additional_eip868_rlp_pairs: Default::default(), external_ip_resolver: Some(Default::default()), + secondary_advertised_ip: None, // By default retry public IP using a 5min interval resolve_external_ip_interval: Some(Duration::from_secs(60 * 5)), } @@ -298,6 +304,12 @@ impl Discv4ConfigBuilder { self } + /// Sets the secondary-family external IP advertised in the local EIP-868 ENR. + pub const fn secondary_advertised_ip(&mut self, ip: IpAddr) -> &mut Self { + self.config.secondary_advertised_ip = Some(ip); + self + } + /// Sets the interval at which the external IP is to be resolved. pub const fn resolve_external_ip_interval( &mut self, diff --git a/crates/net/discv4/src/lib.rs b/crates/net/discv4/src/lib.rs index 3ef421ea628..3eadac6a623 100644 --- a/crates/net/discv4/src/lib.rs +++ b/crates/net/discv4/src/lib.rs @@ -249,22 +249,35 @@ impl Discv4 { trace!(target: "discv4", local_addr=?socket.local_addr(), "opened UDP socket"); let (tx, rx) = mpsc::channel(config.udp_ingress_message_buffer); - Self::bind_with_socket(socket, Some(tx), rx, local_node_record, secret_key, config) + Self::bind_with_socket(socket, None, Some(tx), rx, local_node_record, secret_key, config) } /// Creates a new `Discv4` instance using a pre-bound shared socket. No receive loop is /// spawned; instead returns an [`IngressHandler`] that should be used to forward raw packets /// received by the socket owner (e.g. discv5 unrecognized frames). + /// + /// An optional secondary socket of the opposite IP family (e.g. discv5's dual-stack sibling + /// socket) makes the service answer peers of that family too: outgoing packets are routed to + /// the socket matching the destination family, while ingress for both arrives via the + /// returned [`IngressHandler`]. pub fn bind_shared( socket: Arc, + secondary_socket: Option>, local_node_record: NodeRecord, secret_key: SecretKey, config: Discv4Config, ) -> io::Result<(Self, Discv4Service, IngressHandler)> { let (tx, rx) = mpsc::channel(config.udp_ingress_message_buffer); let local_id = local_node_record.id; - let (discv4, service) = - Self::bind_with_socket(socket, None, rx, local_node_record, secret_key, config)?; + let (discv4, service) = Self::bind_with_socket( + socket, + secondary_socket, + None, + rx, + local_node_record, + secret_key, + config, + )?; let handler = IngressHandler::new(tx, local_id); @@ -273,6 +286,7 @@ impl Discv4 { fn bind_with_socket( socket: Arc, + secondary_socket: Option>, ingress_tx: Option, ingress_rx: IngressReceiver, mut local_node_record: NodeRecord, @@ -284,6 +298,7 @@ impl Discv4 { let mut service = Discv4Service::new( socket, + secondary_socket, ingress_tx, ingress_rx, local_addr, @@ -560,15 +575,22 @@ impl Discv4Service { /// /// If `ingress_tx` is `Some`, the receive loop is spawned to read from the socket. If `None`, /// the caller feeds packets into `ingress_rx` externally (shared socket mode). + #[expect(clippy::too_many_arguments)] pub(crate) fn new( socket: Arc, + secondary_socket: Option>, ingress_tx: Option, ingress_rx: IngressReceiver, local_address: SocketAddr, local_node_record: NodeRecord, secret_key: SecretKey, - config: Discv4Config, + mut config: Discv4Config, ) -> Self { + // normalize once: a same-family secondary would overwrite the primary ENR endpoint + config.secondary_advertised_ip = config + .secondary_advertised_ip + .filter(|ip| ip.is_ipv4() != local_node_record.address.is_ipv4()); + let (egress_tx, egress_rx) = mpsc::channel(config.udp_egress_message_buffer); let mut tasks = JoinSet::<()>::new(); @@ -578,7 +600,7 @@ impl Discv4Service { } let udp = Arc::clone(&socket); - tasks.spawn(send_loop(udp, egress_rx)); + tasks.spawn(send_loop(udp, secondary_socket, local_address.is_ipv4(), egress_rx)); let kbuckets = KBucketsTable::new( NodeKey::from(&local_node_record).into(), @@ -611,13 +633,26 @@ impl Discv4Service { // for EIP-868 construct an ENR let local_eip_868_enr = { let mut builder = Enr::builder(); - builder.ip(local_node_record.address); - if local_node_record.address.is_ipv4() { - builder.udp4(local_node_record.udp_port); - builder.tcp4(local_node_record.tcp_port); - } else { - builder.udp6(local_node_record.udp_port); - builder.tcp6(local_node_record.tcp_port); + { + let mut set_endpoint = |ip: IpAddr| { + // don't advertise an unspecified ip key (EIP-778 keys are optional), but keep + // the port keys: `set_external_ip_addr` only sets the ip key, so NAT + // resolution must find the ports already in place to complete the record + if !ip.is_unspecified() { + builder.ip(ip); + } + if ip.is_ipv4() { + builder.udp4(local_node_record.udp_port); + builder.tcp4(local_node_record.tcp_port); + } else { + builder.udp6(local_node_record.udp_port); + builder.tcp6(local_node_record.tcp_port); + } + }; + set_endpoint(local_node_record.address); + if let Some(ip) = config.secondary_advertised_ip.filter(|ip| !ip.is_unspecified()) { + set_endpoint(ip); + } } for (key, val) in &config.additional_eip868_rlp_pairs { @@ -697,6 +732,14 @@ impl Discv4Service { /// Sets the given ip address as the node's external IP in the node record announced in /// discovery pub fn set_external_ip_addr(&mut self, external_ip: IpAddr) { + // a secondary advertisement pins the record's family: an opposite-family resolution + // would silently overwrite the secondary ENR key and flip the record's family + if self.config.secondary_advertised_ip.is_some() && + external_ip.is_ipv4() != self.local_node_record.address.is_ipv4() + { + debug!(target: "discv4", ?external_ip, "Ignoring opposite-family external ip"); + return + } if self.local_node_record.address != external_ip { debug!(target: "discv4", ?external_ip, "Updating external ip"); self.local_node_record.address = external_ip; @@ -1864,6 +1907,13 @@ impl Discv4Service { } else { let _ = self.local_eip_868_enr.set_tcp6(port, &self.secret_key); } + if let Some(ip) = self.config.secondary_advertised_ip { + if ip.is_ipv4() { + let _ = self.local_eip_868_enr.set_tcp4(port, &self.secret_key); + } else { + let _ = self.local_eip_868_enr.set_tcp6(port, &self.secret_key); + } + } } Discv4Command::Terminated => { @@ -2001,10 +2051,22 @@ pub enum Discv4Event { } /// Continuously reads new messages from the channel and writes them to the socket -pub(crate) async fn send_loop(udp: Arc, rx: EgressReceiver) { +pub(crate) async fn send_loop( + udp: Arc, + secondary_udp: Option>, + primary_is_ipv4: bool, + rx: EgressReceiver, +) { let mut stream = ReceiverStream::new(rx); while let Some((payload, to)) = stream.next().await { - match udp.send_to(&payload, to).await { + // a destination with no matching-family socket falls through to the primary, failing + // in `send_to` like any other unreachable destination + let socket = if to.is_ipv4() == primary_is_ipv4 { + &udp + } else { + secondary_udp.as_ref().unwrap_or(&udp) + }; + match socket.send_to(&payload, to).await { Ok(size) => { trace!(target: "discv4", ?to, ?size,"sent payload"); } @@ -2571,7 +2633,7 @@ mod tests { use rand_08::Rng; use reth_ethereum_forks::{EnrForkIdEntry, ForkHash}; use reth_network_peers::mainnet_nodes; - use std::future::poll_fn; + use std::{future::poll_fn, net::Ipv6Addr}; #[tokio::test] async fn test_configured_enr_forkid_entry() { @@ -2591,6 +2653,77 @@ mod tests { assert_eq!(expected, decoded); } + #[tokio::test] + async fn test_secondary_advertised_ip_in_enr() { + let ip6: Ipv6Addr = "2001:db8::1".parse().unwrap(); + let config = Discv4Config::builder().secondary_advertised_ip(ip6.into()).build(); + let (_discv4, service) = create_discv4_with_config(config).await; + + let enr = &service.local_eip_868_enr; + assert_eq!(enr.ip6(), Some(ip6)); + assert_eq!(enr.udp6(), Some(service.local_node_record.udp_port)); + assert_eq!(enr.tcp6(), Some(service.local_node_record.tcp_port)); + } + + #[tokio::test] + async fn test_external_ip_completes_unspecified_enr_endpoint() { + let mut rng = rand_08::thread_rng(); + let socket: SocketAddr = "127.0.0.1:0".parse().unwrap(); + let (secret_key, pk) = secp256k1::SECP256K1.generate_keypair(&mut rng); + let local_enr = NodeRecord { + address: Ipv4Addr::UNSPECIFIED.into(), + tcp_port: 30303, + udp_port: 30303, + id: pk2id(&pk), + }; + let (_discv4, mut service) = + Discv4::bind(socket, local_enr, secret_key, Default::default()).await.unwrap(); + + let udp_port = service.local_node_record.udp_port; + let enr = &service.local_eip_868_enr; + assert_eq!(enr.ip4(), None); + assert_eq!(enr.udp4(), Some(udp_port)); + assert_eq!(enr.tcp4(), Some(30303)); + + let external_ip: Ipv4Addr = "1.2.3.4".parse().unwrap(); + service.set_external_ip_addr(external_ip.into()); + + let enr = &service.local_eip_868_enr; + assert_eq!(enr.ip4(), Some(external_ip)); + assert_eq!(enr.udp4(), Some(udp_port)); + assert_eq!(enr.tcp4(), Some(30303)); + } + + #[tokio::test] + async fn test_send_loop_routes_by_destination_family() { + let (Ok(secondary), Ok(sink6)) = + (UdpSocket::bind("[::1]:0").await, UdpSocket::bind("[::1]:0").await) + else { + // no IPv6 loopback on this host + return; + }; + let primary = Arc::new(UdpSocket::bind("127.0.0.1:0").await.unwrap()); + let sink4 = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + + let (tx, rx) = mpsc::channel(8); + tokio::spawn(send_loop(primary, Some(Arc::new(secondary)), true, rx)); + + tx.send((Bytes::from_static(b"v4"), sink4.local_addr().unwrap())).await.unwrap(); + tx.send((Bytes::from_static(b"v6"), sink6.local_addr().unwrap())).await.unwrap(); + + let mut buf = [0u8; 2]; + let (read, _) = tokio::time::timeout(Duration::from_secs(5), sink4.recv_from(&mut buf)) + .await + .unwrap() + .unwrap(); + assert_eq!(&buf[..read], b"v4"); + let (read, _) = tokio::time::timeout(Duration::from_secs(5), sink6.recv_from(&mut buf)) + .await + .unwrap() + .unwrap(); + assert_eq!(&buf[..read], b"v6"); + } + #[test] fn test_enr_forkid_entry_decode() { let raw: [u8; 8] = [0xc7, 0xc6, 0x84, 0xdc, 0xe9, 0x6c, 0x2d, 0x80]; @@ -3177,7 +3310,7 @@ mod tests { } #[tokio::test] - async fn test_bootnode_not_in_update_stream() { + async fn test_bootnode_in_update_stream() { reth_tracing::init_test_tracing(); let (_, service_1) = create_discv4().await; let peerid_1 = *service_1.local_peer_id(); @@ -3209,7 +3342,6 @@ mod tests { } } - // Assert bootnode did not appear in update stream assert!(bootnode_appeared, "Bootnode should appear in update stream"); } diff --git a/crates/net/discv4/src/test_utils.rs b/crates/net/discv4/src/test_utils.rs index 293c712cac2..3e5594abdb1 100644 --- a/crates/net/discv4/src/test_utils.rs +++ b/crates/net/discv4/src/test_utils.rs @@ -49,7 +49,8 @@ impl MockDiscovery { /// Creates a new instance and opens a socket pub async fn new() -> io::Result<(Self, mpsc::Sender)> { let mut rng = thread_rng(); - let socket = SocketAddr::from_str("0.0.0.0:0").unwrap(); + // loopback, not 0.0.0.0: only Linux delivers sends to a 0.0.0.0 destination + let socket = SocketAddr::from_str("127.0.0.1:0").unwrap(); let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); let id = pk2id(&pk); let socket = Arc::new(UdpSocket::bind(socket).await?); @@ -69,7 +70,7 @@ impl MockDiscovery { tasks.spawn(receive_loop(udp, ingress_tx, local_enr.id)); let udp = Arc::clone(&socket); - tasks.spawn(send_loop(udp, egress_rx)); + tasks.spawn(send_loop(udp, None, local_addr.is_ipv4(), egress_rx)); let (tx, command_rx) = mpsc::channel(128); let this = Self { @@ -240,7 +241,8 @@ pub async fn create_discv4() -> (Discv4, Discv4Service) { /// Creates a new testing instance for [`Discv4`] and its service with the given config. pub async fn create_discv4_with_config(config: Discv4Config) -> (Discv4, Discv4Service) { let mut rng = thread_rng(); - let socket = SocketAddr::from_str("0.0.0.0:0").unwrap(); + // loopback, not 0.0.0.0: only Linux delivers sends to a 0.0.0.0 destination + let socket = SocketAddr::from_str("127.0.0.1:0").unwrap(); let (secret_key, pk) = SECP256K1.generate_keypair(&mut rng); let id = pk2id(&pk); let local_enr = diff --git a/crates/net/network/src/discovery.rs b/crates/net/network/src/discovery.rs index 0006ad45ac3..6ad1a5a9432 100644 --- a/crates/net/network/src/discovery.rs +++ b/crates/net/network/src/discovery.rs @@ -109,16 +109,29 @@ impl Discovery { }; // In shared-port mode, bind the shared socket and start discv4 without its own receive - // loop. Unrecognized frames from discv5 will be forwarded to the ingress handler. - let (discv4, discv4_updates, _discv4_service, discv4_ingress, shared_socket) = + // loop. Unrecognized frames from discv5 will be forwarded to the ingress handler, and + // discv5's opposite-family sibling socket doubles as discv4's secondary-family egress. + let (discv4, discv4_updates, _discv4_service, discv4_ingress, shared_sockets) = if let Some(config) = discv4_config { if let Some(discv5_config) = &mut discv5_config && discv5_config.has_matching_socket(discovery_v4_addr) { let socket = bind_socket(discovery_v4_addr).await?; + let listen_config = &discv5_config.discv5_config_mut().listen_config; + let secondary_addr = if discovery_v4_addr.is_ipv4() { + reth_discv5::config::ipv6(listen_config).map(SocketAddr::V6) + } else { + reth_discv5::config::ipv4(listen_config).map(SocketAddr::V4) + }; + let secondary_socket = match secondary_addr { + Some(addr) => Some(bind_socket(addr).await?), + None => None, + }; + let (discv4, mut discv4_service, ingress) = Discv4::bind_shared( socket.clone(), + secondary_socket.clone(), local_enr, sk, config, @@ -135,7 +148,7 @@ impl Discovery { Some(discv4_updates), Some(discv4_service), Some(ingress), - Some(socket), + Some((socket, secondary_socket)), ) } else { let (discv4, mut discv4_service) = @@ -164,25 +177,14 @@ impl Discovery { // Set OS-assigned advertised RLPx ports to the bound listener port. set_bound_rlpx_port_if_unset(&mut config, tcp_addr.port()); - if let Some(socket) = shared_socket { - let discv5_cfg = config.discv5_config_mut(); - - // The shared socket covers discv4's address family; bind the opposite family - // only if discv5 was configured for dual-stack. - let (mut ipv4, mut ipv6) = (None, None); - if discovery_v4_addr.is_ipv4() { - ipv4 = Some(socket); - if let Some(addr) = reth_discv5::config::ipv6(&discv5_cfg.listen_config) { - ipv6 = Some(bind_socket(SocketAddr::V6(addr)).await?); - } + if let Some((primary, secondary)) = shared_sockets { + let (ipv4, ipv6) = if discovery_v4_addr.is_ipv4() { + (Some(primary), secondary) } else { - ipv6 = Some(socket); - if let Some(addr) = reth_discv5::config::ipv4(&discv5_cfg.listen_config) { - ipv4 = Some(bind_socket(SocketAddr::V4(addr)).await?); - } - } - - discv5_cfg.listen_config = discv5::ListenConfig::FromSockets { ipv4, ipv6 }; + (secondary, Some(primary)) + }; + config.discv5_config_mut().listen_config = + discv5::ListenConfig::FromSockets { ipv4, ipv6 }; } let (discv5, discv5_updates) = Discv5::start(&sk, config).await?; @@ -474,7 +476,10 @@ impl Discovery { mod tests { use super::*; use secp256k1::SECP256K1; - use std::net::{Ipv4Addr, SocketAddrV4}; + use std::{ + net::{Ipv4Addr, SocketAddrV4}, + time::Duration, + }; #[tokio::test(flavor = "multi_thread")] async fn test_discovery_setup() { @@ -810,4 +815,82 @@ mod tests { .await .expect("discovery should start with shared port + dual-stack"); } + + /// A discv4 ping over the opposite family must complete the endpoint proof: the sibling + /// socket bound for discv5's dual-stack listen config is discv4's secondary-family egress. + #[tokio::test(flavor = "multi_thread")] + async fn test_shared_port_dual_stack_answers_opposite_family_discv4() { + reth_tracing::init_test_tracing(); + + let Ok(probe6) = UdpSocket::bind("[::1]:0").await else { + // no IPv6 loopback on this host + return; + }; + drop(probe6); + + let probe = UdpSocket::bind("0.0.0.0:0").await.expect("probe bind"); + let port = probe.local_addr().unwrap().port(); + drop(probe); + + let secret_key = SecretKey::new(&mut rand_08::thread_rng()); + let node_id = reth_network_peers::pk2id(&secret_key.public_key(SECP256K1)); + let v4_addr: SocketAddr = format!("0.0.0.0:{port}").parse().unwrap(); + let tcp_addr: SocketAddr = "0.0.0.0:30303".parse().unwrap(); + + let discv4_config = Discv4ConfigBuilder::default().external_ip_resolver(None).build(); + let discv5_listen_config = discv5::ListenConfig::DualStack { + ipv4: std::net::Ipv4Addr::UNSPECIFIED, + ipv4_port: port, + ipv6: std::net::Ipv6Addr::UNSPECIFIED, + ipv6_port: port, + }; + let discv5_config = reth_discv5::Config::builder(tcp_addr) + .discv5_config(discv5::ConfigBuilder::new(discv5_listen_config).build()) + .build(); + + let _node = Discovery::new( + tcp_addr, + v4_addr, + secret_key, + Some(discv4_config), + Some(discv5_config), + None, + ) + .await + .expect("discovery should start with shared port + dual-stack"); + + // discv4-only prober on IPv6 loopback, seeded with the node's IPv6 endpoint + let probe_sk = SecretKey::new(&mut rand_08::thread_rng()); + let node_v6 = NodeRecord { + address: "::1".parse().unwrap(), + udp_port: port, + tcp_port: port, + id: node_id, + }; + let probe_config = Discv4ConfigBuilder::default() + .external_ip_resolver(None) + .add_boot_node(node_v6) + .build(); + let probe_addr: SocketAddr = "[::1]:0".parse().unwrap(); + let probe_enr = NodeRecord::from_secret_key(probe_addr, &probe_sk); + let (_probe, mut probe_service) = + Discv4::bind(probe_addr, probe_enr, probe_sk, probe_config).await.unwrap(); + let mut updates = probe_service.update_stream(); + probe_service.spawn(); + + // the bootnode only becomes `Added` once the node's pong arrives over IPv6 + let added = tokio::time::timeout(Duration::from_secs(5), async { + while let Some(update) = updates.next().await { + if let DiscoveryUpdate::Added(record) = update && + record.id == node_id + { + return true + } + } + false + }) + .await + .unwrap_or(false); + assert!(added, "node should answer discv4 over its opposite (IPv6) family"); + } } diff --git a/docs/vocs/docs/pages/cli/reth/p2p/bootnode.mdx b/docs/vocs/docs/pages/cli/reth/p2p/bootnode.mdx index b46512df099..efc85e3bd38 100644 --- a/docs/vocs/docs/pages/cli/reth/p2p/bootnode.mdx +++ b/docs/vocs/docs/pages/cli/reth/p2p/bootnode.mdx @@ -22,12 +22,17 @@ Options: --nat NAT resolution method (any|none|upnp|publicip|extip:\). - Can be repeated with one IPv4 and one IPv6 `extip:` to advertise a dual-stack discv5 ENR (discv4 binds a single socket and always advertises only the `--addr` family). + Can be repeated with one IPv4 and one IPv6 `extip:` to advertise a dual-stack node. The enode advertises only the `--addr` family; the discv5 and EIP-868 ENRs carry both families, and with `--v5` discv4 also answers the opposite family on the shared port. [default: any] --v5 - Also run discv5, sharing the discv4 UDP port (`--addr`) + Also run discv5, sharing the discv4 UDP port (`--addr`). + + The opposite-family sibling socket is only bound when an `extip:` of that family is advertised via `--nat`; the default `--nat any` serves the `--addr` family only. + + --bootnodes + Comma-separated nodes to seed discovery: `enode://` URLs with an IP host and/or signed `enr:` records. Each entry seeds discv4 and (with `--v5`) discv5 -h, --help Print help (see a summary with '-h')