From 1be714d97d9c6cbbf1f46a894cc162db36feaecf Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 13:54:10 +0200 Subject: [PATCH 1/8] Removes FIXME comment --- wasm_for_tests/tx_fail/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/wasm_for_tests/tx_fail/Cargo.toml b/wasm_for_tests/tx_fail/Cargo.toml index f64c125dee..8610f2301f 100644 --- a/wasm_for_tests/tx_fail/Cargo.toml +++ b/wasm_for_tests/tx_fail/Cargo.toml @@ -8,7 +8,6 @@ version.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html -# FIXME: also check these normal deps for all the wasm for tests [dependencies] namada_tx_prelude.workspace = true rlsf.workspace = true From 43336ef63a3d33deb95bb2eb08dd4af0ca6e6232 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 13:54:21 +0200 Subject: [PATCH 2/8] Propagates error in sdk if decryption of signing key fails --- crates/apps_lib/src/cli/wallet.rs | 1 + crates/apps_lib/src/client/tx.rs | 2 ++ crates/apps_lib/src/wallet/mod.rs | 4 ++++ crates/sdk/src/signing.rs | 38 ++++++++++++++++--------------- crates/sdk/src/wallet/mod.rs | 36 +++++++++++++++-------------- 5 files changed, 46 insertions(+), 35 deletions(-) diff --git a/crates/apps_lib/src/cli/wallet.rs b/crates/apps_lib/src/cli/wallet.rs index ebdce5d35c..e22bb8523a 100644 --- a/crates/apps_lib/src/cli/wallet.rs +++ b/crates/apps_lib/src/cli/wallet.rs @@ -885,6 +885,7 @@ fn transparent_key_find( public_key_hash: Option, unsafe_show_secret: bool, ) { + eprintln!("IN TRANSPARENT KEY FIND"); //FIXME: remove let mut wallet = load_wallet(ctx); let found_keypair = match public_key { Some(pk) => wallet.find_key_by_pk(&pk, None), diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index c640aec100..9887fb6b68 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -1039,6 +1039,8 @@ where { Some(secret_key) } else { + //FIXME: also here, we should crash if possible + //FIXME: ah wait it depends what we return from aux_signing_data edisplay_line!( namada.io(), "Couldn't find the secret key for {}. Skipping signature \ diff --git a/crates/apps_lib/src/wallet/mod.rs b/crates/apps_lib/src/wallet/mod.rs index b841240c59..27afc07a44 100644 --- a/crates/apps_lib/src/wallet/mod.rs +++ b/crates/apps_lib/src/wallet/mod.rs @@ -43,6 +43,7 @@ impl FsWalletStorage for CliWalletUtils { impl WalletIo for CliWalletUtils { type Rng = OsRng; + //FIXME: this functio nmust be called twice somewhere fn read_password(confirm: bool) -> Zeroizing { let pwd = match env::var("NAMADA_WALLET_PASSWORD_FILE") { Ok(path) => Zeroizing::new( @@ -64,6 +65,8 @@ impl WalletIo for CliWalletUtils { ) } Err(_) => { + //FIXME: this is out branch + eprintln!("IN OUR BRANCH"); //FIXME :remove let prompt = "Enter your decryption password: "; rpassword::read_password_from_tty(Some(prompt)) .map(Zeroizing::new) @@ -226,6 +229,7 @@ where F: Fn(&ValidatorData) -> common::SecretKey, U: WalletIo, { + eprintln!("IN FIND SECRET KEY WRAPPER"); //FIXME: remove maybe_pk .map(|pk| { let pkh = PublicKeyHash::from(&pk); diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 5300cf34c7..a094c85c9f 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -61,6 +61,7 @@ use crate::{args, display_line, rpc, MaybeSend, Namada}; pub struct SigningTxData { /// The address owning the transaction pub owner: Option
, + //FIXME: review this comment because this looks like the list of all the pubkeys associated to a multisig account but it doesn't look like so /// The public keys associated to an account pub public_keys: Vec, /// The threshold associated to an account @@ -226,23 +227,20 @@ where if let Some(account_public_keys_map) = signing_data.account_public_keys_map { let mut wallet = wallet.write().await; - let signing_tx_keypairs = signing_data - .public_keys - .iter() - .filter_map(|public_key| { - if used_pubkeys.contains(public_key) { - None - } else { - match find_key_by_pk(&mut wallet, args, public_key) { - Ok(secret_key) => { - used_pubkeys.insert(public_key.clone()); - Some(secret_key) - } - Err(_) => None, - } - } - }) - .collect::>(); + //FIXME: Maybe try_fold + let mut signing_tx_keypairs = vec![]; + + for public_key in &signing_data.public_keys { + if !used_pubkeys.contains(public_key) { + //FIXME: so we need to: + // - log which alias we are trying to retrieve from the wallet + // - crash the client if password is wrong (or let the user retry another time) + let secret_key = find_key_by_pk(&mut wallet, args, public_key)?; + used_pubkeys.insert(public_key.clone()); + signing_tx_keypairs.push(secret_key); + } + } + if !signing_tx_keypairs.is_empty() { tx.sign_raw( signing_tx_keypairs, @@ -269,7 +267,7 @@ where } } - // Then try signing the fee header with the software wallet otherwise use + // Then try signing the wrapper header (fee payer) with the software wallet otherwise use // the fallback let key = { // Lock the wallet just long enough to extract a key from it without @@ -282,6 +280,9 @@ where tx.sign_wrapper(fee_payer_keypair); } Err(_) => { + //FIXME: in this case we should sign the entire tx with the first signer right? + //FIXME: also in this case I'm not sure this is correct, if we failed to retrieve the fee payer that the user specified we should crash + //FIXME: unless this is needed for the hardware wallet but I don't think so *tx = sign( tx.clone(), signing_data.fee_payer.clone(), @@ -337,6 +338,7 @@ pub async fn aux_signing_data( ), }; + //FIXME: seems like here we already do all the checks for the fee payer, should we remove the redundant thing from the other side? let fee_payer = if args.disposable_signing_key { context .wallet_mut() diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index a54930878e..73702de973 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -847,7 +847,8 @@ impl Wallet { alias_pkh_or_pk: impl AsRef, password: Option>, ) -> Result { - // Try cache first + eprintln!("IN FIND SECRET KEY"); //FIXME: remove + // Try cache first if let Some(cached_key) = self .decrypted_key_cache .get(&Alias::from(alias_pkh_or_pk.as_ref())) @@ -940,7 +941,9 @@ impl Wallet { pk: &common::PublicKey, password: Option>, ) -> Result { - // Try to look-up alias for the given pk. Otherwise, use the PKH string. + //FIXME: we enter here + eprintln!("IN SDK"); //FIXME: remove + // Try to look-up alias for the given pk. Otherwise, use the PKH string. let pkh: PublicKeyHash = pk.into(); self.find_key_by_pkh(&pkh, password) } @@ -978,7 +981,9 @@ impl Wallet { pkh: &PublicKeyHash, password: Option>, ) -> Result { - // Try to look-up alias for the given pk. Otherwise, use the PKH string. + //FIXME: we enter here + eprintln!("IN FIND KEY BY PKH"); //FIXME: remove + // Try to look-up alias for the given pk. Otherwise, use the PKH string. let alias = self .store .find_alias_by_pkh(pkh) @@ -1017,6 +1022,7 @@ impl Wallet { { match stored_key { StoredKeypair::Encrypted(encrypted) => { + eprintln!("ABOUT TO ASK PASSWORD TO THE USER"); //FIXME :remove let password = password.unwrap_or_else(|| U::read_password(false)); let key = encrypted @@ -1272,22 +1278,18 @@ mod tests { // check that indeed the first keypair was not gc'd let keypair_1_pk = keypair_1().to_public(); - assert!( - wallet - .store - .get_public_keys() - .values() - .any(|pk| *pk == keypair_1_pk) - ); + assert!(wallet + .store + .get_public_keys() + .values() + .any(|pk| *pk == keypair_1_pk)); // check that the only other present key is the newly generated sk let new_key_pk = new_key.to_public(); - assert!( - wallet - .store - .get_public_keys() - .values() - .any(|pk| *pk == new_key_pk) - ); + assert!(wallet + .store + .get_public_keys() + .values() + .any(|pk| *pk == new_key_pk)); } } From 5a566093cee098aac98267c10e60cdbba4f7556b Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 15:12:07 +0200 Subject: [PATCH 3/8] Terminates client if decryption of signing key fails --- crates/apps_lib/src/client/tx.rs | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 9887fb6b68..4d2c2897a9 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -1030,32 +1030,18 @@ where .await?; let mut wallet = namada.wallet_mut().await; - let secret_keys = &signing_data - .public_keys - .iter() - .filter_map(|public_key| { - if let Ok(secret_key) = - signing::find_key_by_pk(&mut wallet, &tx_args, public_key) - { - Some(secret_key) - } else { - //FIXME: also here, we should crash if possible - //FIXME: ah wait it depends what we return from aux_signing_data - edisplay_line!( - namada.io(), - "Couldn't find the secret key for {}. Skipping signature \ - generation.", - public_key - ); - None - } - }) - .collect::>(); + + let mut secret_keys = vec![]; + for public_key in &signing_data.public_keys { + let secret_key = + signing::find_key_by_pk(&mut wallet, &tx_args, public_key)?; + secret_keys.push(secret_key); + } if let Some(account_public_keys_map) = signing_data.account_public_keys_map { let signatures = tx.compute_section_signature( - secret_keys, + &secret_keys, &account_public_keys_map, Some(owner), ); From 0c084961013cc516af4f87c4fbab9fbde3fe3e4e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 15:42:34 +0200 Subject: [PATCH 4/8] Prompts the key that we are trying to decrypt --- crates/apps_lib/src/cli/wallet.rs | 13 +++++-- crates/apps_lib/src/wallet/mod.rs | 24 ++++++++---- crates/apps_lib/src/wallet/pre_genesis.rs | 26 ++++++++----- crates/sdk/src/wallet/keys.rs | 6 ++- crates/sdk/src/wallet/mod.rs | 46 ++++++++++++----------- 5 files changed, 70 insertions(+), 45 deletions(-) diff --git a/crates/apps_lib/src/cli/wallet.rs b/crates/apps_lib/src/cli/wallet.rs index e22bb8523a..14a0eab8cb 100644 --- a/crates/apps_lib/src/cli/wallet.rs +++ b/crates/apps_lib/src/cli/wallet.rs @@ -114,7 +114,11 @@ fn shielded_keys_list( // A subset of viewing keys will have corresponding spending keys. // Print those too if they are available and requested. if let Some(spending_key) = spending_key_opt { - match spending_key.get::(decrypt, None) { + match spending_key.get::( + decrypt, + None, + Some(&alias), + ) { // Here the spending key is unencrypted or successfully // decrypted Ok(spending_key) => { @@ -885,7 +889,6 @@ fn transparent_key_find( public_key_hash: Option, unsafe_show_secret: bool, ) { - eprintln!("IN TRANSPARENT KEY FIND"); //FIXME: remove let mut wallet = load_wallet(ctx); let found_keypair = match public_key { Some(pk) => wallet.find_key_by_pk(&pk, None), @@ -1202,7 +1205,11 @@ fn transparent_keys_list( // A subset of public keys will have corresponding secret keys. // Print those too if they are available and requested. if let Some((stored_keypair, _pkh)) = stored_keypair { - match stored_keypair.get::(decrypt, None) { + match stored_keypair.get::( + decrypt, + None, + Some(&alias), + ) { Ok(keypair) => { if unsafe_show_secret { display_line!(io, diff --git a/crates/apps_lib/src/wallet/mod.rs b/crates/apps_lib/src/wallet/mod.rs index 27afc07a44..56b66d5879 100644 --- a/crates/apps_lib/src/wallet/mod.rs +++ b/crates/apps_lib/src/wallet/mod.rs @@ -3,6 +3,7 @@ pub mod pre_genesis; mod store; mod transport; +use std::borrow::Cow; use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::{env, fs}; @@ -43,8 +44,10 @@ impl FsWalletStorage for CliWalletUtils { impl WalletIo for CliWalletUtils { type Rng = OsRng; - //FIXME: this functio nmust be called twice somewhere - fn read_password(confirm: bool) -> Zeroizing { + fn read_password( + confirm: bool, + target_key: Option<&str>, + ) -> Zeroizing { let pwd = match env::var("NAMADA_WALLET_PASSWORD_FILE") { Ok(path) => Zeroizing::new( fs::read_to_string(path) @@ -65,10 +68,16 @@ impl WalletIo for CliWalletUtils { ) } Err(_) => { - //FIXME: this is out branch - eprintln!("IN OUR BRANCH"); //FIXME :remove - let prompt = "Enter your decryption password: "; - rpassword::read_password_from_tty(Some(prompt)) + let prompt = match target_key { + Some(target) => Cow::Owned(format!( + "Enter your decryption password for {}: ", + target + )), + None => { + Cow::Borrowed("Enter your decryption password: ") + } + }; + rpassword::read_password_from_tty(Some(&prompt)) .map(Zeroizing::new) .expect("Failed reading password from tty.") } @@ -229,7 +238,6 @@ where F: Fn(&ValidatorData) -> common::SecretKey, U: WalletIo, { - eprintln!("IN FIND SECRET KEY WRAPPER"); //FIXME: remove maybe_pk .map(|pk| { let pkh = PublicKeyHash::from(&pk); @@ -286,7 +294,7 @@ pub fn read_and_confirm_encryption_password( println!("Warning: The keypair will NOT be encrypted."); None } else { - Some(CliWalletUtils::read_password(true)) + Some(CliWalletUtils::read_password(true, None)) } } diff --git a/crates/apps_lib/src/wallet/pre_genesis.rs b/crates/apps_lib/src/wallet/pre_genesis.rs index d383688831..34041b523c 100644 --- a/crates/apps_lib/src/wallet/pre_genesis.rs +++ b/crates/apps_lib/src/wallet/pre_genesis.rs @@ -73,21 +73,27 @@ pub fn load(store_dir: &Path) -> Result { let store = ValidatorStore::decode(store).map_err(ReadError::Decode)?; let password = if store.consensus_key.is_encrypted() { - Some(CliWalletUtils::read_password(false)) + Some(CliWalletUtils::read_password(false, Some("consensus key"))) } else { None }; - let consensus_key = store - .consensus_key - .get::(true, password.clone())?; - let eth_cold_key = store - .eth_cold_key - .get::(true, password.clone())?; + let consensus_key = store.consensus_key.get::( + true, + password.clone(), + Some("consensus key"), + )?; + let eth_cold_key = store.eth_cold_key.get::( + true, + password.clone(), + Some("eth cold key"), + )?; let eth_hot_key = store.validator_keys.eth_bridge_keypair.clone(); - let tendermint_node_key = store - .tendermint_node_key - .get::(true, password)?; + let tendermint_node_key = store.tendermint_node_key.get::( + true, + password, + Some("tendermint node key"), + )?; Ok(ValidatorWallet { store, diff --git a/crates/sdk/src/wallet/keys.rs b/crates/sdk/src/wallet/keys.rs index 17e0dc4077..34b8c62f82 100644 --- a/crates/sdk/src/wallet/keys.rs +++ b/crates/sdk/src/wallet/keys.rs @@ -304,14 +304,16 @@ where /// from stdin. pub fn get( &self, + // FIXME: review these args decrypt: bool, password: Option>, + target_key: Option<&str>, ) -> Result { match self { StoredKeypair::Encrypted(encrypted_keypair) => { if decrypt { - let password = - password.unwrap_or_else(|| U::read_password(false)); + let password = password + .unwrap_or_else(|| U::read_password(false, target_key)); let key = encrypted_keypair.decrypt(password)?; Ok(key) } else { diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 73702de973..16256deb36 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -62,7 +62,10 @@ pub trait WalletIo: Sized + Clone { } /// Read the password for decryption from the file/env/stdin. - fn read_password(_confirm: bool) -> Zeroizing { + fn read_password( + _confirm: bool, + _target_key: Option<&str>, + ) -> Zeroizing { panic!("attempted to prompt for password in non-interactive mode"); } @@ -847,8 +850,7 @@ impl Wallet { alias_pkh_or_pk: impl AsRef, password: Option>, ) -> Result { - eprintln!("IN FIND SECRET KEY"); //FIXME: remove - // Try cache first + // Try cache first if let Some(cached_key) = self .decrypted_key_cache .get(&Alias::from(alias_pkh_or_pk.as_ref())) @@ -941,9 +943,7 @@ impl Wallet { pk: &common::PublicKey, password: Option>, ) -> Result { - //FIXME: we enter here - eprintln!("IN SDK"); //FIXME: remove - // Try to look-up alias for the given pk. Otherwise, use the PKH string. + // Try to look-up alias for the given pk. Otherwise, use the PKH string. let pkh: PublicKeyHash = pk.into(); self.find_key_by_pkh(&pkh, password) } @@ -981,9 +981,7 @@ impl Wallet { pkh: &PublicKeyHash, password: Option>, ) -> Result { - //FIXME: we enter here - eprintln!("IN FIND KEY BY PKH"); //FIXME: remove - // Try to look-up alias for the given pk. Otherwise, use the PKH string. + // Try to look-up alias for the given pk. Otherwise, use the PKH string. let alias = self .store .find_alias_by_pkh(pkh) @@ -1022,9 +1020,9 @@ impl Wallet { { match stored_key { StoredKeypair::Encrypted(encrypted) => { - eprintln!("ABOUT TO ASK PASSWORD TO THE USER"); //FIXME :remove - let password = - password.unwrap_or_else(|| U::read_password(false)); + let password = password.unwrap_or_else(|| { + U::read_password(false, Some(&alias.to_string())) + }); let key = encrypted .decrypt(password) .map_err(FindKeyError::KeyDecryptionError)?; @@ -1278,18 +1276,22 @@ mod tests { // check that indeed the first keypair was not gc'd let keypair_1_pk = keypair_1().to_public(); - assert!(wallet - .store - .get_public_keys() - .values() - .any(|pk| *pk == keypair_1_pk)); + assert!( + wallet + .store + .get_public_keys() + .values() + .any(|pk| *pk == keypair_1_pk) + ); // check that the only other present key is the newly generated sk let new_key_pk = new_key.to_public(); - assert!(wallet - .store - .get_public_keys() - .values() - .any(|pk| *pk == new_key_pk)); + assert!( + wallet + .store + .get_public_keys() + .values() + .any(|pk| *pk == new_key_pk) + ); } } From 1d40166314bf23998dcf352f3e37a3667b8498f9 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 16:37:45 +0200 Subject: [PATCH 5/8] Removes comments --- crates/sdk/src/signing.rs | 14 +++----------- crates/sdk/src/wallet/keys.rs | 1 - 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index a094c85c9f..90bd2aaf90 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -61,7 +61,6 @@ use crate::{args, display_line, rpc, MaybeSend, Namada}; pub struct SigningTxData { /// The address owning the transaction pub owner: Option
, - //FIXME: review this comment because this looks like the list of all the pubkeys associated to a multisig account but it doesn't look like so /// The public keys associated to an account pub public_keys: Vec, /// The threshold associated to an account @@ -227,14 +226,11 @@ where if let Some(account_public_keys_map) = signing_data.account_public_keys_map { let mut wallet = wallet.write().await; - //FIXME: Maybe try_fold let mut signing_tx_keypairs = vec![]; for public_key in &signing_data.public_keys { if !used_pubkeys.contains(public_key) { - //FIXME: so we need to: - // - log which alias we are trying to retrieve from the wallet - // - crash the client if password is wrong (or let the user retry another time) + // FIXME: should allow for a retry? let secret_key = find_key_by_pk(&mut wallet, args, public_key)?; used_pubkeys.insert(public_key.clone()); signing_tx_keypairs.push(secret_key); @@ -267,8 +263,8 @@ where } } - // Then try signing the wrapper header (fee payer) with the software wallet otherwise use - // the fallback + // Then try signing the wrapper header (fee payer) with the software wallet + // otherwise use the fallback let key = { // Lock the wallet just long enough to extract a key from it without // interfering with the sign closure call @@ -280,9 +276,6 @@ where tx.sign_wrapper(fee_payer_keypair); } Err(_) => { - //FIXME: in this case we should sign the entire tx with the first signer right? - //FIXME: also in this case I'm not sure this is correct, if we failed to retrieve the fee payer that the user specified we should crash - //FIXME: unless this is needed for the hardware wallet but I don't think so *tx = sign( tx.clone(), signing_data.fee_payer.clone(), @@ -338,7 +331,6 @@ pub async fn aux_signing_data( ), }; - //FIXME: seems like here we already do all the checks for the fee payer, should we remove the redundant thing from the other side? let fee_payer = if args.disposable_signing_key { context .wallet_mut() diff --git a/crates/sdk/src/wallet/keys.rs b/crates/sdk/src/wallet/keys.rs index 34b8c62f82..4da7a1e584 100644 --- a/crates/sdk/src/wallet/keys.rs +++ b/crates/sdk/src/wallet/keys.rs @@ -304,7 +304,6 @@ where /// from stdin. pub fn get( &self, - // FIXME: review these args decrypt: bool, password: Option>, target_key: Option<&str>, From 249aaf2edaf4822e42bfb3c24f828251a42101f7 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 17:15:56 +0200 Subject: [PATCH 6/8] Two attempts for decryption password --- crates/sdk/src/signing.rs | 1 - crates/sdk/src/wallet/mod.rs | 29 +++++++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 90bd2aaf90..9e487522cc 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -230,7 +230,6 @@ where for public_key in &signing_data.public_keys { if !used_pubkeys.contains(public_key) { - // FIXME: should allow for a retry? let secret_key = find_key_by_pk(&mut wallet, args, public_key)?; used_pubkeys.insert(public_key.clone()); signing_tx_keypairs.push(secret_key); diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 16256deb36..e3d1c3f5a1 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -1020,12 +1020,29 @@ impl Wallet { { match stored_key { StoredKeypair::Encrypted(encrypted) => { - let password = password.unwrap_or_else(|| { - U::read_password(false, Some(&alias.to_string())) - }); - let key = encrypted - .decrypt(password) - .map_err(FindKeyError::KeyDecryptionError)?; + let key = match password { + Some(pwd) => encrypted.decrypt(pwd), + None => { + // Two attempts to get the password right in interactive + // mode + let mut key_result = + Err(keys::DecryptionError::EmptyPassword); + for _ in 0..2 { + let pwd = U::read_password( + false, + Some(&alias.to_string()), + ); + key_result = encrypted.decrypt(pwd); + if key_result.is_ok() { + break; + } + } + + key_result + } + } + .map_err(FindKeyError::KeyDecryptionError)?; + decrypted_key_cache.insert(alias.clone(), key); decrypted_key_cache .get(&alias) From 2113c7c781b5fc88cc70a77efded2ba5e0a24e95 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 17:42:10 +0200 Subject: [PATCH 7/8] Changelog #3681 --- .changelog/unreleased/improvements/3681-client-tx-signature.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3681-client-tx-signature.md diff --git a/.changelog/unreleased/improvements/3681-client-tx-signature.md b/.changelog/unreleased/improvements/3681-client-tx-signature.md new file mode 100644 index 0000000000..63dc064d04 --- /dev/null +++ b/.changelog/unreleased/improvements/3681-client-tx-signature.md @@ -0,0 +1,2 @@ +- Improved the user experience of the secret key decryption process. + ([\#3681](https://github.com/anoma/namada/pull/3681)) \ No newline at end of file From ae2d9496712247b3843adb4d238cdd08ed27f179 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 21 Aug 2024 19:10:58 +0200 Subject: [PATCH 8/8] Updates expected msg in wallet e2e test --- crates/tests/src/e2e/wallet_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/tests/src/e2e/wallet_tests.rs b/crates/tests/src/e2e/wallet_tests.rs index 9ac110008b..8435917202 100644 --- a/crates/tests/src/e2e/wallet_tests.rs +++ b/crates/tests/src/e2e/wallet_tests.rs @@ -67,7 +67,7 @@ fn wallet_encrypted_key_cmds() -> Result<()> { ))?; cmd.exp_string(" Public key hash:")?; cmd.exp_string(" Public key:")?; - cmd.exp_string("Enter your decryption password:")?; + cmd.exp_string("Enter your decryption password")?; cmd.send_line(password)?; // 3. key list