From f27b3b83ad3ce2857d8b2680c3862a202ed9a9b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 29 Apr 2025 13:38:40 -0300 Subject: [PATCH 1/3] Remove unwraps --- crates/common/src/signer/loader.rs | 5 +++-- crates/common/src/signer/schemes/ecdsa.rs | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/common/src/signer/loader.rs b/crates/common/src/signer/loader.rs index 1ed0b068..c1e3d00e 100644 --- a/crates/common/src/signer/loader.rs +++ b/crates/common/src/signer/loader.rs @@ -295,11 +295,12 @@ pub fn load_bls_signer(keys_path: PathBuf, secrets_path: PathBuf) -> eyre::Resul } pub fn load_ecdsa_signer(keys_path: PathBuf, secrets_path: PathBuf) -> eyre::Result { - let key_file = std::fs::File::open(keys_path.to_string_lossy().to_string())?; + let key_file = std::fs::File::open(keys_path)?; let key_reader = std::io::BufReader::new(key_file); let keystore: JsonKeystore = serde_json::from_reader(key_reader)?; let password = std::fs::read(secrets_path)?; - let decrypted_password = eth2_keystore::decrypt(&password, &keystore.crypto).unwrap(); + let decrypted_password = eth2_keystore::decrypt(&password, &keystore.crypto) + .map_err(|_| eyre::eyre!("Error decrypting ECDSA keystore"))?; EcdsaSigner::new_from_bytes(decrypted_password.as_bytes()) } diff --git a/crates/common/src/signer/schemes/ecdsa.rs b/crates/common/src/signer/schemes/ecdsa.rs index f84020a0..612df5e3 100644 --- a/crates/common/src/signer/schemes/ecdsa.rs +++ b/crates/common/src/signer/schemes/ecdsa.rs @@ -1,7 +1,7 @@ use std::{ops::Deref, str::FromStr}; use alloy::{ - primitives::{Address, PrimitiveSignature, B256}, + primitives::{Address, PrimitiveSignature}, signers::{local::PrivateKeySigner, SignerSync}, }; use eyre::ensure; @@ -63,8 +63,7 @@ pub enum EcdsaSigner { impl EcdsaSigner { pub fn new_random() -> Self { - let secret = B256::random(); - Self::new_from_bytes(secret.as_slice()).unwrap() + Self::Local(PrivateKeySigner::random()) } pub fn new_from_bytes(bytes: &[u8]) -> eyre::Result { From 1d4f4487589ee8d5d24a9a7e304ae5480d8f34e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 29 Apr 2025 13:55:31 -0300 Subject: [PATCH 2/3] Use PathBuf instead of String --- crates/common/src/signer/loader.rs | 52 ++++++++++++------------------ 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/crates/common/src/signer/loader.rs b/crates/common/src/signer/loader.rs index c1e3d00e..63d8a045 100644 --- a/crates/common/src/signer/loader.rs +++ b/crates/common/src/signer/loader.rs @@ -73,12 +73,10 @@ impl SignerLoader { SignerLoader::ValidatorsDir { keys_path, secrets_path, format } => { // TODO: hacky way to load for now, we should support reading the // definitions.yml file - let keys_path = load_env_var(SIGNER_DIR_KEYS_ENV).unwrap_or( - keys_path.to_str().ok_or_eyre("Missing signer keys path")?.to_string(), - ); - let secrets_path = load_env_var(SIGNER_DIR_SECRETS_ENV).unwrap_or( - secrets_path.to_str().ok_or_eyre("Missing signer secrets path")?.to_string(), - ); + let keys_path = + load_env_var(SIGNER_DIR_KEYS_ENV).map(PathBuf::from).unwrap_or(keys_path); + let secrets_path = + load_env_var(SIGNER_DIR_SECRETS_ENV).map(PathBuf::from).unwrap_or(secrets_path); return match format { ValidatorKeysFormat::Lighthouse => { @@ -114,8 +112,8 @@ impl<'de> Deserialize<'de> for FileKey { } fn load_from_lighthouse_format( - keys_path: String, - secrets_path: String, + keys_path: PathBuf, + secrets_path: PathBuf, ) -> eyre::Result> { let entries = fs::read_dir(keys_path.clone())?; @@ -129,8 +127,8 @@ fn load_from_lighthouse_format( if path.is_dir() { if let Some(maybe_pubkey) = path.file_name().and_then(|d| d.to_str()) { if let Ok(pubkey) = BlsPublicKey::from_hex(maybe_pubkey) { - let ks_path = format!("{}/{}/voting-keystore.json", keys_path, maybe_pubkey); - let pw_path = format!("{}/{}", secrets_path, pubkey); + let ks_path = keys_path.join(maybe_pubkey).join("voting-keystore.json"); + let pw_path = secrets_path.join(pubkey.to_string()); match load_one(ks_path, pw_path) { Ok(signer) => signers.push(signer), @@ -147,8 +145,8 @@ fn load_from_lighthouse_format( } fn load_from_teku_format( - keys_path: String, - secrets_path: String, + keys_path: PathBuf, + secrets_path: PathBuf, ) -> eyre::Result> { let entries = fs::read_dir(keys_path.clone())?; let mut signers = Vec::new(); @@ -171,8 +169,8 @@ fn load_from_teku_format( .0; match load_one( - format!("{keys_path}/{file_name}.json"), - format!("{secrets_path}/{file_name}.txt"), + keys_path.join(format!("{file_name}.json")), + secrets_path.join(format!("{file_name}.txt")), ) { Ok(signer) => signers.push(signer), Err(e) => warn!("Sign load error: {e}"), @@ -183,8 +181,8 @@ fn load_from_teku_format( } fn load_from_lodestar_format( - keys_path: String, - password_path: String, + keys_path: PathBuf, + password_path: PathBuf, ) -> eyre::Result> { let entries = fs::read_dir(keys_path)?; let mut signers = Vec::new(); @@ -198,15 +196,7 @@ fn load_from_lodestar_format( continue; } - let key_path = match path.as_os_str().to_str() { - Some(key_path) => key_path, - None => { - warn!("Path {path:?} cannot be converted to string"); - continue; - } - }; - - match load_one(key_path.to_string(), password_path.clone()) { + match load_one(path, password_path.clone()) { Ok(signer) => signers.push(signer), Err(e) => warn!("Sign load error: {e}"), } @@ -233,8 +223,8 @@ fn load_from_lodestar_format( /// } /// ``` fn load_from_prysm_format( - accounts_path: String, - password_path: String, + accounts_path: PathBuf, + password_path: PathBuf, ) -> eyre::Result> { let accounts_file = File::open(accounts_path)?; let accounts_reader = BufReader::new(accounts_file); @@ -281,17 +271,17 @@ fn load_from_prysm_format( Ok(signers) } -fn load_one(ks_path: String, pw_path: String) -> eyre::Result { +fn load_one(ks_path: PathBuf, pw_path: PathBuf) -> eyre::Result { let keystore = Keystore::from_json_file(ks_path).map_err(|_| eyre!("failed reading json"))?; - let password = - fs::read(pw_path.clone()).map_err(|e| eyre!("Failed to read password ({pw_path}): {e}"))?; + let password = fs::read(pw_path.clone()) + .map_err(|e| eyre!("Failed to read password ({}): {e}", pw_path.display()))?; let key = keystore.decrypt_keypair(&password).map_err(|_| eyre!("failed decrypting keypair"))?; ConsensusSigner::new_from_bytes(key.sk.serialize().as_bytes()) } pub fn load_bls_signer(keys_path: PathBuf, secrets_path: PathBuf) -> eyre::Result { - load_one(keys_path.to_string_lossy().to_string(), secrets_path.to_string_lossy().to_string()) + load_one(keys_path, secrets_path) } pub fn load_ecdsa_signer(keys_path: PathBuf, secrets_path: PathBuf) -> eyre::Result { From 7cc8d86d04eef02ea7ce460ab32ac8dbef69b90c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 29 Apr 2025 13:56:57 -0300 Subject: [PATCH 3/3] Clean up --- crates/common/src/signer/loader.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/common/src/signer/loader.rs b/crates/common/src/signer/loader.rs index 63d8a045..9e60d85b 100644 --- a/crates/common/src/signer/loader.rs +++ b/crates/common/src/signer/loader.rs @@ -57,9 +57,7 @@ impl SignerLoader { pub fn load_from_env(self) -> eyre::Result> { Ok(match self { SignerLoader::File { key_path } => { - let path = load_env_var(SIGNER_KEYS_ENV).unwrap_or( - key_path.to_str().ok_or_eyre("Missing signer key path")?.to_string(), - ); + let path = load_env_var(SIGNER_KEYS_ENV).map(PathBuf::from).unwrap_or(key_path); let file = std::fs::read_to_string(path) .unwrap_or_else(|_| panic!("Unable to find keys file"));