From 2877c29ca3af03c9022912a3bb2f6814e74633c1 Mon Sep 17 00:00:00 2001 From: tim gretler Date: Mon, 9 May 2022 07:21:38 +0000 Subject: [PATCH] Add remotekey API support (#3162) ## Issue Addressed #3068 ## Proposed Changes Adds support for remote key API. ## Additional Info Needed to add `is_local_keystore` argument to `delete_definition_and_keystore` to know if we want to delete local or remote key. Previously this wasn't necessary because remotekeys(web3signers) could be deleted. --- common/eth2/src/lighthouse_vc/http_client.rs | 34 + common/eth2/src/lighthouse_vc/std_types.rs | 56 ++ .../src/http_api/create_validator.rs | 20 +- validator_client/src/http_api/keystores.rs | 16 +- validator_client/src/http_api/mod.rs | 77 +- validator_client/src/http_api/remotekeys.rs | 211 +++++ .../src/http_api/tests/keystores.rs | 873 +++++++++++++++++- .../src/initialized_validators.rs | 19 +- 8 files changed, 1236 insertions(+), 70 deletions(-) create mode 100644 validator_client/src/http_api/remotekeys.rs diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index e7c74668e..5e02ec0bb 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -476,6 +476,16 @@ impl ValidatorClientHttpClient { Ok(url) } + fn make_remotekeys_url(&self) -> Result { + let mut url = self.server.full.clone(); + url.path_segments_mut() + .map_err(|()| Error::InvalidUrl(self.server.clone()))? + .push("eth") + .push("v1") + .push("remotekeys"); + Ok(url) + } + /// `GET lighthouse/auth` pub async fn get_auth(&self) -> Result { let mut url = self.server.full.clone(); @@ -509,6 +519,30 @@ impl ValidatorClientHttpClient { let url = self.make_keystores_url()?; self.delete_with_unsigned_response(url, req).await } + + /// `GET eth/v1/remotekeys` + pub async fn get_remotekeys(&self) -> Result { + let url = self.make_remotekeys_url()?; + self.get_unsigned(url).await + } + + /// `POST eth/v1/remotekeys` + pub async fn post_remotekeys( + &self, + req: &ImportRemotekeysRequest, + ) -> Result { + let url = self.make_remotekeys_url()?; + self.post_with_unsigned_response(url, req).await + } + + /// `DELETE eth/v1/remotekeys` + pub async fn delete_remotekeys( + &self, + req: &DeleteRemotekeysRequest, + ) -> Result { + let url = self.make_remotekeys_url()?; + self.delete_with_unsigned_response(url, req).await + } } /// Returns `Ok(response)` if the response is a `200 OK` response. Otherwise, creates an diff --git a/common/eth2/src/lighthouse_vc/std_types.rs b/common/eth2/src/lighthouse_vc/std_types.rs index ebcce3fab..d9fe96913 100644 --- a/common/eth2/src/lighthouse_vc/std_types.rs +++ b/common/eth2/src/lighthouse_vc/std_types.rs @@ -102,3 +102,59 @@ pub enum DeleteKeystoreStatus { NotFound, Error, } + +#[derive(Debug, Deserialize, Serialize, PartialEq)] +pub struct ListRemotekeysResponse { + pub data: Vec, +} + +#[derive(Debug, Deserialize, Serialize, PartialEq)] +pub struct SingleListRemotekeysResponse { + pub pubkey: PublicKeyBytes, + pub url: String, + pub readonly: bool, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct ImportRemotekeysRequest { + pub remote_keys: Vec, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +pub struct SingleImportRemotekeysRequest { + pub pubkey: PublicKeyBytes, + pub url: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum ImportRemotekeyStatus { + Imported, + Duplicate, + Error, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct ImportRemotekeysResponse { + pub data: Vec>, +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct DeleteRemotekeysRequest { + pub pubkeys: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DeleteRemotekeyStatus { + Deleted, + NotFound, + Error, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct DeleteRemotekeysResponse { + pub data: Vec>, +} diff --git a/validator_client/src/http_api/create_validator.rs b/validator_client/src/http_api/create_validator.rs index a8e4fd262..db59c25f7 100644 --- a/validator_client/src/http_api/create_validator.rs +++ b/validator_client/src/http_api/create_validator.rs @@ -1,5 +1,5 @@ use crate::ValidatorStore; -use account_utils::validator_definitions::{SigningDefinition, ValidatorDefinition}; +use account_utils::validator_definitions::ValidatorDefinition; use account_utils::{ eth2_wallet::{bip39::Mnemonic, WalletBuilder}, random_mnemonic, random_password, ZeroizeString, @@ -164,24 +164,12 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, } pub async fn create_validators_web3signer( - validator_requests: &[api_types::Web3SignerValidatorRequest], + validators: Vec, validator_store: &ValidatorStore, ) -> Result<(), warp::Rejection> { - for request in validator_requests { - let validator_definition = ValidatorDefinition { - enabled: request.enable, - voting_public_key: request.voting_public_key.clone(), - graffiti: request.graffiti.clone(), - suggested_fee_recipient: request.suggested_fee_recipient, - description: request.description.clone(), - signing_definition: SigningDefinition::Web3Signer { - url: request.url.clone(), - root_certificate_path: request.root_certificate_path.clone(), - request_timeout_ms: request.request_timeout_ms, - }, - }; + for validator in validators { validator_store - .add_validator(validator_definition) + .add_validator(validator) .await .map_err(|e| { warp_utils::reject::custom_server_error(format!( diff --git a/validator_client/src/http_api/keystores.rs b/validator_client/src/http_api/keystores.rs index ce6089c5b..63cd94606 100644 --- a/validator_client/src/http_api/keystores.rs +++ b/validator_client/src/http_api/keystores.rs @@ -1,5 +1,8 @@ //! Implementation of the standard keystore management API. -use crate::{signing_method::SigningMethod, InitializedValidators, ValidatorStore}; +use crate::{ + initialized_validators::Error, signing_method::SigningMethod, InitializedValidators, + ValidatorStore, +}; use account_utils::ZeroizeString; use eth2::lighthouse_vc::std_types::{ DeleteKeystoreStatus, DeleteKeystoresRequest, DeleteKeystoresResponse, ImportKeystoreStatus, @@ -282,9 +285,14 @@ fn delete_single_keystore( .decompress() .map_err(|e| format!("invalid pubkey, {:?}: {:?}", pubkey_bytes, e))?; - runtime - .block_on(initialized_validators.delete_definition_and_keystore(&pubkey)) - .map_err(|e| format!("unable to disable and delete: {:?}", e)) + match runtime.block_on(initialized_validators.delete_definition_and_keystore(&pubkey, true)) + { + Ok(_) => Ok(DeleteKeystoreStatus::Deleted), + Err(e) => match e { + Error::ValidatorNotInitialized(_) => Ok(DeleteKeystoreStatus::NotFound), + _ => Err(format!("unable to disable and delete: {:?}", e)), + }, + } } else { Err("validator client shutdown".into()) } diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index 8e1f5a739..1207ed3b0 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -1,10 +1,14 @@ mod api_secret; mod create_validator; mod keystores; +mod remotekeys; mod tests; use crate::ValidatorStore; -use account_utils::mnemonic_from_phrase; +use account_utils::{ + mnemonic_from_phrase, + validator_definitions::{SigningDefinition, ValidatorDefinition}, +}; use create_validator::{create_validators_mnemonic, create_validators_web3signer}; use eth2::lighthouse_vc::{ std_types::AuthResponse, @@ -459,7 +463,25 @@ pub fn serve( runtime: Weak| { blocking_signed_json_task(signer, move || { if let Some(runtime) = runtime.upgrade() { - runtime.block_on(create_validators_web3signer(&body, &validator_store))?; + let web3signers: Vec = body + .into_iter() + .map(|web3signer| ValidatorDefinition { + enabled: web3signer.enable, + voting_public_key: web3signer.voting_public_key, + graffiti: web3signer.graffiti, + suggested_fee_recipient: web3signer.suggested_fee_recipient, + description: web3signer.description, + signing_definition: SigningDefinition::Web3Signer { + url: web3signer.url, + root_certificate_path: web3signer.root_certificate_path, + request_timeout_ms: web3signer.request_timeout_ms, + }, + }) + .collect(); + runtime.block_on(create_validators_web3signer( + web3signers, + &validator_store, + ))?; Ok(()) } else { Err(warp_utils::reject::custom_server_error( @@ -536,6 +558,7 @@ pub fn serve( // Standard key-manager endpoints. let eth_v1 = warp::path("eth").and(warp::path("v1")); let std_keystores = eth_v1.and(warp::path("keystores")).and(warp::path::end()); + let std_remotekeys = eth_v1.and(warp::path("remotekeys")).and(warp::path::end()); // GET /eth/v1/keystores let get_std_keystores = std_keystores @@ -564,16 +587,50 @@ pub fn serve( // DELETE /eth/v1/keystores let delete_std_keystores = std_keystores .and(warp::body::json()) - .and(signer) - .and(validator_store_filter) - .and(runtime_filter) - .and(log_filter) + .and(signer.clone()) + .and(validator_store_filter.clone()) + .and(runtime_filter.clone()) + .and(log_filter.clone()) .and_then(|request, signer, validator_store, runtime, log| { blocking_signed_json_task(signer, move || { keystores::delete(request, validator_store, runtime, log) }) }); + // GET /eth/v1/remotekeys + let get_std_remotekeys = std_remotekeys + .and(signer.clone()) + .and(validator_store_filter.clone()) + .and_then(|signer, validator_store: Arc>| { + blocking_signed_json_task(signer, move || Ok(remotekeys::list(validator_store))) + }); + + // POST /eth/v1/remotekeys + let post_std_remotekeys = std_remotekeys + .and(warp::body::json()) + .and(signer.clone()) + .and(validator_store_filter.clone()) + .and(runtime_filter.clone()) + .and(log_filter.clone()) + .and_then(|request, signer, validator_store, runtime, log| { + blocking_signed_json_task(signer, move || { + remotekeys::import(request, validator_store, runtime, log) + }) + }); + + // DELETE /eth/v1/remotekeys + let delete_std_remotekeys = std_remotekeys + .and(warp::body::json()) + .and(signer) + .and(validator_store_filter) + .and(runtime_filter) + .and(log_filter.clone()) + .and_then(|request, signer, validator_store, runtime, log| { + blocking_signed_json_task(signer, move || { + remotekeys::delete(request, validator_store, runtime, log) + }) + }); + let routes = warp::any() .and(authorization_header_filter) // Note: it is critical that the `authorization_header_filter` is applied to all routes. @@ -588,17 +645,19 @@ pub fn serve( .or(get_lighthouse_spec) .or(get_lighthouse_validators) .or(get_lighthouse_validators_pubkey) - .or(get_std_keystores), + .or(get_std_keystores) + .or(get_std_remotekeys), ) .or(warp::post().and( post_validators .or(post_validators_keystore) .or(post_validators_mnemonic) .or(post_validators_web3signer) - .or(post_std_keystores), + .or(post_std_keystores) + .or(post_std_remotekeys), )) .or(warp::patch().and(patch_validators)) - .or(warp::delete().and(delete_std_keystores)), + .or(warp::delete().and(delete_std_keystores.or(delete_std_remotekeys))), ) // The auth route is the only route that is allowed to be accessed without the API token. .or(warp::get().and(get_auth)) diff --git a/validator_client/src/http_api/remotekeys.rs b/validator_client/src/http_api/remotekeys.rs new file mode 100644 index 000000000..b3702a028 --- /dev/null +++ b/validator_client/src/http_api/remotekeys.rs @@ -0,0 +1,211 @@ +//! Implementation of the standard remotekey management API. +use crate::{initialized_validators::Error, InitializedValidators, ValidatorStore}; +use account_utils::validator_definitions::{SigningDefinition, ValidatorDefinition}; +use eth2::lighthouse_vc::std_types::{ + DeleteRemotekeyStatus, DeleteRemotekeysRequest, DeleteRemotekeysResponse, + ImportRemotekeyStatus, ImportRemotekeysRequest, ImportRemotekeysResponse, + ListRemotekeysResponse, SingleListRemotekeysResponse, Status, +}; +use slog::{info, warn, Logger}; +use slot_clock::SlotClock; +use std::sync::{Arc, Weak}; +use tokio::runtime::Runtime; +use types::{EthSpec, PublicKeyBytes}; +use url::Url; +use warp::Rejection; +use warp_utils::reject::custom_server_error; + +pub fn list( + validator_store: Arc>, +) -> ListRemotekeysResponse { + let initialized_validators_rwlock = validator_store.initialized_validators(); + let initialized_validators = initialized_validators_rwlock.read(); + + let keystores = initialized_validators + .validator_definitions() + .iter() + .filter(|def| def.enabled) + .filter_map(|def| { + let validating_pubkey = def.voting_public_key.compress(); + + match &def.signing_definition { + SigningDefinition::LocalKeystore { .. } => None, + SigningDefinition::Web3Signer { url, .. } => Some(SingleListRemotekeysResponse { + pubkey: validating_pubkey, + url: url.clone(), + readonly: false, + }), + } + }) + .collect::>(); + + ListRemotekeysResponse { data: keystores } +} + +pub fn import( + request: ImportRemotekeysRequest, + validator_store: Arc>, + runtime: Weak, + log: Logger, +) -> Result { + info!( + log, + "Importing remotekeys via standard HTTP API"; + "count" => request.remote_keys.len(), + ); + // Import each remotekey. Some remotekeys may fail to be imported, so we record a status for each. + let mut statuses = Vec::with_capacity(request.remote_keys.len()); + + for remotekey in request.remote_keys { + let status = if let Some(runtime) = runtime.upgrade() { + // Import the keystore. + match import_single_remotekey( + remotekey.pubkey, + remotekey.url, + &validator_store, + runtime, + ) { + Ok(status) => Status::ok(status), + Err(e) => { + warn!( + log, + "Error importing keystore, skipped"; + "pubkey" => remotekey.pubkey.to_string(), + "error" => ?e, + ); + Status::error(ImportRemotekeyStatus::Error, e) + } + } + } else { + Status::error( + ImportRemotekeyStatus::Error, + "validator client shutdown".into(), + ) + }; + statuses.push(status); + } + Ok(ImportRemotekeysResponse { data: statuses }) +} + +fn import_single_remotekey( + pubkey: PublicKeyBytes, + url: String, + validator_store: &ValidatorStore, + runtime: Arc, +) -> Result { + if let Err(url_err) = Url::parse(&url) { + return Err(format!("failed to parse remotekey URL: {}", url_err)); + } + + let pubkey = pubkey + .decompress() + .map_err(|_| format!("invalid pubkey: {}", pubkey))?; + + if let Some(def) = validator_store + .initialized_validators() + .read() + .validator_definitions() + .iter() + .find(|def| def.voting_public_key == pubkey) + { + if def.signing_definition.is_local_keystore() { + return Err("Pubkey already present in local keystore.".into()); + } else if def.enabled { + return Ok(ImportRemotekeyStatus::Duplicate); + } + } + + // Remotekeys are stored as web3signers. + // The remotekey API provides less confgiuration option than the web3signer API. + let web3signer_validator = ValidatorDefinition { + enabled: true, + voting_public_key: pubkey, + graffiti: None, + suggested_fee_recipient: None, + description: String::from("Added by remotekey API"), + signing_definition: SigningDefinition::Web3Signer { + url, + root_certificate_path: None, + request_timeout_ms: None, + }, + }; + runtime + .block_on(validator_store.add_validator(web3signer_validator)) + .map_err(|e| format!("failed to initialize validator: {:?}", e))?; + + Ok(ImportRemotekeyStatus::Imported) +} + +pub fn delete( + request: DeleteRemotekeysRequest, + validator_store: Arc>, + runtime: Weak, + log: Logger, +) -> Result { + info!( + log, + "Deleting remotekeys via standard HTTP API"; + "count" => request.pubkeys.len(), + ); + // Remove from initialized validators. + let initialized_validators_rwlock = validator_store.initialized_validators(); + let mut initialized_validators = initialized_validators_rwlock.write(); + + let statuses = request + .pubkeys + .iter() + .map(|pubkey_bytes| { + match delete_single_remotekey( + pubkey_bytes, + &mut initialized_validators, + runtime.clone(), + ) { + Ok(status) => Status::ok(status), + Err(error) => { + warn!( + log, + "Error deleting keystore"; + "pubkey" => ?pubkey_bytes, + "error" => ?error, + ); + Status::error(DeleteRemotekeyStatus::Error, error) + } + } + }) + .collect::>(); + + // Use `update_validators` to update the key cache. It is safe to let the key cache get a bit out + // of date as it resets when it can't be decrypted. We update it just a single time to avoid + // continually resetting it after each key deletion. + if let Some(runtime) = runtime.upgrade() { + runtime + .block_on(initialized_validators.update_validators()) + .map_err(|e| custom_server_error(format!("unable to update key cache: {:?}", e)))?; + } + + Ok(DeleteRemotekeysResponse { data: statuses }) +} + +fn delete_single_remotekey( + pubkey_bytes: &PublicKeyBytes, + initialized_validators: &mut InitializedValidators, + runtime: Weak, +) -> Result { + if let Some(runtime) = runtime.upgrade() { + let pubkey = pubkey_bytes + .decompress() + .map_err(|e| format!("invalid pubkey, {:?}: {:?}", pubkey_bytes, e))?; + + match runtime + .block_on(initialized_validators.delete_definition_and_keystore(&pubkey, false)) + { + Ok(_) => Ok(DeleteRemotekeyStatus::Deleted), + Err(e) => match e { + Error::ValidatorNotInitialized(_) => Ok(DeleteRemotekeyStatus::NotFound), + _ => Err(format!("unable to disable and delete: {:?}", e)), + }, + } + } else { + Err("validator client shutdown".into()) + } +} diff --git a/validator_client/src/http_api/tests/keystores.rs b/validator_client/src/http_api/tests/keystores.rs index 427f22adc..69410456f 100644 --- a/validator_client/src/http_api/tests/keystores.rs +++ b/validator_client/src/http_api/tests/keystores.rs @@ -8,8 +8,7 @@ use eth2::lighthouse_vc::{ use itertools::Itertools; use rand::{rngs::SmallRng, Rng, SeedableRng}; use slashing_protection::interchange::{Interchange, InterchangeMetadata}; -use std::collections::HashMap; -use std::path::Path; +use std::{collections::HashMap, path::Path}; fn new_keystore(password: ZeroizeString) -> Keystore { let keypair = Keypair::random(); @@ -44,6 +43,19 @@ fn web3signer_validator_with_pubkey(pubkey: PublicKey) -> Web3SignerValidatorReq } } +fn new_remotekey_validator() -> (Keypair, SingleImportRemotekeysRequest) { + let keypair = Keypair::random(); + let pk = keypair.pk.clone(); + (keypair, remotekey_validator_with_pubkey(pk)) +} + +fn remotekey_validator_with_pubkey(pubkey: PublicKey) -> SingleImportRemotekeysRequest { + SingleImportRemotekeysRequest { + pubkey: pubkey.compress(), + url: web3_signer_url(), + } +} + fn run_test(f: F) where F: FnOnce(ApiTester) -> V, @@ -107,7 +119,7 @@ fn all_delete_error(count: usize) -> impl Iterator all_with_status(count, DeleteKeystoreStatus::Error) } -fn check_get_response<'a>( +fn check_keystore_get_response<'a>( response: &ListKeystoresResponse, expected_keystores: impl IntoIterator, ) { @@ -118,7 +130,7 @@ fn check_get_response<'a>( } } -fn check_import_response( +fn check_keystore_import_response( response: &ImportKeystoresResponse, expected_statuses: impl IntoIterator, ) { @@ -131,7 +143,7 @@ fn check_import_response( } } -fn check_delete_response<'a>( +fn check_keystore_delete_response<'a>( response: &DeleteKeystoresResponse, expected_statuses: impl IntoIterator, ) { @@ -144,6 +156,41 @@ fn check_delete_response<'a>( } } +fn check_remotekey_get_response( + response: &ListRemotekeysResponse, + expected_keystores: impl IntoIterator, +) { + for expected in expected_keystores { + assert!(response.data.contains(&expected)); + } +} + +fn check_remotekey_import_response( + response: &ImportRemotekeysResponse, + expected_statuses: impl IntoIterator, +) { + for (status, expected_status) in response.data.iter().zip_eq(expected_statuses) { + assert_eq!( + expected_status, status.status, + "message: {:?}", + status.message + ); + } +} + +fn check_remotekey_delete_response( + response: &DeleteRemotekeysResponse, + expected_statuses: impl IntoIterator, +) { + for (status, expected_status) in response.data.iter().zip_eq(expected_statuses) { + assert_eq!( + status.status, expected_status, + "message: {:?}", + status.message + ); + } +} + #[test] fn get_auth_no_token() { run_test(|mut tester| async move { @@ -189,11 +236,11 @@ fn import_new_keystores() { .unwrap(); // All keystores should be imported. - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // Check that GET lists all the imported keystores. let get_res = tester.client.get_keystores().await.unwrap(); - check_get_response(&get_res, &keystores); + check_keystore_get_response(&get_res, &keystores); }) } @@ -214,15 +261,15 @@ fn import_only_duplicate_keystores() { // All keystores should be imported on first import. let import_res = tester.client.post_keystores(&req).await.unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // No keystores should be imported on repeat import. let import_res = tester.client.post_keystores(&req).await.unwrap(); - check_import_response(&import_res, all_duplicate(keystores.len())); + check_keystore_import_response(&import_res, all_duplicate(keystores.len())); // Check that GET lists all the imported keystores. let get_res = tester.client.get_keystores().await.unwrap(); - check_get_response(&get_res, &keystores); + check_keystore_get_response(&get_res, &keystores); }) } @@ -262,7 +309,7 @@ fn import_some_duplicate_keystores() { }; let import_res = tester.client.post_keystores(&req1).await.unwrap(); - check_import_response(&import_res, all_imported(keystores1.len())); + check_keystore_import_response(&import_res, all_imported(keystores1.len())); // Check partial import. let expected = (0..num_keystores).map(|i| { @@ -273,7 +320,7 @@ fn import_some_duplicate_keystores() { } }); let import_res = tester.client.post_keystores(&req2).await.unwrap(); - check_import_response(&import_res, expected); + check_keystore_import_response(&import_res, expected); }) } @@ -323,7 +370,7 @@ fn get_web3_signer_keystores() { .unwrap(); // All keystores should be imported. - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // Add some web3signer validators. let remote_vals = (0..num_remote) @@ -391,14 +438,14 @@ fn import_and_delete_conflicting_web3_signer_keystores() { slashing_protection: None, }; let import_res = tester.client.post_keystores(&import_req).await.unwrap(); - check_import_response(&import_res, all_import_error(keystores.len())); + check_keystore_import_response(&import_res, all_import_error(keystores.len())); // Attempt to delete the web3signer validators, which should fail. let delete_req = DeleteKeystoresRequest { pubkeys: pubkeys.clone(), }; let delete_res = tester.client.delete_keystores(&delete_req).await.unwrap(); - check_delete_response(&delete_res, all_delete_error(keystores.len())); + check_keystore_delete_response(&delete_res, all_delete_error(keystores.len())); // Get should still list all the validators as `readonly`. let get_res = tester.client.get_keystores().await.unwrap(); @@ -418,9 +465,9 @@ fn import_and_delete_conflicting_web3_signer_keystores() { .unwrap(); } let import_res = tester.client.post_keystores(&import_req).await.unwrap(); - check_import_response(&import_res, all_import_error(keystores.len())); + check_keystore_import_response(&import_res, all_import_error(keystores.len())); let delete_res = tester.client.delete_keystores(&delete_req).await.unwrap(); - check_delete_response(&delete_res, all_delete_error(keystores.len())); + check_keystore_delete_response(&delete_res, all_delete_error(keystores.len())); }) } @@ -464,7 +511,7 @@ fn import_keystores_wrong_password() { ImportKeystoreStatus::Imported } }); - check_import_response(&import_res, expected_statuses); + check_keystore_import_response(&import_res, expected_statuses); // Import again with the correct passwords and check that the statuses are as expected. let correct_import_req = ImportKeystoresRequest { @@ -484,7 +531,7 @@ fn import_keystores_wrong_password() { ImportKeystoreStatus::Duplicate } }); - check_import_response(&import_res, expected_statuses); + check_keystore_import_response(&import_res, expected_statuses); // Import one final time, at which point all keys should be duplicates. let import_res = tester @@ -492,7 +539,7 @@ fn import_keystores_wrong_password() { .post_keystores(&correct_import_req) .await .unwrap(); - check_import_response( + check_keystore_import_response( &import_res, (0..num_keystores).map(|_| ImportKeystoreStatus::Duplicate), ); @@ -528,11 +575,11 @@ fn import_invalid_slashing_protection() { .unwrap(); // All keystores should be imported. - check_import_response(&import_res, all_import_error(keystores.len())); + check_keystore_import_response(&import_res, all_import_error(keystores.len())); // Check that GET lists none of the failed keystores. let get_res = tester.client.get_keystores().await.unwrap(); - check_get_response(&get_res, &[]); + check_keystore_get_response(&get_res, &[]); }) } @@ -669,7 +716,7 @@ fn generic_migration_test( }) .await .unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // Sign attestations on VC1. for (validator_index, mut attestation) in first_vc_attestations { @@ -694,7 +741,7 @@ fn generic_migration_test( }) .await .unwrap(); - check_delete_response(&delete_res, all_deleted(delete_indices.len())); + check_keystore_delete_response(&delete_res, all_deleted(delete_indices.len())); // Check that slashing protection data was returned for all selected validators. assert_eq!( @@ -745,7 +792,7 @@ fn generic_migration_test( }) .await .unwrap(); - check_import_response(&import_res, all_imported(import_indices.len())); + check_keystore_import_response(&import_res, all_imported(import_indices.len())); // Sign attestations on the second VC. for (validator_index, mut attestation, should_succeed) in second_vc_attestations { @@ -779,18 +826,18 @@ fn delete_keystores_twice() { slashing_protection: None, }; let import_res = tester.client.post_keystores(&import_req).await.unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // 2. Delete all. let delete_req = DeleteKeystoresRequest { pubkeys: keystores.iter().map(keystore_pubkey).collect(), }; let delete_res = tester.client.delete_keystores(&delete_req).await.unwrap(); - check_delete_response(&delete_res, all_deleted(keystores.len())); + check_keystore_delete_response(&delete_res, all_deleted(keystores.len())); // 3. Delete again. let delete_res = tester.client.delete_keystores(&delete_req).await.unwrap(); - check_delete_response(&delete_res, all_not_active(keystores.len())); + check_keystore_delete_response(&delete_res, all_not_active(keystores.len())); }) } @@ -808,7 +855,7 @@ fn delete_nonexistent_keystores() { pubkeys: keystores.iter().map(keystore_pubkey).collect(), }; let delete_res = tester.client.delete_keystores(&delete_req).await.unwrap(); - check_delete_response(&delete_res, all_not_found(keystores.len())); + check_keystore_delete_response(&delete_res, all_not_found(keystores.len())); }) } @@ -868,7 +915,7 @@ fn delete_concurrent_with_signing() { }) .await .unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // Start several threads signing attestations at sequential epochs. let mut join_handles = vec![]; @@ -972,7 +1019,7 @@ fn delete_then_reimport() { slashing_protection: None, }; let import_res = tester.client.post_keystores(&import_req).await.unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); // 2. Delete all. let delete_res = tester @@ -982,10 +1029,770 @@ fn delete_then_reimport() { }) .await .unwrap(); - check_delete_response(&delete_res, all_deleted(keystores.len())); + check_keystore_delete_response(&delete_res, all_deleted(keystores.len())); // 3. Re-import let import_res = tester.client.post_keystores(&import_req).await.unwrap(); - check_import_response(&import_res, all_imported(keystores.len())); + check_keystore_import_response(&import_res, all_imported(keystores.len())); + }) +} + +#[test] +fn get_empty_remotekeys() { + run_test(|tester| async move { + let _ = &tester; + let res = tester.client.get_remotekeys().await.unwrap(); + assert_eq!(res, ListRemotekeysResponse { data: vec![] }); + }) +} + +#[test] +fn import_new_remotekeys() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekeys. + let remotekeys = (0..3) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + + // All keystores should be imported. + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Check list response. + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .collect::>(); + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_same_remotekey_different_url() { + run_test(|tester| async move { + let _ = &tester; + + // Create two remotekeys with different urls. + let remotekey1 = new_remotekey_validator().1; + let mut remotekey2 = remotekey1.clone(); + remotekey2.url = "http://localhost:1/this-url-hopefully-does-also-not-exist".into(); + let remotekeys = vec![remotekey1, remotekey2]; + + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + + // Both remotekeys have the same public key and therefore only the first one should be imported. + check_remotekey_import_response( + &import_res, + vec![ + ImportRemotekeyStatus::Imported, + ImportRemotekeyStatus::Duplicate, + ] + .into_iter(), + ); + + // Only first key is imported and should be returned. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response( + &get_res, + vec![SingleListRemotekeysResponse { + pubkey: remotekeys[0].pubkey, + url: remotekeys[0].url.clone(), + readonly: false, + }], + ); + }) +} + +#[test] +fn delete_remotekey_then_reimport_different_url() { + run_test(|tester| async move { + let _ = &tester; + + // Create two remotekeys with different urls. + let mut remotekey = new_remotekey_validator().1; + let remotekeys = vec![remotekey.clone()]; + + // Import and Delete remotekey. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + vec![ImportRemotekeyStatus::Imported].into_iter(), + ); + let delete_req = DeleteRemotekeysRequest { + pubkeys: remotekeys.iter().map(|k| k.pubkey).collect(), + }; + let delete_res = tester.client.delete_remotekeys(&delete_req).await.unwrap(); + check_remotekey_delete_response( + &delete_res, + all_with_status(remotekeys.len(), DeleteRemotekeyStatus::Deleted), + ); + + // Change remotekey url. + remotekey.url = "http://localhost:1/this-url-hopefully-does-also-not-exist".into(); + let remotekeys = vec![remotekey.clone()]; + + // Reimport remotekey. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + vec![ImportRemotekeyStatus::Imported].into_iter(), + ); + }) +} + +#[test] +fn import_only_duplicate_remotekeys() { + run_test(|tester| async move { + let _ = &tester; + let remotekeys = (0..3) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // All remotekeys should be imported on first import. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // No remotekeys should be imported on repeat import. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Duplicate), + ); + + // Check list response. + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .collect::>(); + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_some_duplicate_remotekeys() { + run_test(|tester| async move { + let _ = &tester; + let num_remotekeys = 5; + let remotekeys_all = (0..num_remotekeys) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // Select even numbered keystores. + let remotekeys_even = remotekeys_all + .iter() + .enumerate() + .filter_map(|(i, remotekey)| { + if i % 2 == 0 { + Some(remotekey.clone()) + } else { + None + } + }) + .collect::>(); + + // Only import every second remotekey. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys_even.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys_even.len(), ImportRemotekeyStatus::Imported), + ); + + let expected = (0..num_remotekeys).map(|i| { + if i % 2 == 0 { + ImportRemotekeyStatus::Duplicate + } else { + ImportRemotekeyStatus::Imported + } + }); + + // Try to import all keys. Every second import should be a duplicate. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys_all.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response(&import_res, expected); + + // Check list response. + let expected_responses = remotekeys_all + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .collect::>(); + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_remote_and_local_keys() { + run_test(|tester| async move { + let _ = &tester; + let num_local = 3; + let num_remote = 2; + + // Generate local keystores. + let password = random_password_string(); + let keystores = (0..num_local) + .map(|_| new_keystore(password.clone())) + .collect::>(); + + // Import keystores. + let import_res = tester + .client + .post_keystores(&ImportKeystoresRequest { + keystores: keystores.clone(), + passwords: vec![password.clone(); keystores.len()], + slashing_protection: None, + }) + .await + .unwrap(); + + // All keystores should be imported. + check_keystore_import_response( + &import_res, + all_with_status(keystores.len(), ImportKeystoreStatus::Imported), + ); + + // Add some remotekey validators. + let remotekeys = (0..num_remote) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + + // All remotekeys should be imported. + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Check that only remote validators are returned. + let get_res = tester.client.get_keystores().await.unwrap(); + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleKeystoreResponse { + validating_pubkey: remotekey.pubkey, + derivation_path: None, + readonly: Some(true), + }) + .collect::>(); + for response in expected_responses { + assert!(get_res.data.contains(&response), "{:?}", response); + } + }) +} + +#[test] +fn import_same_local_and_remote_keys() { + run_test(|tester| async move { + let _ = &tester; + let num_local = 3; + + // Generate local keystores. + let password = random_password_string(); + let keystores = (0..num_local) + .map(|_| new_keystore(password.clone())) + .collect::>(); + + // Generate remotekeys with same pubkey as local keystores. + let mut remotekeys = Vec::new(); + for keystore in keystores.iter() { + remotekeys.push(remotekey_validator_with_pubkey( + keystore.public_key().unwrap(), + )); + } + + // Import keystores. + let import_res = tester + .client + .post_keystores(&ImportKeystoresRequest { + keystores: keystores.clone(), + passwords: vec![password.clone(); keystores.len()], + slashing_protection: None, + }) + .await + .unwrap(); + + // All keystores should be imported. + check_keystore_import_response( + &import_res, + all_with_status(keystores.len(), ImportKeystoreStatus::Imported), + ); + + // Try to import remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + + // All remotekey import should fail. Already imported as local keystore. + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Error), + ); + + // Check that only local keystores are returned. + let get_res = tester.client.get_keystores().await.unwrap(); + let expected_responses = keystores + .iter() + .map(|local_keystore| SingleKeystoreResponse { + validating_pubkey: keystore_pubkey(local_keystore), + derivation_path: local_keystore.path(), + readonly: None, + }) + .collect::>(); + for response in expected_responses { + assert!(get_res.data.contains(&response), "{:?}", response); + } + }) +} +#[test] +fn import_same_remote_and_local_keys() { + run_test(|tester| async move { + let _ = &tester; + let num_local = 3; + + // Generate local keystores. + let password = random_password_string(); + let keystores = (0..num_local) + .map(|_| new_keystore(password.clone())) + .collect::>(); + + // Generate remotekeys with same pubkey as local keystores. + let mut remotekeys = Vec::new(); + for keystore in keystores.iter() { + remotekeys.push(remotekey_validator_with_pubkey( + keystore.public_key().unwrap(), + )); + } + + // Import remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + + // All remotekeys should be imported. + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Try to import local keystores. + let import_res = tester + .client + .post_keystores(&ImportKeystoresRequest { + keystores: keystores.clone(), + passwords: vec![password.clone(); keystores.len()], + slashing_protection: None, + }) + .await + .unwrap(); + + // All local keystore imports should fail. Already imported as remotekeys. + check_keystore_import_response( + &import_res, + all_with_status(keystores.len(), ImportKeystoreStatus::Error), + ); + + // Check that only remotekeys are returned. + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .collect::>(); + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn delete_remotekeys_twice() { + run_test(|tester| async move { + let _ = &tester; + + // Generate some remotekeys. + let remotekeys = (0..2) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // Import all remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Delete all. + let delete_req = DeleteRemotekeysRequest { + pubkeys: remotekeys.iter().map(|k| k.pubkey).collect(), + }; + let delete_res = tester.client.delete_remotekeys(&delete_req).await.unwrap(); + check_remotekey_delete_response( + &delete_res, + all_with_status(remotekeys.len(), DeleteRemotekeyStatus::Deleted), + ); + + // Try to delete again. + let delete_res = tester.client.delete_remotekeys(&delete_req).await.unwrap(); + check_remotekey_delete_response( + &delete_res, + all_with_status(remotekeys.len(), DeleteRemotekeyStatus::NotFound), + ); + + // Check list response. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, Vec::new()); + }) +} + +#[test] +fn delete_nonexistent_remotekey() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekeys. + let remotekeys = (0..2) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // Try to delete remotekeys. + let delete_req = DeleteRemotekeysRequest { + pubkeys: remotekeys.iter().map(|k| k.pubkey).collect(), + }; + let delete_res = tester.client.delete_remotekeys(&delete_req).await.unwrap(); + check_remotekey_delete_response( + &delete_res, + all_with_status(remotekeys.len(), DeleteRemotekeyStatus::NotFound), + ); + + // Check list response. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, Vec::new()); + }) +} + +#[test] +fn delete_then_reimport_remotekeys() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekeys. + let mut remotekeys = (0..2) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // Import all remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Delete all. + let delete_req = DeleteRemotekeysRequest { + pubkeys: remotekeys.iter().map(|k| k.pubkey).collect(), + }; + let delete_res = tester.client.delete_remotekeys(&delete_req).await.unwrap(); + check_remotekey_delete_response( + &delete_res, + all_with_status(remotekeys.len(), DeleteRemotekeyStatus::Deleted), + ); + + // Change remote key url + for rk in remotekeys.iter_mut() { + rk.url = "http://localhost:1/this-url-hopefully-does-also-not-exist".into(); + } + + // Re-import + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + // Check list response. + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .collect::>(); + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_remotekey_web3signer() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekeys. + let remotekeys = (0..2) + .map(|_| new_remotekey_validator().1) + .collect::>(); + + // Generate web3signers. + let web3signers = (0..2) + .map(|_| new_web3signer_validator().1) + .collect::>(); + + // Import web3signers. + tester + .client + .post_lighthouse_validators_web3signer(&web3signers) + .await + .unwrap(); + + // Import remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: remotekeys.clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(remotekeys.len(), ImportRemotekeyStatus::Imported), + ); + + let expected_responses = remotekeys + .iter() + .map(|remotekey| SingleListRemotekeysResponse { + pubkey: remotekey.pubkey, + url: remotekey.url.clone(), + readonly: false, + }) + .chain( + web3signers + .iter() + .map(|websigner| SingleListRemotekeysResponse { + pubkey: websigner.voting_public_key.compress(), + url: websigner.url.clone(), + readonly: false, + }), + ) + .collect::>(); + + // Check remotekey list response. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_remotekey_web3signer_disabled() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekey. + let (kp, remotekey_req) = new_remotekey_validator(); + + // Generate web3signer with same PK. + let mut web3signer_req = web3signer_validator_with_pubkey(kp.pk); + web3signer_req.enable = false; + + // Import web3signers. + let _ = tester + .client + .post_lighthouse_validators_web3signer(&vec![web3signer_req]) + .await + .unwrap(); + + // 1 validator imported. + assert_eq!(tester.vals_total(), 1); + assert_eq!(tester.vals_enabled(), 0); + + // Import remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: vec![remotekey_req.clone()].clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(1, ImportRemotekeyStatus::Imported), + ); + + // Still only one validator. Web3signer is overwritten by remotekey. + assert_eq!(tester.vals_total(), 1); + assert_eq!(tester.vals_enabled(), 1); + + // Remotekey overwrites web3signer. + let expected_responses = vec![SingleListRemotekeysResponse { + pubkey: remotekey_req.pubkey, + url: remotekey_req.url.clone(), + readonly: false, + }]; + + // Check remotekey list response. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); + }) +} + +#[test] +fn import_remotekey_web3signer_enabled() { + run_test(|tester| async move { + let _ = &tester; + + // Generate remotekey. + let (kp, remotekey_req) = new_remotekey_validator(); + + // Generate web3signer with same PK. + let mut web3signer_req = web3signer_validator_with_pubkey(kp.pk); + web3signer_req.url = "http://localhost:1/this-url-hopefully-does-also-not-exist".into(); + web3signer_req.enable = true; + + // Import web3signers. + tester + .client + .post_lighthouse_validators_web3signer(&vec![web3signer_req.clone()]) + .await + .unwrap(); + + // 1 validator imported. + assert_eq!(tester.vals_total(), 1); + assert_eq!(tester.vals_enabled(), 1); + let vals = tester.initialized_validators.read(); + let web3_vals = vals.validator_definitions().clone(); + + // Import remotekeys. + let import_res = tester + .client + .post_remotekeys(&ImportRemotekeysRequest { + remote_keys: vec![remotekey_req.clone()].clone(), + }) + .await + .unwrap(); + check_remotekey_import_response( + &import_res, + all_with_status(1, ImportRemotekeyStatus::Duplicate), + ); + + assert_eq!(tester.vals_total(), 1); + assert_eq!(tester.vals_enabled(), 1); + let vals = tester.initialized_validators.read(); + let remote_vals = vals.validator_definitions().clone(); + + // Web3signer should not be overwritten since it is enabled. + assert!(web3_vals == remote_vals); + + // Remotekey should not be imported. + let expected_responses = vec![SingleListRemotekeysResponse { + pubkey: web3signer_req.voting_public_key.compress(), + url: web3signer_req.url.clone(), + readonly: false, + }]; + + // Check remotekey list response. + let get_res = tester.client.get_remotekeys().await.unwrap(); + check_remotekey_get_response(&get_res, expected_responses); }) } diff --git a/validator_client/src/initialized_validators.rs b/validator_client/src/initialized_validators.rs index a4dedf16b..2e00ef36c 100644 --- a/validator_client/src/initialized_validators.rs +++ b/validator_client/src/initialized_validators.rs @@ -14,7 +14,6 @@ use account_utils::{ }, ZeroizeString, }; -use eth2::lighthouse_vc::std_types::DeleteKeystoreStatus; use eth2_keystore::Keystore; use lighthouse_metrics::set_gauge; use lockfile::{Lockfile, LockfileError}; @@ -90,8 +89,8 @@ pub enum Error { InvalidWeb3SignerRootCertificateFile(io::Error), InvalidWeb3SignerRootCertificate(ReqwestError), UnableToBuildWeb3SignerClient(ReqwestError), - /// Unable to apply an action to a validator because it is using a remote signer. - InvalidActionOnRemoteValidator, + /// Unable to apply an action to a validator. + InvalidActionOnValidator, } impl From for Error { @@ -443,7 +442,8 @@ impl InitializedValidators { pub async fn delete_definition_and_keystore( &mut self, pubkey: &PublicKey, - ) -> Result { + is_local_keystore: bool, + ) -> Result<(), Error> { // 1. Disable the validator definition. // // We disable before removing so that in case of a crash the auto-discovery mechanism @@ -454,16 +454,19 @@ impl InitializedValidators { .iter_mut() .find(|def| &def.voting_public_key == pubkey) { - if def.signing_definition.is_local_keystore() { + // Update definition for local keystore + if def.signing_definition.is_local_keystore() && is_local_keystore { def.enabled = false; self.definitions .save(&self.validators_dir) .map_err(Error::UnableToSaveDefinitions)?; + } else if !def.signing_definition.is_local_keystore() && !is_local_keystore { + def.enabled = false; } else { - return Err(Error::InvalidActionOnRemoteValidator); + return Err(Error::InvalidActionOnValidator); } } else { - return Ok(DeleteKeystoreStatus::NotFound); + return Err(Error::ValidatorNotInitialized(pubkey.clone())); } // 2. Delete from `self.validators`, which holds the signing method. @@ -491,7 +494,7 @@ impl InitializedValidators { .save(&self.validators_dir) .map_err(Error::UnableToSaveDefinitions)?; - Ok(DeleteKeystoreStatus::Deleted) + Ok(()) } /// Attempt to delete the voting keystore file, or its entire validator directory.