diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 6eb7911139..f43dfcdb8f 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -1,4 +1,5 @@ use crate::wallet::create::{PASSWORD_FLAG, STDIN_INPUTS_FLAG}; +use account_utils::validator_definitions::SigningDefinition; use account_utils::{ eth2_keystore::Keystore, read_password_from_user, @@ -208,10 +209,35 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin } }; + let voting_pubkey = keystore + .public_key() + .ok_or_else(|| format!("Keystore public key is invalid: {}", keystore.pubkey()))?; + // The keystore is placed in a directory that matches the name of the public key. This // provides some loose protection against adding the same keystore twice. let dest_dir = validator_dir.join(format!("0x{}", keystore.pubkey())); if dest_dir.exists() { + // Check if we should update password for existing validator in case if it was provided via reimport: #2854 + let old_validator_def_opt = defs + .as_mut_slice() + .iter_mut() + .find(|def| def.voting_public_key == voting_pubkey); + if let Some(ValidatorDefinition { + signing_definition: + SigningDefinition::LocalKeystore { + voting_keystore_password: ref mut old_passwd, + .. + }, + .. + }) = old_validator_def_opt + { + if old_passwd.is_none() && password_opt.is_some() { + *old_passwd = password_opt; + defs.save(&validator_dir) + .map_err(|e| format!("Unable to save {}: {:?}", CONFIG_FILENAME, e))?; + eprintln!("Password updated for public key {}", voting_pubkey); + } + } eprintln!( "Skipping import of keystore for existing public key: {:?}", src_keystore @@ -234,9 +260,6 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin .map_err(|e| format!("Unable to copy keystore: {:?}", e))?; // Register with slashing protection. - let voting_pubkey = keystore - .public_key() - .ok_or_else(|| format!("Keystore public key is invalid: {}", keystore.pubkey()))?; slashing_protection .register_validator(voting_pubkey.compress()) .map_err(|e| { diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index d985a3d1a7..96be44fcad 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -22,7 +22,7 @@ use std::env; use std::fs::{self, File}; use std::io::{BufRead, BufReader, Write}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, Stdio}; +use std::process::{Child, Command, Output, Stdio}; use std::str::from_utf8; use tempfile::{tempdir, TempDir}; use types::{Keypair, PublicKey}; @@ -528,6 +528,128 @@ fn validator_import_launchpad() { ); } +#[test] +fn validator_import_launchpad_no_password_then_add_password() { + const PASSWORD: &str = "cats"; + const KEYSTORE_NAME: &str = "keystore-m_12381_3600_0_0_0-1595406747.json"; + const NOT_KEYSTORE_NAME: &str = "keystore-m_12381_3600_0_0-1595406747.json"; + + let src_dir = tempdir().unwrap(); + let dst_dir = tempdir().unwrap(); + + let keypair = Keypair::random(); + let keystore = KeystoreBuilder::new(&keypair, PASSWORD.as_bytes(), "".into()) + .unwrap() + .build() + .unwrap(); + + let dst_keystore_dir = dst_dir.path().join(format!("0x{}", keystore.pubkey())); + + // Create a keystore in the src dir. + File::create(src_dir.path().join(KEYSTORE_NAME)) + .map(|mut file| keystore.to_json_writer(&mut file).unwrap()) + .unwrap(); + + // Create a not-keystore file in the src dir. + File::create(src_dir.path().join(NOT_KEYSTORE_NAME)).unwrap(); + + let validator_import_key_cmd = || { + validator_cmd() + .arg(format!("--{}", VALIDATOR_DIR_FLAG)) + .arg(dst_dir.path().as_os_str()) + .arg(IMPORT_CMD) + .arg(format!("--{}", STDIN_INPUTS_FLAG)) // Using tty does not work well with tests. + .arg(format!("--{}", import::DIR_FLAG)) + .arg(src_dir.path().as_os_str()) + .stderr(Stdio::piped()) + .stdin(Stdio::piped()) + .spawn() + .unwrap() + }; + + let wait_for_password_prompt = |child: &mut Child| { + let mut stderr = child.stderr.as_mut().map(BufReader::new).unwrap().lines(); + + loop { + if stderr.next().unwrap().unwrap() == import::PASSWORD_PROMPT { + break; + } + } + }; + + let mut child = validator_import_key_cmd(); + wait_for_password_prompt(&mut child); + let stdin = child.stdin.as_mut().unwrap(); + stdin.write("\n".as_bytes()).unwrap(); + child.wait().unwrap(); + + assert!( + src_dir.path().join(KEYSTORE_NAME).exists(), + "keystore should not be removed from src dir" + ); + assert!( + src_dir.path().join(NOT_KEYSTORE_NAME).exists(), + "not-keystore should not be removed from src dir." + ); + + let voting_keystore_path = dst_keystore_dir.join(KEYSTORE_NAME); + + assert!( + voting_keystore_path.exists(), + "keystore should be present in dst dir" + ); + assert!( + !dst_dir.path().join(NOT_KEYSTORE_NAME).exists(), + "not-keystore should not be present in dst dir" + ); + + // Validator should be registered with slashing protection. + check_slashing_protection(&dst_dir, std::iter::once(keystore.public_key().unwrap())); + + let defs = ValidatorDefinitions::open(&dst_dir).unwrap(); + + let expected_def = ValidatorDefinition { + enabled: true, + description: "".into(), + graffiti: None, + voting_public_key: keystore.public_key().unwrap(), + signing_definition: SigningDefinition::LocalKeystore { + voting_keystore_path, + voting_keystore_password_path: None, + voting_keystore_password: None, + }, + }; + + assert!( + defs.as_slice() == &[expected_def.clone()], + "validator defs file should be accurate" + ); + + let mut child = validator_import_key_cmd(); + wait_for_password_prompt(&mut child); + let stdin = child.stdin.as_mut().unwrap(); + stdin.write(format!("{}\n", PASSWORD).as_bytes()).unwrap(); + child.wait().unwrap(); + + let expected_def = ValidatorDefinition { + enabled: true, + description: "".into(), + graffiti: None, + voting_public_key: keystore.public_key().unwrap(), + signing_definition: SigningDefinition::LocalKeystore { + voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME), + voting_keystore_password_path: None, + voting_keystore_password: Some(ZeroizeString::from(PASSWORD.to_string())), + }, + }; + + let defs = ValidatorDefinitions::open(&dst_dir).unwrap(); + assert!( + defs.as_slice() == &[expected_def.clone()], + "validator defs file should be accurate" + ); +} + #[test] fn validator_import_launchpad_password_file() { const PASSWORD: &str = "cats";