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

@@ -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.
+    /// The address to broadcast to peers about which address we are listening on. None indicates
-    pub discovery_address: std::net::IpAddr,
+    /// that no discovery address has been set in the CLI args.
+    pub discovery_address: Option<std::net::IpAddr>,
 
     /// 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,

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

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(

beacon_node/src/config.rs

@@ -100,7 +100,6 @@ pub fn get_configs<E: EthSpec>(
             .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<E: EthSpec>(
     }
 
     if let Some(discovery_address_str) = cli_args.value_of("discovery-address") {
-        client_config.network.discovery_address = discovery_address_str
+        client_config.network.discovery_address = Some(
-            .parse()
+            discovery_address_str
-            .map_err(|_| format!("Invalid discovery address: {:?}", 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<E: EthSpec>(
      * 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)) {
+        if client_config.network.discovery_address == Some(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.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<E: EthSpec>(
         client_config.websocket_server.port = 0;
     }
 
-    // ENR ip needs to be explicit for node to be discoverable
+    // 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)) {
+    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))
 }