Merge branch 'v0.3.0-staging' into directory-restructure

beacon_node/src/lib.rs

@@ -7,7 +7,7 @@ mod config;
 pub use beacon_chain;
 pub use cli::cli_app;
 pub use client::{Client, ClientBuilder, ClientConfig, ClientGenesis};
-pub use config::{get_data_dir, get_eth2_testnet_config, set_network_config};
+pub use config::{get_config, get_data_dir, get_eth2_testnet_config, set_network_config};
 pub use eth2_config::Eth2Config;
 
 use beacon_chain::events::TeeEventHandler;
@@ -17,7 +17,6 @@ use beacon_chain::{
     builder::Witness, eth1_chain::CachingEth1Backend, slot_clock::SystemTimeSlotClock,
 };
 use clap::ArgMatches;
-use config::get_config;
 use environment::RuntimeContext;
 use slog::{info, warn};
 use std::ops::{Deref, DerefMut};
@@ -54,7 +53,7 @@ impl<E: EthSpec> ProductionBeaconNode<E> {
     /// configurations hosted remotely.
     pub async fn new_from_cli(
         context: RuntimeContext<E>,
-        matches: &ArgMatches<'_>,
+        matches: ArgMatches<'static>,
     ) -> Result<Self, String> {
         let client_config = get_config::<E>(
             &matches,

crypto/eth2_key_derivation/tests/eip2333_vectors.rs

@@ -65,38 +65,38 @@ fn assert_vector_passes(raw: RawTestVector) {
 fn eip2333_test_case_0() {
     assert_vector_passes(RawTestVector {
         seed: "0xc55257c360c07c72029aebc1b53c05ed0362ada38ead3e3e9efa3708e53495531f09a6987599d18264c1e1c92f2cf141630c7a3c4ab7c81b2f001698e7463b04",
-        master_sk: "12513733877922233913083619867448865075222526338446857121953625441395088009793",
+        master_sk: "6083874454709270928345386274498605044986640685124978867557563392430687146096",
         child_index: 0,
-        child_sk: "7419543105316279183937430842449358701327973165530407166294956473095303972104"
+        child_sk: "20397789859736650942317412262472558107875392172444076792671091975210932703118",
     })
 }
 
 #[test]
 fn eip2333_test_case_1() {
     assert_vector_passes(RawTestVector {
         seed: "0x3141592653589793238462643383279502884197169399375105820974944592",
-        master_sk: "46029459550803682895343812821003080589696405386150182061394330539196052371668",
+        master_sk: "29757020647961307431480504535336562678282505419141012933316116377660817309383",
         child_index: 3141592653,
-        child_sk: "43469287647733616183478983885105537266268532274998688773496918571876759327260",
+        child_sk: "25457201688850691947727629385191704516744796114925897962676248250929345014287",
     })
 }
 
 #[test]
 fn eip2333_test_case_2() {
     assert_vector_passes(RawTestVector {
         seed: "0x0099FF991111002299DD7744EE3355BBDD8844115566CC55663355668888CC00",
-        master_sk: "45379166311535261329029945990467475187325618028073620882733843918126031931161",
+        master_sk: "27580842291869792442942448775674722299803720648445448686099262467207037398656",
         child_index: 4294967295,
-        child_sk: "46475244006136701976831062271444482037125148379128114617927607151318277762946",
+        child_sk: "29358610794459428860402234341874281240803786294062035874021252734817515685787",
     })
 }
 
 #[test]
 fn eip2333_test_case_3() {
     assert_vector_passes(RawTestVector {
         seed: "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
-        master_sk: "31740500954810567003972734830331791822878290325762596213711963944729383643688",
+        master_sk: "19022158461524446591288038168518313374041767046816487870552872741050760015818",
         child_index: 42,
-        child_sk: "51041472511529980987749393477251359993058329222191894694692317000136653813011",
+        child_sk: "31372231650479070279774297061823572166496564838472787488249775572789064611981",
     })
 }

lighthouse/src/main.rs

@@ -299,61 +299,63 @@ fn run<E: EthSpec>(
         "name" => testnet_name
     );
 
-    let beacon_node = if let Some(sub_matches) = matches.subcommand_matches("beacon_node") {
-        let runtime_context = environment.core_context();
-
-        let beacon = environment
-            .runtime()
-            .block_on(ProductionBeaconNode::new_from_cli(
-                runtime_context,
-                sub_matches,
-            ))
-            .map_err(|e| format!("Failed to start beacon node: {}", e))?;
-
-        Some(beacon)
-    } else {
-        None
-    };
-
-    let validator_client = if let Some(sub_matches) = matches.subcommand_matches("validator_client")
-    {
-        let runtime_context = environment.core_context();
-
-        let mut validator = environment
-            .runtime()
-            .block_on(ProductionValidatorClient::new_from_cli(
-                runtime_context,
-                sub_matches,
-            ))
-            .map_err(|e| format!("Failed to init validator client: {}", e))?;
-
-        environment
-            .core_context()
-            .executor
-            .runtime_handle()
-            .enter(|| {
-                validator
-                    .start_service()
-                    .map_err(|e| format!("Failed to start validator client service: {}", e))
-            })?;
-
-        Some(validator)
-    } else {
-        None
+    match matches.subcommand() {
+        ("beacon_node", Some(matches)) => {
+            let context = environment.core_context();
+            let log = context.log().clone();
+            let executor = context.executor.clone();
+            let config = beacon_node::get_config::<E>(
+                matches,
+                &context.eth2_config.spec_constants,
+                &context.eth2_config().spec,
+                context.log().clone(),
+            )?;
+            environment.runtime().spawn(async move {
+                if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await {
+                    crit!(log, "Failed to start beacon node"; "reason" => e);
+                    // Ignore the error since it always occurs during normal operation when
+                    // shutting down.
+                    let _ = executor
+                        .shutdown_sender()
+                        .try_send("Failed to start beacon node");
+                }
+            })
+        }
+        ("validator_client", Some(matches)) => {
+            let context = environment.core_context();
+            let log = context.log().clone();
+            let executor = context.executor.clone();
+            let config = validator_client::Config::from_cli(&matches)
+                .map_err(|e| format!("Unable to initialize validator config: {}", e))?;
+            environment.runtime().spawn(async move {
+                let run = async {
+                    ProductionValidatorClient::new(context, config)
+                        .await?
+                        .start_service()?;
+
+                    Ok::<(), String>(())
+                };
+                if let Err(e) = run.await {
+                    crit!(log, "Failed to start validator client"; "reason" => e);
+                    // Ignore the error since it always occurs during normal operation when
+                    // shutting down.
+                    let _ = executor
+                        .shutdown_sender()
+                        .try_send("Failed to start validator client");
+                }
+            })
+        }
+        _ => {
+            crit!(log, "No subcommand supplied. See --help .");
+            return Err("No subcommand supplied.".into());
+        }
     };
 
-    if beacon_node.is_none() && validator_client.is_none() {
-        crit!(log, "No subcommand supplied. See --help .");
-        return Err("No subcommand supplied.".into());
-    }
-
     // Block this thread until we get a ctrl-c or a task sends a shutdown signal.
     environment.block_until_shutdown_requested()?;
     info!(log, "Shutting down..");
 
     environment.fire_signal();
-    drop(beacon_node);
-    drop(validator_client);
 
     // Shutdown the environment once all tasks have completed.
     environment.shutdown_on_idle();

validator_client/src/cli.rs

@@ -51,11 +51,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                        nodes using the same key. Automatically enabled unless `--strict` is specified",
         ))
         .arg(
-            Arg::with_name("strict-lockfiles")
-            .long("strict-lockfiles")
+            Arg::with_name("delete-lockfiles")
+            .long("delete-lockfiles")
             .help(
-                "If present, do not load validators that are guarded by a lockfile. Note: for \
-                Eth2 mainnet, this flag will likely be removed and its behaviour will become default."
+                "If present, ignore and delete any keystore lockfiles encountered during start up. \
+                This is useful if the validator client did not exit gracefully on the last run. \
+                WARNING: lockfiles help prevent users from accidentally running the same validator \
+                using two different validator clients, an action that likely leads to slashing. \
+                Ensure you are certain that there are no other validator client instances running \
+                that might also be using the same keystores."
             )
         )
         .arg(

validator_client/src/config.rs

@@ -26,8 +26,8 @@ pub struct Config {
     /// If true, the validator client will still poll for duties and produce blocks even if the
     /// beacon node is not synced at startup.
     pub allow_unsynced_beacon_node: bool,
-    /// If true, refuse to unlock a keypair that is guarded by a lockfile.
-    pub strict_lockfiles: bool,
+    /// If true, delete any validator keystore lockfiles that would prevent starting.
+    pub delete_lockfiles: bool,
     /// If true, don't scan the validators dir for new keystores.
     pub disable_auto_discover: bool,
     /// If true, don't re-register existing validators in definitions.yml for slashing protection.
@@ -52,7 +52,7 @@ impl Default for Config {
             secrets_dir,
             http_server: DEFAULT_HTTP_SERVER.to_string(),
             allow_unsynced_beacon_node: false,
-            strict_lockfiles: false,
+            delete_lockfiles: false,
             disable_auto_discover: false,
             strict_slashing_protection: false,
             graffiti: None,
@@ -107,7 +107,7 @@ impl Config {
         }
 
         config.allow_unsynced_beacon_node = cli_args.is_present("allow-unsynced");
-        config.strict_lockfiles = cli_args.is_present("strict-lockfiles");
+        config.delete_lockfiles = cli_args.is_present("delete-lockfiles");
         config.disable_auto_discover = cli_args.is_present("disable-auto-discover");
         config.strict_slashing_protection = cli_args.is_present("strict-slashing-protection");
 

validator_client/src/initialized_validators.rs

@@ -54,6 +54,10 @@ pub enum Error {
     PasswordUnknown(PathBuf),
     /// There was an error reading from stdin.
     UnableToReadPasswordFromUser(String),
+    /// There was an error running a tokio async task.
+    TokioJoin(tokio::task::JoinError),
+    /// There was a filesystem error when deleting a lockfile.
+    UnableToDeleteLockfile(io::Error),
 }
 
 /// A method used by a validator to sign messages.
@@ -86,7 +90,7 @@ impl InitializedValidator {
     /// If the validator is unable to be initialized for whatever reason.
     pub fn from_definition(
         def: ValidatorDefinition,
-        strict_lockfiles: bool,
+        delete_lockfiles: bool,
         log: &Logger,
     ) -> Result<Self, Error> {
         if !def.enabled {
@@ -150,16 +154,17 @@ impl InitializedValidator {
                     })?;
 
                 if voting_keystore_lockfile_path.exists() {
-                    if strict_lockfiles {
-                        return Err(Error::LockfileExists(voting_keystore_lockfile_path));
-                    } else {
-                        // If **not** respecting lockfiles, just raise a warning if the voting
-                        // keypair cannot be unlocked.
+                    if delete_lockfiles {
                         warn!(
                             log,
-                            "Ignoring validator lockfile";
+                            "Deleting validator lockfile";
                             "file" => format!("{:?}", voting_keystore_lockfile_path)
                         );
+
+                        fs::remove_file(&voting_keystore_lockfile_path)
+                            .map_err(Error::UnableToDeleteLockfile)?;
+                    } else {
+                        return Err(Error::LockfileExists(voting_keystore_lockfile_path));
                     }
                 } else {
                     // Create a new lockfile.
@@ -279,7 +284,7 @@ pub struct InitializedValidators {
 
 impl InitializedValidators {
     /// Instantiates `Self`, initializing all validators in `definitions`.
-    pub fn from_definitions(
+    pub async fn from_definitions(
         definitions: ValidatorDefinitions,
         validators_dir: PathBuf,
         strict_lockfiles: bool,
@@ -292,7 +297,7 @@ impl InitializedValidators {
             validators: HashMap::default(),
             log,
         };
-        this.update_validators()?;
+        this.update_validators().await?;
         Ok(this)
     }
 
@@ -328,7 +333,7 @@ impl InitializedValidators {
     /// validator will be removed from `self.validators`.
     ///
     /// Saves the `ValidatorDefinitions` to file, even if no definitions were changed.
-    pub fn set_validator_status(
+    pub async fn set_validator_status(
         &mut self,
         voting_public_key: &PublicKey,
         enabled: bool,
@@ -342,7 +347,7 @@ impl InitializedValidators {
             def.enabled = enabled;
         }
 
-        self.update_validators()?;
+        self.update_validators().await?;
 
         self.definitions
             .save(&self.validators_dir)
@@ -362,7 +367,7 @@ impl InitializedValidators {
     /// A validator is considered "already known" and skipped if the public key is already known.
     /// I.e., if there are two different definitions with the same public key then the second will
     /// be ignored.
-    fn update_validators(&mut self) -> Result<(), Error> {
+    async fn update_validators(&mut self) -> Result<(), Error> {
         for def in self.definitions.as_slice() {
             if def.enabled {
                 match &def.signing_definition {
@@ -371,11 +376,23 @@ impl InitializedValidators {
                             continue;
                         }
 
-                        match InitializedValidator::from_definition(
-                            def.clone(),
-                            self.strict_lockfiles,
-                            &self.log,
-                        ) {
+                        // Decoding a local keystore can take several seconds, therefore it's best
+                        // to keep if off the core executor. This also has the fortunate effect of
+                        // interrupting the potentially long-running task during shut down.
+                        let inner_def = def.clone();
+                        let strict_lockfiles = self.strict_lockfiles;
+                        let inner_log = self.log.clone();
+                        let result = tokio::task::spawn_blocking(move || {
+                            InitializedValidator::from_definition(
+                                inner_def,
+                                strict_lockfiles,
+                                &inner_log,
+                            )
+                        })
+                        .await
+                        .map_err(Error::TokioJoin)?;
+
+                        match result {
                             Ok(init) => {
                                 self.validators
                                     .insert(init.voting_public_key().clone(), init);