From 8c96739cab62b28d9646d3eb9f61445e81bc96fd Mon Sep 17 00:00:00 2001 From: Age Manning Date: Thu, 23 Jan 2020 17:31:08 +1100 Subject: [PATCH] Correct discovery address CLI functionality (#818) * Improve handling of discovery IP address CLI config * Remove excess debug logging * Add reviewers suggestions --- beacon_node/eth2-libp2p/src/config.rs | 7 ++++--- beacon_node/eth2-libp2p/src/discovery.rs | 7 +++++-- beacon_node/src/cli.rs | 3 ++- beacon_node/src/config.rs | 20 +++++++++++--------- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/beacon_node/eth2-libp2p/src/config.rs b/beacon_node/eth2-libp2p/src/config.rs index 682a648b3..d0ec62df2 100644 --- a/beacon_node/eth2-libp2p/src/config.rs +++ b/beacon_node/eth2-libp2p/src/config.rs @@ -20,8 +20,9 @@ pub struct Config { /// The TCP port that libp2p listens on. pub libp2p_port: u16, - /// The address to broadcast to peers about which address we are listening on. - pub discovery_address: std::net::IpAddr, + /// The address to broadcast to peers about which address we are listening on. None indicates + /// that no discovery address has been set in the CLI args. + pub discovery_address: Option, /// UDP port that discovery listens on. pub discovery_port: u16, @@ -86,7 +87,7 @@ impl Default for Config { network_dir, listen_address: "127.0.0.1".parse().expect("valid ip address"), libp2p_port: 9000, - discovery_address: "127.0.0.1".parse().expect("valid ip address"), + discovery_address: None, discovery_port: 9000, max_peers: 10, secret_key_hex: None, diff --git a/beacon_node/eth2-libp2p/src/discovery.rs b/beacon_node/eth2-libp2p/src/discovery.rs index fa3a3b604..d2c46da1a 100644 --- a/beacon_node/eth2-libp2p/src/discovery.rs +++ b/beacon_node/eth2-libp2p/src/discovery.rs @@ -310,7 +310,9 @@ fn load_enr( // Note: Discovery should update the ENR record's IP to the external IP as seen by the // majority of our peers. let mut local_enr = EnrBuilder::new("v4") - .ip(config.discovery_address) + .ip(config + .discovery_address + .unwrap_or_else(|| "127.0.0.1".parse().expect("valid ip"))) .tcp(config.libp2p_port) .udp(config.discovery_port) .build(&local_key) @@ -325,7 +327,8 @@ fn load_enr( match Enr::from_str(&enr_string) { Ok(enr) => { if enr.node_id() == local_enr.node_id() { - if enr.ip().map(Into::into) == Some(config.discovery_address) + if (config.discovery_address.is_none() + || enr.ip().map(Into::into) == config.discovery_address) && enr.tcp() == Some(config.libp2p_port) && enr.udp() == Some(config.discovery_port) { diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index bdd4a1b1a..741e9045f 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -90,7 +90,8 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long("discovery-address") .value_name("ADDRESS") .help("The IP address to broadcast to other peers on how to reach this node. \ - Default is determined automatically.") + Default will load previous values from disk failing this it is set to 127.0.0.1 \ + and will be updated when connecting to other nodes on the network.") .takes_value(true), ) .arg( diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index b68e1841e..75efeea6c 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -100,7 +100,6 @@ pub fn get_configs( .parse() .map_err(|_| format!("Invalid listen address: {:?}", listen_address_str))?; client_config.network.listen_address = listen_address; - client_config.network.discovery_address = listen_address; } if let Some(max_peers_str) = cli_args.value_of("maxpeers") { @@ -140,9 +139,11 @@ pub fn get_configs( } if let Some(discovery_address_str) = cli_args.value_of("discovery-address") { - client_config.network.discovery_address = discovery_address_str - .parse() - .map_err(|_| format!("Invalid discovery address: {:?}", discovery_address_str))? + client_config.network.discovery_address = Some( + discovery_address_str + .parse() + .map_err(|_| format!("Invalid discovery address: {:?}", discovery_address_str))?, + ) } if let Some(disc_port_str) = cli_args.value_of("disc-port") { @@ -283,8 +284,8 @@ pub fn get_configs( * Discovery address is set to localhost by default. */ if cli_args.is_present("zero-ports") { - 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") + if client_config.network.discovery_address == Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))) { + client_config.network.discovery_address = None } client_config.network.libp2p_port = unused_port("tcp").map_err(|e| format!("Failed to get port for libp2p: {}", e))?; @@ -294,13 +295,14 @@ pub fn get_configs( 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)) { + // ENR IP needs to be explicit for node to be discoverable + if client_config.network.discovery_address == Some(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") + client_config.network.discovery_address = + Some("127.0.0.1".parse().expect("Valid IP address")) } Ok((client_config, eth2_config, log)) }