From 7320f8497f99769392b33be2ecb5be3c1ce2eb51 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Fri, 3 Jan 2020 10:07:05 +0530 Subject: [PATCH] Remove the logic allowing lighthouse to update it's own ENR (#682) * Set random port when zero-port option is set * Remove logic allowing lighthouse to update its own ENR * Discovery address is set to localhost by default * Return error if discovery-addr isn't explicit --- beacon_node/eth2-libp2p/src/behaviour.rs | 5 -- beacon_node/eth2-libp2p/src/discovery.rs | 18 ------- beacon_node/eth2-libp2p/src/service.rs | 40 -------------- beacon_node/src/config.rs | 67 ++++++++++++++++++++++-- 4 files changed, 63 insertions(+), 67 deletions(-) diff --git a/beacon_node/eth2-libp2p/src/behaviour.rs b/beacon_node/eth2-libp2p/src/behaviour.rs index 251b482969..a9e697473c 100644 --- a/beacon_node/eth2-libp2p/src/behaviour.rs +++ b/beacon_node/eth2-libp2p/src/behaviour.rs @@ -254,11 +254,6 @@ impl Behaviour { pub fn peer_unbanned(&mut self, peer_id: &PeerId) { self.discovery.peer_unbanned(peer_id); } - - /// Informs the discovery behaviour if a new IP/Port is set at the application layer - pub fn update_local_enr_socket(&mut self, socket: std::net::SocketAddr, is_tcp: bool) { - self.discovery.update_local_enr(socket, is_tcp); - } } /// The types of events than can be obtained from polling the behaviour. diff --git a/beacon_node/eth2-libp2p/src/discovery.rs b/beacon_node/eth2-libp2p/src/discovery.rs index fc1653b7c0..fa3a3b604f 100644 --- a/beacon_node/eth2-libp2p/src/discovery.rs +++ b/beacon_node/eth2-libp2p/src/discovery.rs @@ -110,24 +110,6 @@ impl Discovery { }) } - /// Allows the application layer to update the `IP` and `port` of the local ENR. The second - /// parameter defines whether the port is a TCP port. If false, this is interpreted as a UDP - /// port. - pub fn update_local_enr(&mut self, socket: std::net::SocketAddr, is_tcp: bool) { - // discv5 checks to see if an update is necessary before performing it, so we do not - // need to check here - if self.discovery.update_local_enr_socket(socket, is_tcp) { - let enr = self.discovery.local_enr(); - info!( - self.log, - "ENR Updated"; - "enr" => enr.to_base64(), - "seq" => enr.seq(), - "address" => format!("{:?}", socket)); - save_enr_to_disc(Path::new(&self.enr_dir), enr, &self.log) - } - } - /// Return the nodes local ENR. pub fn local_enr(&self) -> &Enr { self.discovery.local_enr() diff --git a/beacon_node/eth2-libp2p/src/service.rs b/beacon_node/eth2-libp2p/src/service.rs index 55c599826c..11505194bb 100644 --- a/beacon_node/eth2-libp2p/src/service.rs +++ b/beacon_node/eth2-libp2p/src/service.rs @@ -42,9 +42,6 @@ pub struct Service { /// A list of timeouts after which peers become unbanned. peer_ban_timeout: DelayQueue, - /// Indicates if the listening address have been verified and compared to the expected ENR. - verified_listen_address: bool, - /// The libp2p logger handle. pub log: slog::Logger, } @@ -141,7 +138,6 @@ impl Service { swarm, peers_to_ban: DelayQueue::new(), peer_ban_timeout: DelayQueue::new(), - verified_listen_address: false, log, }) } @@ -241,46 +237,10 @@ impl Stream for Service { } } - // swarm is not ready - // check to see if the address is different to the config. If so, update our ENR - if !self.verified_listen_address { - let multiaddr = Swarm::listeners(&self.swarm).next(); - if let Some(multiaddr) = multiaddr { - self.verified_listen_address = true; - if let Some(socket_addr) = multiaddr_to_socket_addr(multiaddr) { - self.swarm.update_local_enr_socket(socket_addr, true); - } - } - } - Ok(Async::NotReady) } } -/// Converts a multiaddr to a `SocketAddr` if the multiaddr has the TCP/IP form. Libp2p currently -/// only supports TCP, so the UDP case is currently ignored. -fn multiaddr_to_socket_addr(multiaddr: &Multiaddr) -> Option { - let protocols = multiaddr.iter().collect::>(); - // assume the IP protocol - match protocols[0] { - Protocol::Ip4(address) => { - if let Protocol::Tcp(port) = protocols[1] { - Some(std::net::SocketAddr::new(address.into(), port)) - } else { - None - } - } - Protocol::Ip6(address) => { - if let Protocol::Tcp(port) = protocols[1] { - Some(std::net::SocketAddr::new(address.into(), port)) - } else { - None - } - } - _ => None, - } -} - /// The implementation supports TCP/IP, WebSockets over TCP/IP, secio as the encryption layer, and /// mplex or yamux as the multiplexing layer. fn build_transport(local_private_key: Keypair) -> Boxed<(PeerId, StreamMuxerBox), Error> { diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 9aaca259b9..6b700259fd 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -5,10 +5,11 @@ use eth2_libp2p::{Enr, Multiaddr}; use eth2_testnet_config::Eth2TestnetConfig; use genesis::recent_genesis_time; use rand::{distributions::Alphanumeric, Rng}; -use slog::{crit, info, Logger}; +use slog::{crit, info, warn, Logger}; use ssz::Encode; use std::fs; -use std::net::Ipv4Addr; +use std::net::{IpAddr, Ipv4Addr}; +use std::net::{TcpListener, UdpSocket}; use std::path::PathBuf; use types::{Epoch, EthSpec, Fork}; @@ -271,14 +272,31 @@ pub fn get_configs( * Zero-ports * * Replaces previously set flags. + * Libp2p and discovery ports are set explicitly by selecting + * a random free port so that we aren't needlessly updating ENR + * from lighthouse. + * Discovery address is set to localhost by default. */ if cli_args.is_present("zero-ports") { - client_config.network.libp2p_port = 0; - client_config.network.discovery_port = 0; + if client_config.network.discovery_address == IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)) { + client_config.network.discovery_address = "127.0.0.1".parse().expect("Valid IP address") + } + client_config.network.libp2p_port = + unused_port("tcp").map_err(|e| format!("Failed to get port for libp2p: {}", e))?; + client_config.network.discovery_port = + unused_port("udp").map_err(|e| format!("Failed to get port for discovery: {}", e))?; client_config.rest_api.port = 0; client_config.websocket_server.port = 0; } + // ENR ip needs to be explicit for node to be discoverable + if client_config.network.discovery_address == IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)) { + warn!( + log, + "Discovery address cannot be 0.0.0.0, Setting to to 127.0.0.1" + ); + client_config.network.discovery_address = "127.0.0.1".parse().expect("Valid IP address") + } Ok((client_config, eth2_config, log)) } @@ -550,3 +568,44 @@ fn random_string(len: usize) -> String { .take(len) .collect::() } + +/// A bit of hack to find an unused port. +/// +/// Does not guarantee that the given port is unused after the function exists, just that it was +/// unused before the function started (i.e., it does not reserve a port). +/// +/// Used for passing unused ports to libp2 so that lighthouse won't have to update +/// its own ENR. +/// +/// NOTE: It is possible that libp2p/discv5 is unable to bind to the +/// ports returned by this function as the OS has a buffer period where +/// it doesn't allow binding to the same port even after the socket is closed. +/// We might have to use SO_REUSEADDR socket option from `std::net2` crate in +/// that case. +pub fn unused_port(transport: &str) -> Result { + let local_addr = match transport { + "tcp" => { + let listener = TcpListener::bind("127.0.0.1:0").map_err(|e| { + format!("Failed to create TCP listener to find unused port: {:?}", e) + })?; + listener.local_addr().map_err(|e| { + format!( + "Failed to read TCP listener local_addr to find unused port: {:?}", + e + ) + })? + } + "udp" => { + let socket = UdpSocket::bind("127.0.0.1:0") + .map_err(|e| format!("Failed to create UDP socket to find unused port: {:?}", e))?; + socket.local_addr().map_err(|e| { + format!( + "Failed to read UDP socket local_addr to find unused port: {:?}", + e + ) + })? + } + _ => return Err("Invalid transport to find unused port".into()), + }; + Ok(local_addr.port()) +}