From fa6eba911ef93d81fe7d35f96133c1d22ae1ae5c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 22 Aug 2022 09:17:46 +1000 Subject: [PATCH] Start adding import validator testing --- Cargo.lock | 1 + validator_client/src/http_api/test_utils.rs | 6 +- validator_manager/Cargo.toml | 1 + .../src/validators/import_validators.rs | 281 ++++++++++-------- 4 files changed, 171 insertions(+), 118 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 899dcb09d8..5ed823ccd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7378,6 +7378,7 @@ dependencies = [ "tokio", "tree_hash", "types", + "validator_client", ] [[package]] diff --git a/validator_client/src/http_api/test_utils.rs b/validator_client/src/http_api/test_utils.rs index 3e670c4914..25af9be6fe 100644 --- a/validator_client/src/http_api/test_utils.rs +++ b/validator_client/src/http_api/test_utils.rs @@ -56,6 +56,7 @@ pub struct ApiTester { pub initialized_validators: Arc>, pub validator_store: Arc>, pub url: SensitiveUrl, + pub api_token: String, pub test_runtime: TestRuntime, pub _server_shutdown: oneshot::Sender<()>, pub _validator_dir: TempDir, @@ -116,7 +117,7 @@ impl ApiTester { let context = Arc::new(Context { task_executor: test_runtime.task_executor.clone(), - api_secret, + api_secret: api_secret, validator_dir: Some(validator_dir.path().into()), validator_store: Some(validator_store.clone()), spec: E::default_spec(), @@ -146,13 +147,14 @@ impl ApiTester { )) .unwrap(); - let client = ValidatorClientHttpClient::new(url.clone(), api_pubkey).unwrap(); + let client = ValidatorClientHttpClient::new(url.clone(), api_pubkey.clone()).unwrap(); Self { client, initialized_validators, validator_store, url, + api_token: api_pubkey, test_runtime, _server_shutdown: shutdown_tx, _validator_dir: validator_dir, diff --git a/validator_manager/Cargo.toml b/validator_manager/Cargo.toml index 38cbbae669..6e56d52310 100644 --- a/validator_manager/Cargo.toml +++ b/validator_manager/Cargo.toml @@ -27,3 +27,4 @@ tempfile = "3.1.0" tokio = { version = "1.14.0", features = ["time", "rt-multi-thread", "macros"] } regex = "1.6.0" eth2_network_config = { path = "../common/eth2_network_config" } +validator_client = { path = "../validator_client" } diff --git a/validator_manager/src/validators/import_validators.rs b/validator_manager/src/validators/import_validators.rs index ffdb88bf87..4ee6b0247a 100644 --- a/validator_manager/src/validators/import_validators.rs +++ b/validator_manager/src/validators/import_validators.rs @@ -70,8 +70,38 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) } +#[derive(Clone)] +struct ImportConfig { + validators_file_path: PathBuf, + vc_url: SensitiveUrl, + vc_token_path: PathBuf, + ignore_duplicates: bool, +} + +impl ImportConfig { + fn from_cli(matches: &ArgMatches) -> Result { + Ok(Self { + validators_file_path: clap_utils::parse_required(matches, VALIDATORS_FILE_FLAG)?, + vc_url: clap_utils::parse_required(matches, VALIDATOR_CLIENT_URL_FLAG)?, + vc_token_path: clap_utils::parse_required(matches, VALIDATOR_CLIENT_TOKEN_FLAG)?, + ignore_duplicates: matches.is_present(IGNORE_DUPLICATES_FLAG), + }) + } +} + pub async fn cli_run<'a>(matches: &'a ArgMatches<'a>) -> Result<(), String> { - let validators_file_path: PathBuf = clap_utils::parse_required(matches, VALIDATORS_FILE_FLAG)?; + let config = ImportConfig::from_cli(matches)?; + run(config).await +} + +async fn run<'a>(config: ImportConfig) -> Result<(), String> { + let ImportConfig { + validators_file_path, + vc_url, + vc_token_path, + ignore_duplicates, + } = config; + if !validators_file_path.exists() { return Err(format!("Unable to find file at {:?}", validators_file_path)); } @@ -81,136 +111,155 @@ pub async fn cli_run<'a>(matches: &'a ArgMatches<'a>) -> Result<(), String> { .create(false) .open(&validators_file_path) .map_err(|e| format!("Unable to open {:?}: {:?}", validators_file_path, e))?; - let validators = serde_json::from_reader(&validators_file).map_err(|e| { - format!( - "Unable to parse JSON in {:?}: {:?}", - validators_file_path, e - ) - })?; + let validators: Vec = serde_json::from_reader(&validators_file) + .map_err(|e| { + format!( + "Unable to parse JSON in {:?}: {:?}", + validators_file_path, e + ) + })?; - import_validators(matches, validators).await -} - -pub async fn import_validators<'a>( - matches: &'a ArgMatches<'a>, - validators: Vec, -) -> Result<(), String> { let count = validators.len(); - let vc_url: Option = - clap_utils::parse_optional(matches, VALIDATOR_CLIENT_URL_FLAG)?; - let vc_token_path: Option = - clap_utils::parse_optional(matches, VALIDATOR_CLIENT_TOKEN_FLAG)?; + let http_client = { + let token_bytes = fs::read(&vc_token_path) + .map_err(|e| format!("Failed to read {:?}: {:?}", vc_token_path, e))?; + let token_string = String::from_utf8(token_bytes) + .map_err(|e| format!("Failed to parse {:?} as utf8: {:?}", vc_token_path, e))?; + let http_client = + ValidatorClientHttpClient::new(vc_url.clone(), token_string).map_err(|e| { + format!( + "Could not instantiate HTTP client from URL and secret: {:?}", + e + ) + })?; - let http_client = match (vc_url, vc_token_path) { - (Some(vc_url), Some(vc_token_path)) => { - let token_bytes = fs::read(&vc_token_path) - .map_err(|e| format!("Failed to read {:?}: {:?}", vc_token_path, e))?; - let token_string = String::from_utf8(token_bytes) - .map_err(|e| format!("Failed to parse {:?} as utf8: {:?}", vc_token_path, e))?; - let http_client = ValidatorClientHttpClient::new(vc_url.clone(), token_string) - .map_err(|e| { - format!( - "Could not instantiate HTTP client from URL and secret: {:?}", - e - ) - })?; - - // Perform a request to check that the connection works - let remote_keystores = http_client - .get_keystores() - .await - .map_err(|e| format!("Failed to list keystores on VC: {:?}", e))?; - eprintln!( - "Validator client is reachable at {} and reports {} validators", - vc_url, - remote_keystores.data.len() - ); - - Some(http_client) - } - _ => { - return Err(format!( - "Inconsistent use of {} and {}", - VALIDATOR_CLIENT_URL_FLAG, VALIDATOR_CLIENT_TOKEN_FLAG - )) - } - }; - - if let Some(http_client) = http_client { + // Perform a request to check that the connection works + let remote_keystores = http_client + .get_keystores() + .await + .map_err(|e| format!("Failed to list keystores on VC: {:?}", e))?; eprintln!( - "Starting to submit validators {} to VC, each validator may take several seconds", - count + "Validator client is reachable at {} and reports {} validators", + vc_url, + remote_keystores.data.len() ); - for (i, validator) in validators.into_iter().enumerate() { - let ValidatorSpecification { - voting_keystore, - voting_keystore_password, - slashing_protection, - fee_recipient, - gas_limit, - builder_proposals, - enabled, - } = validator; + http_client + }; - let voting_public_key = voting_keystore - .public_key() - .ok_or_else(|| { - format!("Validator keystore at index {} is missing a public key", i) - })? - .into(); + eprintln!( + "Starting to submit validators {} to VC, each validator may take several seconds", + count + ); - let request = ImportKeystoresRequest { - keystores: vec![voting_keystore], - passwords: vec![voting_keystore_password], - slashing_protection, - }; + for (i, validator) in validators.into_iter().enumerate() { + let ValidatorSpecification { + voting_keystore, + voting_keystore_password, + slashing_protection, + fee_recipient, + gas_limit, + builder_proposals, + enabled, + } = validator; - if let Err(e) = http_client.post_keystores(&request).await { - eprintln!( - "Failed to upload batch {}. Some keys were imported whilst \ + let voting_public_key = voting_keystore + .public_key() + .ok_or_else(|| format!("Validator keystore at index {} is missing a public key", i))? + .into(); + + let request = ImportKeystoresRequest { + keystores: vec![voting_keystore], + passwords: vec![voting_keystore_password], + slashing_protection, + }; + + if let Err(e) = http_client.post_keystores(&request).await { + eprintln!( + "Failed to upload batch {}. Some keys were imported whilst \ others may not have been imported. A potential solution is to use the \ --{} flag, however care should be taken to ensure that there are no \ duplicate deposits submitted.", - i, IGNORE_DUPLICATES_FLAG - ); - // Return here *without* writing the deposit JSON file. This might help prevent - // users from submitting duplicate deposits or deposits for validators that weren't - // initialized on a VC. - // - // Next the the user runs with the --ignore-duplicates flag there should be a new, - // complete deposit JSON file created. - return Err(format!("Key upload failed: {:?}", e)); - } - - if let Some(fee_recipient) = fee_recipient { - http_client - .post_fee_recipient( - &voting_public_key, - &UpdateFeeRecipientRequest { - ethaddress: fee_recipient, - }, - ) - .await - .map_err(|e| format!("Failed to update fee recipient on VC: {:?}", e))?; - } - - if gas_limit.is_some() || builder_proposals.is_some() || enabled.is_some() { - http_client - .patch_lighthouse_validators( - &voting_public_key, - enabled, - gas_limit, - builder_proposals, - ) - .await - .map_err(|e| format!("Failed to update lighthouse validator on VC: {:?}", e))?; - } - - eprintln!("Uploaded keystore {} of {} to the VC", i + 1, count); + i, IGNORE_DUPLICATES_FLAG + ); + // Return here *without* writing the deposit JSON file. This might help prevent + // users from submitting duplicate deposits or deposits for validators that weren't + // initialized on a VC. + // + // Next the the user runs with the --ignore-duplicates flag there should be a new, + // complete deposit JSON file created. + return Err(format!("Key upload failed: {:?}", e)); } + + if let Some(fee_recipient) = fee_recipient { + http_client + .post_fee_recipient( + &voting_public_key, + &UpdateFeeRecipientRequest { + ethaddress: fee_recipient, + }, + ) + .await + .map_err(|e| format!("Failed to update fee recipient on VC: {:?}", e))?; + } + + if gas_limit.is_some() || builder_proposals.is_some() || enabled.is_some() { + http_client + .patch_lighthouse_validators( + &voting_public_key, + enabled, + gas_limit, + builder_proposals, + ) + .await + .map_err(|e| format!("Failed to update lighthouse validator on VC: {:?}", e))?; + } + + eprintln!("Uploaded keystore {} of {} to the VC", i + 1, count); } Ok(()) } + +#[cfg(test)] +mod test { + use super::*; + use std::fs; + use tempfile::{tempdir, TempDir}; + use validator_client::http_api::test_utils::ApiTester; + + const VALIDATORS_FILE_NAME: &str = "validators.json"; + const VC_TOKEN_FILE_NAME: &str = "vc_token.json"; + + struct TestBuilder { + config: ImportConfig, + vc: ApiTester, + dir: TempDir, + } + + impl TestBuilder { + async fn new() -> Self { + let dir = tempdir().unwrap(); + let vc = ApiTester::new().await; + let vc_token_path = dir.path().join(VC_TOKEN_FILE_NAME); + fs::write(&vc_token_path, &vc.api_token).unwrap(); + + Self { + config: ImportConfig { + validators_file_path: dir.path().join(VALIDATORS_FILE_NAME), + vc_url: vc.url.clone(), + vc_token_path, + ignore_duplicates: false, + }, + vc, + dir, + } + } + } + + #[tokio::test] + async fn blah() { + TestBuilder::new().await; + } +}