Integrate ValidatorDirectory with validator_client

validator_client/src/cli.rs

@@ -1,5 +1,21 @@
-use crate::config::{DEFAULT_SERVER, DEFAULT_SERVER_GRPC_PORT, DEFAULT_SERVER_HTTP_PORT};
+use crate::config::Config;
 use clap::{App, Arg, SubCommand};
+use lazy_static::lazy_static;
+
+lazy_static! {
+    /// The default configuration. Is in lazy_static because clap requires references, therefore we
+    /// can't initialize the defaults in the `cli_app` function
+    static ref DEFAULTS: Config = {
+        Config::default()
+    };
+
+    static ref DEFAULT_SERVER_GRPC_PORT: String = {
+        format!("{}", DEFAULTS.server_grpc_port)
+    };
+    static ref DEFAULT_SERVER_HTTP_PORT: String = {
+        format!("{}", DEFAULTS.server_http_port)
+    };
+}
 
 pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
     App::new("Validator Client")
@@ -35,7 +51,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .long("server")
                 .value_name("NETWORK_ADDRESS")
                 .help("Address to connect to BeaconNode.")
-                .default_value(DEFAULT_SERVER)
+                .default_value(&DEFAULTS.server)
                 .takes_value(true),
         )
         .arg(
@@ -44,7 +60,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .short("g")
                 .value_name("PORT")
                 .help("Port to use for gRPC API connection to the server.")
-                .default_value(DEFAULT_SERVER_GRPC_PORT)
+                .default_value(&DEFAULT_SERVER_GRPC_PORT)
                 .takes_value(true),
         )
         .arg(
@@ -53,7 +69,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .short("h")
                 .value_name("PORT")
                 .help("Port to use for HTTP API connection to the server.")
-                .default_value(DEFAULT_SERVER_HTTP_PORT)
+                .default_value(&DEFAULT_SERVER_HTTP_PORT)
                 .takes_value(true),
         )
         /*

validator_client/src/config.rs

@@ -1,5 +1,5 @@
+use account_manager::validator::ValidatorDirectory;
 use bincode;
-use bls::Keypair;
 use clap::ArgMatches;
 use serde_derive::{Deserialize, Serialize};
 use slog::{error, warn};
@@ -9,13 +9,9 @@ use std::ops::Range;
 use std::path::PathBuf;
 use types::{
     test_utils::{generate_deterministic_keypair, load_keypairs_from_yaml},
-    EthSpec, MainnetEthSpec,
+    EthSpec, Keypair, MainnetEthSpec,
 };
 
-pub const DEFAULT_SERVER: &str = "localhost";
-pub const DEFAULT_SERVER_GRPC_PORT: &str = "5051";
-pub const DEFAULT_SERVER_HTTP_PORT: &str = "5052";
-
 #[derive(Clone)]
 pub enum KeySource {
     /// Load the keypairs from disk.
@@ -58,16 +54,12 @@ impl Default for Config {
     /// Build a new configuration from defaults.
     fn default() -> Self {
         Self {
-            data_dir: PathBuf::from(".lighthouse-validator"),
+            data_dir: PathBuf::from(".lighthouse/validators"),
             key_source: <_>::default(),
             log_file: PathBuf::from(""),
-            server: DEFAULT_SERVER.into(),
-            server_grpc_port: DEFAULT_SERVER_GRPC_PORT
-                .parse::<u16>()
-                .expect("gRPC port constant should be valid"),
-            server_http_port: DEFAULT_SERVER_GRPC_PORT
-                .parse::<u16>()
-                .expect("HTTP port constant should be valid"),
+            server: "localhost".into(),
+            server_grpc_port: 5051,
+            server_http_port: 5052,
             slots_per_epoch: MainnetEthSpec::slots_per_epoch(),
         }
     }
@@ -106,75 +98,44 @@ impl Config {
         Ok(())
     }
 
-    /// Reads a single keypair from the given `path`.
+    /// Loads the validator keys from disk.
     ///
-    /// `path` should be the path to a directory containing a private key. The file name of `path`
-    /// must align with the public key loaded from it, otherwise an error is returned.
+    /// ## Errors
     ///
-    /// An error will be returned if `path` is a file (not a directory).
-    fn read_keypair_file(&self, path: PathBuf) -> Result<Keypair, String> {
-        if !path.is_dir() {
-            return Err("Is not a directory".into());
-        }
-
-        let key_filename: PathBuf = path.join(DEFAULT_PRIVATE_KEY_FILENAME);
-
-        if !key_filename.is_file() {
-            return Err(format!(
-                "Private key is not a file: {:?}",
-                key_filename.to_str()
-            ));
-        }
-
-        let mut key_file = File::open(key_filename.clone())
-            .map_err(|e| format!("Unable to open private key file: {}", e))?;
-
-        let key: Keypair = bincode::deserialize_from(&mut key_file)
-            .map_err(|e| format!("Unable to deserialize private key: {:?}", e))?;
-
-        let ki = key.identifier();
-        if ki
-            != path
-                .file_name()
-                .ok_or_else(|| "Invalid path".to_string())?
-                .to_string_lossy()
-        {
-            Err(format!(
-                "The validator key ({:?}) did not match the directory filename {:?}.",
-                ki,
-                path.to_str()
-            ))
-        } else {
-            Ok(key)
-        }
-    }
-
+    /// Returns an error if the base directory does not exist, however it does not return for any
+    /// invalid directories/files. Instead, it just filters out failures and logs errors. This
+    /// behaviour is intended to avoid the scenario where a single invalid file can stop all
+    /// validators.
     pub fn fetch_keys_from_disk(&self, log: &slog::Logger) -> Result<Vec<Keypair>, String> {
-        Ok(
-            fs::read_dir(&self.full_data_dir().expect("Data dir must exist"))
-                .map_err(|e| format!("Failed to read datadir: {:?}", e))?
-                .filter_map(|validator_dir| {
-                    let path = validator_dir.ok()?.path();
+        let base_dir = self
+            .full_data_dir()
+            .ok_or_else(|| format!("Base directory does not exist: {:?}", self.full_data_dir()))?;
 
-                    if path.is_dir() {
-                        match self.read_keypair_file(path.clone()) {
-                            Ok(keypair) => Some(keypair),
-                            Err(e) => {
-                                error!(
-                                    log,
-                                    "Failed to parse a validator keypair";
-                                    "error" => e,
-                                    "path" => path.to_str(),
-                                );
-                                None
-                            }
+        let keypairs = fs::read_dir(&base_dir)
+            .map_err(|e| format!("Failed to read base directory: {:?}", e))?
+            .filter_map(|validator_dir| {
+                let path = validator_dir.ok()?.path();
+
+                if path.is_dir() {
+                    match ValidatorDirectory::load_for_signing(path.clone()) {
+                        Ok(validator_directory) => validator_directory.voting_keypair,
+                        Err(e) => {
+                            error!(
+                                log,
+                                "Failed to load a validator directory";
+                                "error" => e,
+                                "path" => path.to_str(),
+                            );
+                            None
                         }
-                    } else {
-                        None
                     }
-                })
-                .collect(),
-        )
+                } else {
+                    None
+                }
+            })
+            .collect();
+
+        Ok(keypairs)
     }
 
     pub fn fetch_testing_keypairs(

validator_client/src/service.rs

@@ -14,7 +14,6 @@ use crate::config::Config as ValidatorConfig;
 use crate::duties::{BeaconNodeDuties, DutiesManager, EpochDutiesMap};
 use crate::error as error_chain;
 use crate::signer::Signer;
-use bls::Keypair;
 use eth2_config::Eth2Config;
 use grpcio::{ChannelBuilder, EnvBuilder};
 use parking_lot::RwLock;
@@ -28,7 +27,7 @@ use slot_clock::{SlotClock, SystemTimeSlotClock};
 use std::marker::PhantomData;
 use std::sync::Arc;
 use std::time::Duration;
-use types::{ChainSpec, Epoch, EthSpec, Fork, Slot};
+use types::{ChainSpec, Epoch, EthSpec, Fork, Keypair, Slot};
 
 /// The validator service. This is the main thread that executes and maintains validator
 /// duties.