diff --git a/Cargo.lock b/Cargo.lock index 94eb9038447..a2e9ebad227 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9216,6 +9216,7 @@ dependencies = [ "account_utils", "clap", "clap_utils", + "derivative", "environment", "eth2", "eth2_network_config", diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 7fb0b2f4e70..cc9b1c43e40 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -15,6 +15,7 @@ * [The `validator-manager` Command](./validator-manager.md) * [Creating validators](./validator-manager-create.md) * [Moving validators](./validator-manager-move.md) + * [Managing validators](./validator-manager-api.md) * [Slashing Protection](./slashing-protection.md) * [Voluntary Exits](./voluntary-exit.md) * [Partial Withdrawals](./partial-withdrawal.md) diff --git a/book/src/help_vm.md b/book/src/help_vm.md index f787985b215..302b8797c8b 100644 --- a/book/src/help_vm.md +++ b/book/src/help_vm.md @@ -23,6 +23,11 @@ Commands: "create-validators" command. This command only supports validators signing via a keystore on the local file system (i.e., not Web3Signer validators). + list + Lists all validators in a validator client using the HTTP API. + delete + Deletes one or more validators from a validator client using the HTTP + API. help Print this message or the help of the given subcommand(s) diff --git a/book/src/help_vm_import.md b/book/src/help_vm_import.md index 0883139ad21..93affee547e 100644 --- a/book/src/help_vm_import.md +++ b/book/src/help_vm_import.md @@ -8,6 +8,11 @@ command. Usage: lighthouse validator_manager import [OPTIONS] --validators-file Options: + --builder-boost-factor + When provided, the imported validator will use this percentage + multiplier to apply to the builder's payload value when choosing + between a builder payload header and payload from the local execution + node. -d, --datadir Used to specify a custom root data directory for lighthouse keys and databases. Defaults to $HOME/.lighthouse/{network} where network is @@ -17,6 +22,10 @@ Options: Specifies the verbosity level used when emitting logs to the terminal. [default: info] [possible values: info, debug, trace, warn, error, crit] + --gas-limit + When provided, the imported validator will use this gas limit. It is + recommended to leave this as the default value by not specifying this + flag. --genesis-state-url A URL of a beacon-API compatible server from which to download the genesis state. Checkpoint sync server URLs can generally be used with @@ -50,6 +59,11 @@ Options: --network Name of the Eth2 chain Lighthouse will sync and follow. [possible values: mainnet, gnosis, chiado, sepolia, holesky] + --password + Password of the keystore file. + --prefer-builder-proposals + When provided, the imported validator will always prefer blocks + constructed by builders, regardless of payload value. --safe-slots-to-import-optimistically Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY parameter. This flag should only @@ -58,6 +72,9 @@ Options: attack at the PoS transition block. Incorrect use of this flag can cause your node to possibly accept an invalid chain or sync more slowly. Be extremely careful with this flag. + --suggested-fee-recipient + When provided, the imported validator will use the suggested fee + recipient. Omit this flag to use the default value from the VC. -t, --testnet-dir Path to directory containing eth2_testnet specs. Defaults to a hard-coded Lighthouse testnet. Only effective if there is no existing @@ -95,6 +112,9 @@ Options: http://localhost:5062] Flags: + --builder-proposals + When provided, the imported validator will attempt to create blocks + via builder rather than the local EL. --disable-log-timestamp If present, do not include timestamps in logging output. --disable-malloc-tuning @@ -121,6 +141,9 @@ Flags: contain sensitive information about your validator and so this flag should be used with caution. For Windows users, the log file permissions will be inherited from the parent folder. + --standard-format + Use this flag when the validator keystore files are generated using + staking-deposit-cli or ethstaker-deposit-cli. --stdin-inputs If present, read all user inputs from stdin instead of tty. ``` diff --git a/book/src/validator-manager-api.md b/book/src/validator-manager-api.md new file mode 100644 index 00000000000..31ac833fbb3 --- /dev/null +++ b/book/src/validator-manager-api.md @@ -0,0 +1,39 @@ +# Managing Validators + +The `lighthouse validator-manager` supports the [Keymanager API](https://ethereum.github.io/keymanager-APIs/#/) to list, import and delete keystores via the HTTP API. This requires the validator client running with the flag `--http`. + +## Delete + +The `delete` command deletes one or more validators from the validator client. It will also modify the `validator_definitions.yml` file automatically so there is no manual action required from the user after the delete. To `delete`: + +```bash +lighthouse vm delete --vc-token --validators pubkey1, pubkey2 +``` + +Example: + +```bash +lighthouse vm remove --vc-token ~/.lighthouse/mainnet/validators/api-token.txt --validators 0x8885c29b8f88ee9b9a37b480fd4384fed74bda33d85bc8171a904847e65688b6c9bb4362d6597fd30109fb2def6c3ae4, 0xa262dae3dcd2b2e280af534effa16bedb27c06f2959e114d53bd2a248ca324a018dc73179899a066149471a94a1bc92f +``` + +## Import + +The `import` command imports validator keystores generated by the staking-deposit-cli. To import validator keystore: + +```bash +lighthouse vm import --standard-format --vc-token --validators-file /path/to/json --password keystore_password +``` + +Example: + +``` +lighthouse vm import --standard-format --vc-token ~/.lighthouse/mainnet/validators/api-token.txt --validator-file keystore.json --password keystore_password +``` + +## List + +To list the validators running on the validator client: + +```bash +lighthouse vm list --vc-token ~/.lighthouse/mainnet/validators/api-token.txt +``` diff --git a/book/src/validator-manager-create.md b/book/src/validator-manager-create.md index d97f953fc19..03e06de6a60 100644 --- a/book/src/validator-manager-create.md +++ b/book/src/validator-manager-create.md @@ -69,6 +69,8 @@ lighthouse \ > Be sure to remove `./validators.json` after the import is successful since it > contains unencrypted validator keystores. +> Note: To import validators with validator-manager using keystore files created using the staking deposit cli, refer to [Managing Validators](./validator-manager-api.md#import). + ## Detailed Guide This guide will create two validators and import them to a VC. For simplicity, diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index 2c8bbbf4b4e..c1fa621abb1 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -9,13 +9,16 @@ use eth2_wallet::{ use filesystem::{create_with_600_perms, Error as FsError}; use rand::{distributions::Alphanumeric, Rng}; use serde::{Deserialize, Serialize}; -use std::fs::{self, File}; use std::io; use std::io::prelude::*; use std::path::{Path, PathBuf}; use std::str::from_utf8; use std::thread::sleep; use std::time::Duration; +use std::{ + fs::{self, File}, + str::FromStr, +}; use zeroize::Zeroize; pub mod validator_definitions; @@ -215,6 +218,14 @@ pub fn mnemonic_from_phrase(phrase: &str) -> Result { #[serde(transparent)] pub struct ZeroizeString(String); +impl FromStr for ZeroizeString { + type Err = String; + + fn from_str(s: &str) -> Result { + Ok(Self(s.to_owned())) + } +} + impl From for ZeroizeString { fn from(s: String) -> Self { Self(s) diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index bca6a18ab56..483d0f4fb24 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/lighthouse/tests/validator_manager.rs @@ -203,6 +203,14 @@ pub fn validator_import_defaults() { vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(), vc_token_path: PathBuf::from("./token.json"), ignore_duplicates: false, + password: None, + fee_recipient: None, + builder_boost_factor: None, + gas_limit: None, + builder_proposals: Some(false), + enabled: None, + prefer_builder_proposals: Some(false), + standard_format: false, }; assert_eq!(expected, config); }); @@ -220,6 +228,14 @@ pub fn validator_import_misc_flags() { vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(), vc_token_path: PathBuf::from("./token.json"), ignore_duplicates: true, + password: None, + fee_recipient: None, + builder_boost_factor: None, + gas_limit: None, + builder_proposals: Some(false), + enabled: None, + prefer_builder_proposals: Some(false), + standard_format: false, }; assert_eq!(expected, config); }); diff --git a/validator_client/src/http_api/keystores.rs b/validator_client/src/http_api/keystores.rs index 074c5783475..e5477ff8df7 100644 --- a/validator_client/src/http_api/keystores.rs +++ b/validator_client/src/http_api/keystores.rs @@ -75,12 +75,6 @@ pub fn import( ))); } - info!( - log, - "Importing keystores via standard HTTP API"; - "count" => request.keystores.len(), - ); - // Import slashing protection data before keystores, so that new keystores don't start signing // without it. Do not return early on failure, propagate the failure to each key. let slashing_protection_status = @@ -156,6 +150,19 @@ pub fn import( statuses.push(status); } + let successful_import = statuses + .iter() + .filter(|status| matches!(status.status, ImportKeystoreStatus::Imported)) + .count(); + + if successful_import > 0 { + info!( + log, + "Imported keystores via standard HTTP API"; + "count" => successful_import, + ); + } + Ok(ImportKeystoresResponse { data: statuses }) } @@ -238,7 +245,23 @@ pub fn delete( task_executor: TaskExecutor, log: Logger, ) -> Result { - let export_response = export(request, validator_store, task_executor, log)?; + let export_response = export(request, validator_store, task_executor, log.clone())?; + + // Check the status is Deleted to confirm deletion is successful, then only display the log + let successful_deletion = export_response + .data + .iter() + .filter(|response| matches!(response.status.status, DeleteKeystoreStatus::Deleted)) + .count(); + + if successful_deletion > 0 { + info!( + log, + "Deleted keystore via standard HTTP API"; + "count" => successful_deletion, + ); + } + Ok(DeleteKeystoresResponse { data: export_response .data diff --git a/validator_manager/Cargo.toml b/validator_manager/Cargo.toml index ebcde6a8288..92267ad8755 100644 --- a/validator_manager/Cargo.toml +++ b/validator_manager/Cargo.toml @@ -20,6 +20,7 @@ tree_hash = { workspace = true } eth2 = { workspace = true } hex = { workspace = true } tokio = { workspace = true } +derivative = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/validator_manager/src/create_validators.rs b/validator_manager/src/create_validators.rs index d06fce1d094..62b53f72ee1 100644 --- a/validator_manager/src/create_validators.rs +++ b/validator_manager/src/create_validators.rs @@ -45,15 +45,6 @@ pub fn cli_app() -> Command { Another, optional JSON file is created which contains a list of validator \ deposits in the same format as the \"ethereum/staking-deposit-cli\" tool.", ) - .arg( - Arg::new("help") - .long("help") - .short('h') - .help("Prints help information") - .action(ArgAction::HelpLong) - .display_order(0) - .help_heading(FLAG_HEADER), - ) .arg( Arg::new(OUTPUT_PATH_FLAG) .long(OUTPUT_PATH_FLAG) diff --git a/validator_manager/src/delete_validator.rs b/validator_manager/src/delete_validator.rs new file mode 100644 index 00000000000..644f9b98405 --- /dev/null +++ b/validator_manager/src/delete_validator.rs @@ -0,0 +1,296 @@ +use std::path::PathBuf; + +use clap::{Arg, ArgAction, ArgMatches, Command}; +use eth2::{ + lighthouse_vc::types::{DeleteKeystoreStatus, DeleteKeystoresRequest}, + SensitiveUrl, +}; +use serde::{Deserialize, Serialize}; +use types::PublicKeyBytes; + +use crate::{common::vc_http_client, DumpConfig}; + +pub const CMD: &str = "delete"; +pub const VC_URL_FLAG: &str = "vc-url"; +pub const VC_TOKEN_FLAG: &str = "vc-token"; +pub const VALIDATOR_FLAG: &str = "validators"; + +#[derive(Debug)] +pub enum DeleteError { + InvalidPublicKey, + DeleteFailed(eth2::Error), +} + +pub fn cli_app() -> Command { + Command::new(CMD) + .about("Deletes one or more validators from a validator client using the HTTP API.") + .arg( + Arg::new(VC_URL_FLAG) + .long(VC_URL_FLAG) + .value_name("HTTP_ADDRESS") + .help( + "A HTTP(S) address of a validator client using the keymanager-API. \ + If this value is not supplied then a 'dry run' will be conducted where \ + no changes are made to the validator client.", + ) + .default_value("http://localhost:5062") + .requires(VC_TOKEN_FLAG) + .action(ArgAction::Set) + .display_order(0), + ) + .arg( + Arg::new(VC_TOKEN_FLAG) + .long(VC_TOKEN_FLAG) + .value_name("PATH") + .help("The file containing a token required by the validator client.") + .action(ArgAction::Set) + .display_order(0), + ) + .arg( + Arg::new(VALIDATOR_FLAG) + .long(VALIDATOR_FLAG) + .value_name("STRING") + .help("Comma-separated list of validators (pubkey) that will be deleted.") + .action(ArgAction::Set) + .display_order(0), + ) +} + +#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +pub struct DeleteConfig { + pub vc_url: SensitiveUrl, + pub vc_token_path: PathBuf, + pub validators_to_delete: Vec, +} + +impl DeleteConfig { + fn from_cli(matches: &ArgMatches) -> Result { + let validators_to_delete_str = + clap_utils::parse_required::(matches, VALIDATOR_FLAG)?; + + let validators_to_delete = validators_to_delete_str + .split(',') + .map(|s| s.trim().parse()) + .collect::, _>>()?; + + Ok(Self { + vc_token_path: clap_utils::parse_required(matches, VC_TOKEN_FLAG)?, + validators_to_delete, + vc_url: clap_utils::parse_required(matches, VC_URL_FLAG)?, + }) + } +} + +pub async fn cli_run(matches: &ArgMatches, dump_config: DumpConfig) -> Result<(), String> { + let config = DeleteConfig::from_cli(matches)?; + if dump_config.should_exit_early(&config)? { + Ok(()) + } else { + run(config).await + } +} + +pub async fn run<'a>(config: DeleteConfig) -> Result<(), String> { + let DeleteConfig { + vc_url, + vc_token_path, + validators_to_delete, + } = config; + + let (http_client, validators) = vc_http_client(vc_url.clone(), &vc_token_path).await?; + + for validator_to_delete in &validators_to_delete { + if !validators + .iter() + .any(|validator| &validator.validating_pubkey == validator_to_delete) + { + return Err(format!("Validator {} doesn't exists", validator_to_delete)); + } + } + + let delete_request = DeleteKeystoresRequest { + pubkeys: validators_to_delete.clone(), + }; + + let responses = http_client + .delete_keystores(&delete_request) + .await + .map_err(|e| format!("Error deleting keystore {}", e))? + .data; + + let mut error = false; + for (validator_to_delete, response) in validators_to_delete.iter().zip(responses.iter()) { + if response.status == DeleteKeystoreStatus::Error + || response.status == DeleteKeystoreStatus::NotFound + || response.status == DeleteKeystoreStatus::NotActive + { + error = true; + eprintln!( + "Problem with removing validator {:?}, status: {:?}", + validator_to_delete, response.status + ); + } + } + if error { + return Err("Problem with removing one or more validators".to_string()); + } + + eprintln!("Validator(s) deleted"); + Ok(()) +} + +#[cfg(not(debug_assertions))] +#[cfg(test)] +mod test { + use std::{ + fs::{self, File}, + io::Write, + str::FromStr, + }; + + use super::*; + use crate::{ + common::ValidatorSpecification, import_validators::tests::TestBuilder as ImportTestBuilder, + }; + use validator_client::http_api::{test_utils::ApiTester, Config as HttpConfig}; + + struct TestBuilder { + delete_config: Option, + src_import_builder: Option, + http_config: HttpConfig, + vc_token: Option, + validators: Vec, + } + + impl TestBuilder { + async fn new() -> Self { + Self { + delete_config: None, + src_import_builder: None, + http_config: ApiTester::default_http_config(), + vc_token: None, + validators: vec![], + } + } + + async fn with_validators( + mut self, + count: u32, + first_index: u32, + indices_of_validators_to_delete: Vec, + ) -> Self { + let builder = ImportTestBuilder::new_with_http_config(self.http_config.clone()) + .await + .create_validators(count, first_index) + .await; + + self.vc_token = + Some(fs::read_to_string(builder.get_import_config().vc_token_path).unwrap()); + + let local_validators: Vec = { + let contents = + fs::read_to_string(builder.get_import_config().validators_file_path).unwrap(); + serde_json::from_str(&contents).unwrap() + }; + + let import_config = builder.get_import_config(); + + let validators_to_delete = indices_of_validators_to_delete + .iter() + .map(|&index| { + PublicKeyBytes::from_str( + format!("0x{}", local_validators[index].voting_keystore.pubkey()).as_str(), + ) + .unwrap() + }) + .collect(); + + self.delete_config = Some(DeleteConfig { + vc_url: import_config.vc_url, + vc_token_path: import_config.vc_token_path, + validators_to_delete, + }); + + self.validators = local_validators.clone(); + self.src_import_builder = Some(builder); + self + } + + pub async fn run_test(self) -> TestResult { + let import_builder = self.src_import_builder.unwrap(); + let import_test_result = import_builder.run_test().await; + assert!(import_test_result.result.is_ok()); + + let path = self.delete_config.clone().unwrap().vc_token_path; + let url = self.delete_config.clone().unwrap().vc_url; + let parent = path.parent().unwrap(); + + fs::create_dir_all(parent).expect("Was not able to create parent directory"); + + File::options() + .write(true) + .read(true) + .create(true) + .truncate(true) + .open(path.clone()) + .unwrap() + .write_all(self.vc_token.clone().unwrap().as_bytes()) + .unwrap(); + + let result = run(self.delete_config.clone().unwrap()).await; + + if result.is_ok() { + let (_, list_keystores_response) = vc_http_client(url, path.clone()).await.unwrap(); + + // The remaining number of active keystores (left) = Total validators - Deleted validators (right) + assert_eq!( + list_keystores_response.len(), + self.validators.len() + - self + .delete_config + .clone() + .unwrap() + .validators_to_delete + .len() + ); + + // Check the remaining validator keys are not in validators_to_delete + assert!(list_keystores_response.iter().all(|keystore| { + !self + .delete_config + .clone() + .unwrap() + .validators_to_delete + .contains(&keystore.validating_pubkey) + })); + + return TestResult { result: Ok(()) }; + } + + TestResult { + result: Err(result.unwrap_err()), + } + } + } + + #[must_use] + struct TestResult { + result: Result<(), String>, + } + + impl TestResult { + fn assert_ok(self) { + assert_eq!(self.result, Ok(())) + } + } + #[tokio::test] + async fn delete_multiple_validators() { + TestBuilder::new() + .await + .with_validators(3, 0, vec![0, 1, 2]) + .await + .run_test() + .await + .assert_ok(); + } +} diff --git a/validator_manager/src/import_validators.rs b/validator_manager/src/import_validators.rs index f193e8d0fbd..aa24032a69d 100644 --- a/validator_manager/src/import_validators.rs +++ b/validator_manager/src/import_validators.rs @@ -1,16 +1,28 @@ use super::common::*; use crate::DumpConfig; +use account_utils::{eth2_keystore::Keystore, ZeroizeString}; use clap::{Arg, ArgAction, ArgMatches, Command}; use clap_utils::FLAG_HEADER; +use derivative::Derivative; +use eth2::lighthouse_vc::types::KeystoreJsonStr; use eth2::{lighthouse_vc::std_types::ImportKeystoreStatus, SensitiveUrl}; use serde::{Deserialize, Serialize}; use std::fs; use std::path::PathBuf; +use types::Address; pub const CMD: &str = "import"; pub const VALIDATORS_FILE_FLAG: &str = "validators-file"; pub const VC_URL_FLAG: &str = "vc-url"; pub const VC_TOKEN_FLAG: &str = "vc-token"; +pub const PASSWORD: &str = "password"; +pub const FEE_RECIPIENT: &str = "suggested-fee-recipient"; +pub const GAS_LIMIT: &str = "gas-limit"; +pub const BUILDER_PROPOSALS: &str = "builder-proposals"; +pub const BUILDER_BOOST_FACTOR: &str = "builder-boost-factor"; +pub const PREFER_BUILDER_PROPOSALS: &str = "prefer-builder-proposals"; +pub const ENABLED: &str = "enabled"; +pub const STANDARD_FORMAT: &str = "standard-format"; pub const DETECTED_DUPLICATE_MESSAGE: &str = "Duplicate validator detected!"; @@ -21,15 +33,6 @@ pub fn cli_app() -> Command { are defined in a JSON file which can be generated using the \"create-validators\" \ command.", ) - .arg( - Arg::new("help") - .long("help") - .short('h') - .help("Prints help information") - .action(ArgAction::HelpLong) - .display_order(0) - .help_heading(FLAG_HEADER), - ) .arg( Arg::new(VALIDATORS_FILE_FLAG) .long(VALIDATORS_FILE_FLAG) @@ -80,14 +83,94 @@ pub fn cli_app() -> Command { ) .display_order(0), ) + .arg( + Arg::new(PASSWORD) + .long(PASSWORD) + .value_name("STRING") + .help("Password of the keystore file.") + .action(ArgAction::Set) + .display_order(0), + ) + .arg( + Arg::new(FEE_RECIPIENT) + .long(FEE_RECIPIENT) + .value_name("ETH1_ADDRESS") + .help("When provided, the imported validator will use the suggested fee recipient. Omit this flag to use the default value from the VC.") + .action(ArgAction::Set) + .display_order(0) + .requires(STANDARD_FORMAT), + ) + .arg( + Arg::new(GAS_LIMIT) + .long(GAS_LIMIT) + .value_name("UINT64") + .help("When provided, the imported validator will use this gas limit. It is recommended \ + to leave this as the default value by not specifying this flag.",) + .action(ArgAction::Set) + .display_order(0) + .requires(STANDARD_FORMAT), + ) + .arg( + Arg::new(BUILDER_PROPOSALS) + .long(BUILDER_PROPOSALS) + .help("When provided, the imported validator will attempt to create \ + blocks via builder rather than the local EL.",) + .action(ArgAction::SetTrue) + .help_heading(FLAG_HEADER) + .display_order(0) + .requires(STANDARD_FORMAT), + ) + .arg( + Arg::new(BUILDER_BOOST_FACTOR) + .long(BUILDER_BOOST_FACTOR) + .value_name("UINT64") + .help("When provided, the imported validator will use this \ + percentage multiplier to apply to the builder's payload value \ + when choosing between a builder payload header and payload from \ + the local execution node.",) + .action(ArgAction::Set) + .display_order(0) + .requires(STANDARD_FORMAT), + ) + .arg( + Arg::new(PREFER_BUILDER_PROPOSALS) + .long(PREFER_BUILDER_PROPOSALS) + .help("When provided, the imported validator will always prefer blocks \ + constructed by builders, regardless of payload value.",) + .action(ArgAction::SetTrue) + .display_order(0) + .requires(STANDARD_FORMAT), + ) + .arg( + Arg::new(STANDARD_FORMAT) + .long(STANDARD_FORMAT) + .action(ArgAction::SetTrue) + .help_heading(FLAG_HEADER) + .help( + "Use this flag when the validator keystore files are generated using staking-deposit-cli \ + or ethstaker-deposit-cli." + ) + .display_order(0) + .requires(PASSWORD), + ) } -#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +#[derive(Clone, PartialEq, Serialize, Deserialize, Derivative)] +#[derivative(Debug)] pub struct ImportConfig { pub validators_file_path: PathBuf, pub vc_url: SensitiveUrl, pub vc_token_path: PathBuf, pub ignore_duplicates: bool, + #[derivative(Debug = "ignore")] + pub password: Option, + pub fee_recipient: Option
, + pub gas_limit: Option, + pub builder_proposals: Option, + pub builder_boost_factor: Option, + pub prefer_builder_proposals: Option, + pub enabled: Option, + pub standard_format: bool, } impl ImportConfig { @@ -97,12 +180,21 @@ impl ImportConfig { vc_url: clap_utils::parse_required(matches, VC_URL_FLAG)?, vc_token_path: clap_utils::parse_required(matches, VC_TOKEN_FLAG)?, ignore_duplicates: matches.get_flag(IGNORE_DUPLICATES_FLAG), + password: clap_utils::parse_optional(matches, PASSWORD)?, + fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT)?, + gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT)?, + builder_proposals: Some(matches.get_flag(BUILDER_PROPOSALS)), + builder_boost_factor: clap_utils::parse_optional(matches, BUILDER_BOOST_FACTOR)?, + prefer_builder_proposals: Some(matches.get_flag(PREFER_BUILDER_PROPOSALS)), + enabled: clap_utils::parse_optional(matches, ENABLED)?, + standard_format: matches.get_flag(STANDARD_FORMAT), }) } } pub async fn cli_run(matches: &ArgMatches, dump_config: DumpConfig) -> Result<(), String> { let config = ImportConfig::from_cli(matches)?; + if dump_config.should_exit_early(&config)? { Ok(()) } else { @@ -116,6 +208,14 @@ async fn run<'a>(config: ImportConfig) -> Result<(), String> { vc_url, vc_token_path, ignore_duplicates, + password, + fee_recipient, + gas_limit, + builder_proposals, + builder_boost_factor, + prefer_builder_proposals, + enabled, + standard_format, } = config; if !validators_file_path.exists() { @@ -127,13 +227,31 @@ async fn run<'a>(config: ImportConfig) -> Result<(), String> { .create(false) .open(&validators_file_path) .map_err(|e| format!("Unable to open {:?}: {:?}", validators_file_path, e))?; - let validators: Vec = serde_json::from_reader(&validators_file) - .map_err(|e| { + + let validators: Vec = if standard_format { + vec![ValidatorSpecification { + voting_keystore: KeystoreJsonStr( + Keystore::from_json_file(&validators_file_path).map_err(|e| format!("{e:?}"))?, + ), + voting_keystore_password: password.ok_or_else(|| { + "The --password flag is required to supply the keystore password".to_string() + })?, + slashing_protection: None, + fee_recipient, + gas_limit, + builder_proposals, + builder_boost_factor, + prefer_builder_proposals, + enabled, + }] + } else { + serde_json::from_reader(&validators_file).map_err(|e| { format!( "Unable to parse JSON in {:?}: {:?}", validators_file_path, e ) - })?; + })? + }; let count = validators.len(); @@ -250,7 +368,10 @@ async fn run<'a>(config: ImportConfig) -> Result<(), String> { pub mod tests { use super::*; use crate::create_validators::tests::TestBuilder as CreateTestBuilder; - use std::fs; + use std::{ + fs::{self, File}, + str::FromStr, + }; use tempfile::{tempdir, TempDir}; use validator_client::http_api::{test_utils::ApiTester, Config as HttpConfig}; @@ -283,6 +404,14 @@ pub mod tests { vc_url: vc.url.clone(), vc_token_path, ignore_duplicates: false, + password: Some(ZeroizeString::from_str("password").unwrap()), + fee_recipient: None, + builder_boost_factor: None, + gas_limit: None, + builder_proposals: None, + enabled: None, + prefer_builder_proposals: None, + standard_format: false, }, vc, create_dir: None, @@ -295,6 +424,10 @@ pub mod tests { self } + pub fn get_import_config(&self) -> ImportConfig { + self.import_config.clone() + } + pub async fn create_validators(mut self, count: u32, first_index: u32) -> Self { let create_result = CreateTestBuilder::default() .mutate_config(|config| { @@ -307,7 +440,35 @@ pub mod tests { create_result.result.is_ok(), "precondition: validators are created" ); + + let validators_file_path = create_result.validators_file_path(); + + let validators_file = fs::OpenOptions::new() + .read(true) + .create(false) + .open(&validators_file_path) + .map_err(|e| format!("Unable to open {:?}: {:?}", validators_file_path, e)) + .unwrap(); + + let validators: Vec = serde_json::from_reader(&validators_file) + .map_err(|e| { + format!( + "Unable to parse JSON in {:?}: {:?}", + validators_file_path, e + ) + }) + .unwrap(); + + let validator_standard = &validators[0]; + let validator_json = validator_standard.voting_keystore.0.clone(); + + // Let the created file to have a file name so that it will not overwrite the file created before + let keystore_file_path = validators_file_path.with_file_name("keystore.json"); + let keystore_file = File::create(&keystore_file_path).unwrap(); + validator_json.to_json_writer(keystore_file).unwrap(); + self.import_config.validators_file_path = create_result.validators_file_path(); + self.import_config.password = Some(validator_standard.voting_keystore_password.clone()); self.create_dir = Some(create_result.output_dir); self } @@ -355,6 +516,38 @@ pub mod tests { vc: self.vc, } } + + pub async fn run_test_standard(self) -> TestResult { + let result = run(self.import_config.clone()).await; + + if result.is_ok() { + self.vc.ensure_key_cache_consistency().await; + + let local_keystore: Keystore = + Keystore::from_json_file(&self.import_config.validators_file_path).unwrap(); + + let list_keystores_response = self.vc.client.get_keystores().await.unwrap().data; + + assert_eq!( + 1, + list_keystores_response.len(), + "vc should have exactly the number of validators imported" + ); + + let local_pubkey = local_keystore.public_key().unwrap().into(); + let remote_validator = list_keystores_response + .iter() + .find(|validator| validator.validating_pubkey == local_pubkey) + .expect("validator must exist on VC"); + assert_eq!(&remote_validator.derivation_path, &local_keystore.path()); + assert_eq!(remote_validator.readonly, Some(false)); + } + + TestResult { + result, + vc: self.vc, + } + } } #[must_use] // Use the `assert_ok` or `assert_err` fns to "use" this value. @@ -445,4 +638,55 @@ pub mod tests { .await .assert_ok(); } + + #[tokio::test] + async fn create_one_validator_standard() { + TestBuilder::new() + .await + .create_validators(1, 0) + .await + .run_test_standard() + .await + .assert_ok(); + } + + #[tokio::test] + async fn create_one_validator_with_offset_standard() { + TestBuilder::new() + .await + .create_validators(1, 42) + .await + .run_test_standard() + .await + .assert_ok(); + } + + #[tokio::test] + async fn import_duplicates_when_disallowed_standard() { + TestBuilder::new() + .await + .create_validators(1, 0) + .await + .import_validators_without_checks() + .await + .run_test_standard() + .await + .assert_err_contains("DuplicateValidator"); + } + + #[tokio::test] + async fn import_duplicates_when_allowed_standard() { + TestBuilder::new() + .await + .mutate_import_config(|config| { + config.ignore_duplicates = true; + }) + .create_validators(1, 0) + .await + .import_validators_without_checks() + .await + .run_test_standard() + .await + .assert_ok(); + } } diff --git a/validator_manager/src/lib.rs b/validator_manager/src/lib.rs index 222dd7076de..c8e71631d72 100644 --- a/validator_manager/src/lib.rs +++ b/validator_manager/src/lib.rs @@ -8,7 +8,9 @@ use types::EthSpec; pub mod common; pub mod create_validators; +pub mod delete_validator; pub mod import_validators; +pub mod list_validators; pub mod move_validators; pub const CMD: &str = "validator_manager"; @@ -51,11 +53,14 @@ pub fn cli_app() -> Command { .help("Prints help information") .action(ArgAction::HelpLong) .display_order(0) - .help_heading(FLAG_HEADER), + .help_heading(FLAG_HEADER) + .global(true), ) .subcommand(create_validators::cli_app()) .subcommand(import_validators::cli_app()) .subcommand(move_validators::cli_app()) + .subcommand(list_validators::cli_app()) + .subcommand(delete_validator::cli_app()) } /// Run the account manager, returning an error if the operation did not succeed. @@ -83,6 +88,13 @@ pub fn run(matches: &ArgMatches, env: Environment) -> Result<(), Some((move_validators::CMD, matches)) => { move_validators::cli_run(matches, dump_config).await } + Some((list_validators::CMD, matches)) => { + list_validators::cli_run(matches, dump_config).await + } + Some((delete_validator::CMD, matches)) => { + delete_validator::cli_run(matches, dump_config).await + } + Some(("", _)) => Err("No command supplied. See --help.".to_string()), Some((unknown, _)) => Err(format!( "{} is not a valid {} command. See --help.", unknown, CMD diff --git a/validator_manager/src/list_validators.rs b/validator_manager/src/list_validators.rs new file mode 100644 index 00000000000..778bfa4e8b3 --- /dev/null +++ b/validator_manager/src/list_validators.rs @@ -0,0 +1,204 @@ +use std::path::PathBuf; + +use clap::{Arg, ArgAction, ArgMatches, Command}; +use eth2::SensitiveUrl; +use serde::{Deserialize, Serialize}; + +use crate::{common::vc_http_client, DumpConfig}; + +pub const CMD: &str = "list"; +pub const VC_URL_FLAG: &str = "vc-url"; +pub const VC_TOKEN_FLAG: &str = "vc-token"; + +pub fn cli_app() -> Command { + Command::new(CMD) + .about("Lists all validators in a validator client using the HTTP API.") + .arg( + Arg::new(VC_URL_FLAG) + .long(VC_URL_FLAG) + .value_name("HTTP_ADDRESS") + .help( + "A HTTP(S) address of a validator client using the keymanager-API. \ + If this value is not supplied then a 'dry run' will be conducted where \ + no changes are made to the validator client.", + ) + .default_value("http://localhost:5062") + .requires(VC_TOKEN_FLAG) + .action(ArgAction::Set) + .display_order(0), + ) + .arg( + Arg::new(VC_TOKEN_FLAG) + .long(VC_TOKEN_FLAG) + .value_name("PATH") + .help("The file containing a token required by the validator client.") + .action(ArgAction::Set) + .display_order(0), + ) +} + +#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +pub struct ListConfig { + pub vc_url: SensitiveUrl, + pub vc_token_path: PathBuf, +} + +impl ListConfig { + fn from_cli(matches: &ArgMatches) -> Result { + Ok(Self { + vc_token_path: clap_utils::parse_required(matches, VC_TOKEN_FLAG)?, + vc_url: clap_utils::parse_required(matches, VC_URL_FLAG)?, + }) + } +} + +pub async fn cli_run(matches: &ArgMatches, dump_config: DumpConfig) -> Result<(), String> { + let config = ListConfig::from_cli(matches)?; + if dump_config.should_exit_early(&config)? { + Ok(()) + } else { + println!("{}", run(config).await?); + Ok(()) + } +} + +pub async fn run<'a>(config: ListConfig) -> Result { + let ListConfig { + vc_url, + vc_token_path, + } = config; + + let (_, validators) = vc_http_client(vc_url.clone(), &vc_token_path).await?; + + let mut result = String::new(); + + result.push_str(format!("List of validators ({}):", validators.len()).as_str()); + result.push('\n'); + for validator in validators { + result.push_str(format!("{}", validator.validating_pubkey).as_str()); + result.push('\n'); + } + + Ok(result) +} + +#[cfg(not(debug_assertions))] +#[cfg(test)] +mod test { + use std::{ + fs::{self, File}, + io::Write, + }; + + use super::*; + use crate::{ + common::ValidatorSpecification, import_validators::tests::TestBuilder as ImportTestBuilder, + }; + use validator_client::http_api::{test_utils::ApiTester, Config as HttpConfig}; + + struct TestBuilder { + list_config: Option, + src_import_builder: Option, + http_config: HttpConfig, + vc_token: Option, + validators: Vec, + } + + impl TestBuilder { + async fn new() -> Self { + Self { + list_config: None, + src_import_builder: None, + http_config: ApiTester::default_http_config(), + vc_token: None, + validators: vec![], + } + } + + async fn with_validators(mut self, count: u32, first_index: u32) -> Self { + let builder = ImportTestBuilder::new_with_http_config(self.http_config.clone()) + .await + .create_validators(count, first_index) + .await; + self.list_config = Some(ListConfig { + vc_url: builder.get_import_config().vc_url, + vc_token_path: builder.get_import_config().vc_token_path, + }); + + self.vc_token = + Some(fs::read_to_string(builder.get_import_config().vc_token_path).unwrap()); + + let local_validators: Vec = { + let contents = + fs::read_to_string(builder.get_import_config().validators_file_path).unwrap(); + serde_json::from_str(&contents).unwrap() + }; + + self.validators = local_validators.clone(); + self.src_import_builder = Some(builder); + self + } + + pub async fn run_test(self) -> TestResult { + let import_test_result = self.src_import_builder.unwrap().run_test().await; + assert!(import_test_result.result.is_ok()); + + let path = self.list_config.clone().unwrap().vc_token_path; + let parent = path.parent().unwrap(); + + fs::create_dir_all(parent).expect("Was not able to create parent directory"); + + File::options() + .write(true) + .read(true) + .create(true) + .truncate(true) + .open(path) + .unwrap() + .write_all(self.vc_token.clone().unwrap().as_bytes()) + .unwrap(); + + let result = run(self.list_config.clone().unwrap()).await; + + if result.is_ok() { + let result_ref = result.as_ref().unwrap(); + + for local_validator in &self.validators { + let local_keystore = &local_validator.voting_keystore.0; + let local_pubkey = local_keystore.public_key().unwrap().as_hex_string(); + assert!( + result_ref.contains(&local_pubkey), + "local validator pubkey not found in result" + ); + } + + return TestResult { result: Ok(()) }; + } + + TestResult { + result: Err(result.unwrap_err()), + } + } + } + + #[must_use] // Use the `assert_ok` or `assert_err` fns to "use" this value. + struct TestResult { + result: Result<(), String>, + } + + impl TestResult { + fn assert_ok(self) { + assert_eq!(self.result, Ok(())) + } + } + #[tokio::test] + async fn list_all_validators() { + TestBuilder::new() + .await + .with_validators(3, 0) + .await + .run_test() + .await + .assert_ok(); + } +} diff --git a/validator_manager/src/move_validators.rs b/validator_manager/src/move_validators.rs index 91bc2b0ef85..7651917ea94 100644 --- a/validator_manager/src/move_validators.rs +++ b/validator_manager/src/move_validators.rs @@ -2,7 +2,6 @@ use super::common::*; use crate::DumpConfig; use account_utils::{read_password_from_user, ZeroizeString}; use clap::{Arg, ArgAction, ArgMatches, Command}; -use clap_utils::FLAG_HEADER; use eth2::{ lighthouse_vc::{ std_types::{ @@ -75,15 +74,6 @@ pub fn cli_app() -> Command { command. This command only supports validators signing via a keystore on the local \ file system (i.e., not Web3Signer validators).", ) - .arg( - Arg::new("help") - .long("help") - .short('h') - .help("Prints help information") - .action(ArgAction::HelpLong) - .display_order(0) - .help_heading(FLAG_HEADER), - ) .arg( Arg::new(SRC_VC_URL_FLAG) .long(SRC_VC_URL_FLAG)