From 4c4f2271e161c42c8b6a6be14aec079d3ec18df1 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 11 Jul 2023 12:26:40 +1000 Subject: [PATCH] Check key cache consistency in tests --- validator_client/src/http_api/test_utils.rs | 23 ++++++++++++++--- .../src/initialized_validators.rs | 25 ++++++++++++++----- validator_manager/src/import_validators.rs | 2 ++ validator_manager/src/move_validators.rs | 3 +++ 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/validator_client/src/http_api/test_utils.rs b/validator_client/src/http_api/test_utils.rs index fc9083ea01..d221de1425 100644 --- a/validator_client/src/http_api/test_utils.rs +++ b/validator_client/src/http_api/test_utils.rs @@ -1,7 +1,8 @@ use crate::doppelganger_service::DoppelgangerService; +use crate::key_cache::{KeyCache, CACHE_FILENAME}; use crate::{ http_api::{ApiSecret, Config as HttpConfig, Context}, - initialized_validators::InitializedValidators, + initialized_validators::{InitializedValidators, OnDecryptFailure}, Config, ValidatorDefinitions, ValidatorStore, }; use account_utils::{ @@ -59,7 +60,7 @@ pub struct ApiTester { pub api_token: String, pub test_runtime: TestRuntime, pub _server_shutdown: oneshot::Sender<()>, - pub _validator_dir: TempDir, + pub validator_dir: TempDir, } impl ApiTester { @@ -166,10 +167,26 @@ impl ApiTester { api_token: api_pubkey, test_runtime, _server_shutdown: shutdown_tx, - _validator_dir: validator_dir, + validator_dir, } } + /// Checks that the key cache exists and can be decrypted with the current + /// set of known validators. + pub async fn ensure_key_cache_consistency(&self) { + assert!( + self.validator_dir.as_ref().join(CACHE_FILENAME).exists(), + "the key cache should exist" + ); + let key_cache = + KeyCache::open_or_create(self.validator_dir.as_ref()).expect("should open a key cache"); + self.initialized_validators + .read() + .decrypt_key_cache(key_cache, &mut <_>::default(), OnDecryptFailure::Error) + .await + .expect("key cache should decypt"); + } + pub fn invalid_token_client(&self) -> ValidatorClientHttpClient { let tmp = tempdir().unwrap(); let api_secret = ApiSecret::create_or_open(tmp.path()).unwrap(); diff --git a/validator_client/src/initialized_validators.rs b/validator_client/src/initialized_validators.rs index bf9890d6b6..6eebeb7820 100644 --- a/validator_client/src/initialized_validators.rs +++ b/validator_client/src/initialized_validators.rs @@ -44,6 +44,14 @@ const DEFAULT_REMOTE_SIGNER_REQUEST_TIMEOUT: Duration = Duration::from_secs(12); // Use TTY instead of stdin to capture passwords from users. const USE_STDIN: bool = false; +pub enum OnDecryptFailure { + /// If the key cache fails to decrypt, create a new cache. + CreateNew, + /// Return an error if the key cache fails to decrypt. This should only be + /// used in testing. + Error, +} + pub struct KeystoreAndPassword { pub keystore: Keystore, pub password: Option, @@ -106,6 +114,7 @@ pub enum Error { UnableToReadValidatorPassword(String), UnableToReadKeystoreFile(eth2_keystore::Error), UnableToSaveKeyCache(key_cache::Error), + UnableToDecryptKeyCache(key_cache::Error), } impl From for Error { @@ -606,7 +615,7 @@ impl InitializedValidators { let key_cache = KeyCache::open_or_create(&self.validators_dir) .map_err(Error::UnableToOpenKeyCache)?; let mut decrypted_key_cache = self - .decrypt_key_cache(key_cache, &mut <_>::default()) + .decrypt_key_cache(key_cache, &mut <_>::default(), OnDecryptFailure::CreateNew) .await?; decrypted_key_cache.remove(&uuid); decrypted_key_cache @@ -949,10 +958,11 @@ impl InitializedValidators { /// filesystem accesses for keystores that are already known. In the case that a keystore /// from the validator definitions is not yet in this map, it will be loaded from disk and /// inserted into the map. - async fn decrypt_key_cache( + pub async fn decrypt_key_cache( &self, mut cache: KeyCache, key_stores: &mut HashMap, + on_failure: OnDecryptFailure, ) -> Result { // Read relevant key stores from the filesystem. let mut definitions_map = HashMap::new(); @@ -1020,11 +1030,13 @@ impl InitializedValidators { //decrypt tokio::task::spawn_blocking(move || match cache.decrypt(passwords, public_keys) { - Ok(_) | Err(key_cache::Error::AlreadyDecrypted) => cache, - _ => KeyCache::new(), + Ok(_) | Err(key_cache::Error::AlreadyDecrypted) => Ok(cache), + _ if matches!(on_failure, OnDecryptFailure::CreateNew) => Ok(KeyCache::new()), + Err(e) => Err(e), }) .await - .map_err(Error::TokioJoin) + .map_err(Error::TokioJoin)? + .map_err(Error::UnableToDecryptKeyCache) } /// Scans `self.definitions` and attempts to initialize and validators which are not already @@ -1062,7 +1074,8 @@ impl InitializedValidators { // Only decrypt cache when there is at least one local definition. // Decrypting cache is a very expensive operation which is never used for web3signer. let mut key_cache = if has_local_definitions { - self.decrypt_key_cache(cache, &mut key_stores).await? + self.decrypt_key_cache(cache, &mut key_stores, OnDecryptFailure::CreateNew) + .await? } else { // Assign an empty KeyCache if all definitions are of the Web3Signer type. KeyCache::new() diff --git a/validator_manager/src/import_validators.rs b/validator_manager/src/import_validators.rs index c9e077eb85..10db19b008 100644 --- a/validator_manager/src/import_validators.rs +++ b/validator_manager/src/import_validators.rs @@ -307,6 +307,8 @@ pub mod tests { let result = run(self.import_config.clone()).await; if result.is_ok() { + self.vc.ensure_key_cache_consistency().await; + let local_validators: Vec = { let contents = fs::read_to_string(&self.import_config.validators_file_path).unwrap(); diff --git a/validator_manager/src/move_validators.rs b/validator_manager/src/move_validators.rs index 16b40204f6..cacfbb5dc5 100644 --- a/validator_manager/src/move_validators.rs +++ b/validator_manager/src/move_validators.rs @@ -754,6 +754,9 @@ mod test { let src_vc_final_keystores = src_vc_client.get_keystores().await.unwrap().data; let dest_vc_final_keystores = dest_vc_client.get_keystores().await.unwrap().data; + src_vc.ensure_key_cache_consistency().await; + dest_vc.ensure_key_cache_consistency().await; + match validators { Validators::All => { assert!(