Apply PR suggestions

beacon_node/eth2-libp2p/src/behaviour.rs

@@ -19,6 +19,8 @@ use ssz::{ssz_encode, Encode};
 use std::num::NonZeroU32;
 use std::time::Duration;
 
+const MAX_IDENTIFY_ADDRESSES: usize = 20;
+
 /// Builds the network behaviour that manages the core protocols of eth2.
 /// This core behaviour is managed by `Behaviour` which adds peer management to all core
 /// behaviours.
@@ -148,12 +150,12 @@ impl<TSubstream: AsyncRead + AsyncWrite> NetworkBehaviourEventProcess<IdentifyEv
             IdentifyEvent::Identified {
                 peer_id, mut info, ..
             } => {
-                if info.listen_addrs.len() > 20 {
+                if info.listen_addrs.len() > MAX_IDENTIFY_ADDRESSES {
                     debug!(
                         self.log,
                         "More than 20 addresses have been identified, truncating"
                     );
-                    info.listen_addrs.truncate(20);
+                    info.listen_addrs.truncate(MAX_IDENTIFY_ADDRESSES);
                 }
                 debug!(self.log, "Identified Peer"; "Peer" => format!("{}", peer_id),
                 "Protocol Version" => info.protocol_version,
@@ -264,55 +266,3 @@ impl Encode for PubsubMessage {
         }
     }
 }
-
-/*
-impl Decode for PubsubMessage {
-    fn is_ssz_fixed_len() -> bool {
-        false
-    }
-
-    fn from_ssz_bytes(bytes: &[u8]) -> Result<Self, ssz::DecodeError> {
-        let mut builder = ssz::SszDecoderBuilder::new(&bytes);
-
-        builder.register_type::<u32>()?;
-        builder.register_type::<Vec<u8>>()?;
-
-        let mut decoder = builder.build()?;
-
-        let id: u32 = decoder.decode_next()?;
-        let body: Vec<u8> = decoder.decode_next()?;
-
-        match id {
-            0 => Ok(PubsubMessage::Block(BeaconBlock::from_ssz_bytes(&body)?)),
-            1 => Ok(PubsubMessage::Attestation(Attestation::from_ssz_bytes(
-                &body,
-            )?)),
-            _ => Err(DecodeError::BytesInvalid(
-                "Invalid PubsubMessage id".to_string(),
-            )),
-        }
-    }
-}
-*/
-
-/*
-#[cfg(test)]
-mod test {
-    use super::*;
-    use types::*;
-
-    #[test]
-    fn ssz_encoding() {
-        let original = PubsubMessage::Block(BeaconBlock::<MainnetEthSpec>::empty(
-            &MainnetEthSpec::default_spec(),
-        ));
-
-        let encoded = ssz_encode(&original);
-
-        let decoded = PubsubMessage::from_ssz_bytes(&encoded).unwrap();
-
-        assert_eq!(original, decoded);
-    }
-
-}
-*/

beacon_node/eth2-libp2p/src/rpc/handler.rs

@@ -268,8 +268,11 @@ where
         Self::Error,
     > {
         if let Some(err) = self.pending_error.take() {
+            // Returning an error here will result in dropping any peer that doesn't support any of
+            // the RPC protocols. For our immediate purposes we permit this and simply log that an
+            // upgrade was not supported.
+            // TODO: Add a logger to the handler for trace output.
             dbg!(&err);
-            //return Err(err);
         }
 
         // return any events that need to be reported

beacon_node/src/main.rs

@@ -4,7 +4,7 @@ use clap::{App, Arg};
 use client::{ClientConfig, Eth2Config};
 use env_logger::{Builder, Env};
 use eth2_config::{read_from_file, write_to_file};
-use slog::{crit, o, Drain, Level};
+use slog::{crit, o, warn, Drain, Level};
 use std::fs;
 use std::path::PathBuf;
 
@@ -323,19 +323,36 @@ fn main() {
         Some("interop") => Some(Eth2Config::interop()),
         _ => None,
     };
-    // if cli is specified, write the new config
+    // if a CLI flag is specified, write the new config if it doesn't exist,
+    // otherwise notify the user that the file will not be written.
+    let eth2_config_from_file = match read_from_file::<Eth2Config>(eth2_config_path.clone()) {
+        Ok(config) => config,
+        Err(e) => {
+            crit!(log, "Failed to read the Eth2Config from file"; "error" => format!("{:?}", e));
+            return;
+        }
+    };
+
     let mut eth2_config = {
         if let Some(cli_config) = cli_config {
-            if let Err(e) = write_to_file(eth2_config_path, &cli_config) {
+            if eth2_config_from_file.is_none() {
-                crit!(log, "Failed to write default Eth2Config to file"; "error" => format!("{:?}", e));
+                // write to file if one doesn't exist
-                return;
+                if let Err(e) = write_to_file(eth2_config_path, &cli_config) {
+                    crit!(log, "Failed to write default Eth2Config to file"; "error" => format!("{:?}", e));
+                    return;
+                }
+            } else {
+                warn!(
+                    log,
+                    "Eth2Config file exists. Configuration file is ignored, using default"
+                );
             }
             cli_config
         } else {
-            // config not specified, read from disk
+            // CLI config not specified, read from disk
-            match read_from_file::<Eth2Config>(eth2_config_path.clone()) {
+            match eth2_config_from_file {
-                Ok(Some(c)) => c,
+                Some(config) => config,
-                Ok(None) => {
+                None => {
                     // set default to minimal
                     let eth2_config = Eth2Config::minimal();
                     if let Err(e) = write_to_file(eth2_config_path, &eth2_config) {
@@ -344,10 +361,6 @@ fn main() {
                     }
                     eth2_config
                 }
-                Err(e) => {
-                    crit!(log, "Failed to instantiate an Eth2Config"; "error" => format!("{:?}", e));
-                    return;
-                }
             }
         }
     };
@@ -363,7 +376,7 @@ fn main() {
 
     // check to ensure the spec constants between the client and eth2_config match
     if eth2_config.spec_constants != client_config.spec_constants {
-        crit!(log, "Specification constants do not match."; "Client Config" => format!("{}", client_config.spec_constants), "Eth2 Config" => format!("{}", eth2_config.spec_constants));
+        crit!(log, "Specification constants do not match."; "client_config" => format!("{}", client_config.spec_constants), "eth2_config" => format!("{}", eth2_config.spec_constants));
         return;
     }
 

validator_client/src/main.rs

@@ -11,7 +11,7 @@ use crate::service::Service as ValidatorService;
 use clap::{App, Arg};
 use eth2_config::{read_from_file, write_to_file, Eth2Config};
 use protos::services_grpc::ValidatorServiceClient;
-use slog::{crit, error, info, o, Drain, Level};
+use slog::{crit, error, info, o, warn, Drain, Level};
 use std::fs;
 use std::path::PathBuf;
 use types::{InteropEthSpec, Keypair, MainnetEthSpec, MinimalEthSpec};
@@ -173,19 +173,36 @@ fn main() {
         Some("interop") => Some(Eth2Config::interop()),
         _ => None,
     };
-    // if cli is specified, write the new config
+    // if a CLI flag is specified, write the new config if it doesn't exist,
+    // otherwise notify the user that the file will not be written.
+    let eth2_config_from_file = match read_from_file::<Eth2Config>(eth2_config_path.clone()) {
+        Ok(config) => config,
+        Err(e) => {
+            crit!(log, "Failed to read the Eth2Config from file"; "error" => format!("{:?}", e));
+            return;
+        }
+    };
+
     let mut eth2_config = {
         if let Some(cli_config) = cli_config {
-            if let Err(e) = write_to_file(eth2_config_path, &cli_config) {
+            if eth2_config_from_file.is_none() {
-                crit!(log, "Failed to write default Eth2Config to file"; "error" => format!("{:?}", e));
+                // write to file if one doesn't exist
-                return;
+                if let Err(e) = write_to_file(eth2_config_path, &cli_config) {
+                    crit!(log, "Failed to write default Eth2Config to file"; "error" => format!("{:?}", e));
+                    return;
+                }
+            } else {
+                warn!(
+                    log,
+                    "Eth2Config file exists. Configuration file is ignored, using default"
+                );
             }
             cli_config
         } else {
-            // config not specified, read from disk
+            // CLI config not specified, read from disk
-            match read_from_file::<Eth2Config>(eth2_config_path.clone()) {
+            match eth2_config_from_file {
-                Ok(Some(c)) => c,
+                Some(config) => config,
-                Ok(None) => {
+                None => {
                     // set default to minimal
                     let eth2_config = Eth2Config::minimal();
                     if let Err(e) = write_to_file(eth2_config_path, &eth2_config) {
@@ -194,10 +211,6 @@ fn main() {
                     }
                     eth2_config
                 }
-                Err(e) => {
-                    crit!(log, "Failed to instantiate an Eth2Config"; "error" => format!("{:?}", e));
-                    return;
-                }
             }
         }
     };