Skip to content

Commit

Permalink
Improve UX on password's retrying scenarios
Browse files Browse the repository at this point in the history
• Change program behavior to quit if wrong seed phrase is given [#49]
• Change program behavior to have three attempts for entering a password [#46]

Resolves #46, #49
  • Loading branch information
sophielouisefeith committed Sep 27, 2022
1 parent 9d90912 commit b3aab8d
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 40 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove `Command::NotSupported` [#57]
- Rename `DEFAULT_GAS_LIMIT`, `DEFAULT_GAS_PRICE`, `MIN_GAS_LIMIT`
- Remove `Addresses` type in favour of `Vec<Address>`
- Remove `refund-addr` arg in `withdraw` command [#86]

### Fixed

Expand Down Expand Up @@ -297,6 +298,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Implementation of `Store` trait from `wallet-core`
- Implementation of `State` and `Prover` traits from `wallet-core`

[#86]: https://github.com/dusk-network/wallet-cli/issues/86
[#84]: https://github.com/dusk-network/wallet-cli/issues/84
[#72]: https://github.com/dusk-network/wallet-cli/issues/72
[#57]: https://github.com/dusk-network/wallet-cli/issues/57
Expand Down
11 changes: 3 additions & 8 deletions src/bin/command.rs
Expand Up @@ -17,6 +17,7 @@ use dusk_wallet::{Address, Dusk, Lux, Wallet};
use dusk_wallet_core::{BalanceInfo, StakeInfo};

/// Commands that can be run against the Dusk wallet
#[allow(clippy::large_enum_variant)]
#[derive(PartialEq, Eq, Hash, Clone, Subcommand, Debug)]
pub(crate) enum Command {
/// Create a new wallet
Expand Down Expand Up @@ -121,14 +122,10 @@ pub(crate) enum Command {

/// Withdraw accumulated reward for a stake key
Withdraw {
/// Address to pay transaction costs
/// Address from which your DUSK was staked
#[clap(short, long)]
addr: Option<Address>,

/// Address to which the reward will be sent to
#[clap(short, long)]
refund_addr: Address,

/// Max amt of gas for this transaction
#[clap(short = 'l', long)]
gas_limit: Option<u64>,
Expand Down Expand Up @@ -241,7 +238,6 @@ impl Command {
}
Command::Withdraw {
addr,
refund_addr,
gas_limit,
gas_price,
} => {
Expand All @@ -254,8 +250,7 @@ impl Command {
gas.set_price(gas_price);
gas.set_limit(gas_limit);

let tx =
wallet.withdraw_reward(addr, &refund_addr, gas).await?;
let tx = wallet.withdraw_reward(addr, gas).await?;
Ok(RunResult::Tx(tx.hash()))
}
Command::Export { addr, dir } => {
Expand Down
31 changes: 22 additions & 9 deletions src/bin/interactive.rs
Expand Up @@ -15,6 +15,7 @@ use crate::settings::Settings;
use crate::Menu;
use crate::WalletFile;
use crate::{Command, RunResult};
use dusk_wallet::Error;
use std::path::PathBuf;

/// Run the interactive UX loop with a loaded wallet
Expand Down Expand Up @@ -202,7 +203,6 @@ fn menu_op(addr: Address, balance: Dusk, settings: &Settings) -> AddrOp {
})),
CMI::Withdraw => AddrOp::Run(Box::new(Command::Withdraw {
addr: Some(addr),
refund_addr: prompt::request_rcvr_addr("refund"),
gas_limit: Some(prompt::request_gas_limit()),
gas_price: Some(prompt::request_gas_price()),
})),
Expand Down Expand Up @@ -258,11 +258,26 @@ pub(crate) fn load_wallet(
// display main menu
let wallet = match menu_wallet(wallet_found) {
MainMenu::Load(path) => {
let pwd = prompt::request_auth(
"Please enter you wallet's password",
password,
);
Wallet::from_file(WalletFile { path, pwd })?
let mut attempt = 1;
loop {
let pwd = prompt::request_auth(
"Please enter you wallet's password",
password,
);
match Wallet::from_file(WalletFile {
path: path.clone(),
pwd,
}) {
Ok(wallet) => break wallet,
Err(_err) if attempt > 2 => {
return Err(Error::AttemptsExhausted)?;
}
Err(_) => {
println!("Invalid password please try again");
attempt += 1;
}
}
}
}
MainMenu::Create => {
// create a new randomly generated mnemonic phrase
Expand All @@ -280,7 +295,7 @@ pub(crate) fn load_wallet(
}
MainMenu::Recover => {
// ask user for 12-word recovery phrase
let phrase = prompt::request_recovery_phrase();
let phrase = prompt::request_recovery_phrase()?;
// ask user for a password to secure the wallet
let pwd = prompt::create_password(&None);
// create and store the recovered wallet
Expand Down Expand Up @@ -392,7 +407,6 @@ fn confirm(cmd: &Command) -> bool {
}
Command::Withdraw {
addr,
refund_addr,
gas_limit,
gas_price,
} => {
Expand All @@ -401,7 +415,6 @@ fn confirm(cmd: &Command) -> bool {
let gas_price = gas_price.expect("gas price not set");
let max_fee = gas_limit * gas_price;
println!(" > Reward from {}", addr.preview());
println!(" > Refund into {}", refund_addr.preview());
println!(" > Max fee = {} DUSK", Dusk::from(max_fee));
prompt::ask_confirm()
}
Expand Down
44 changes: 26 additions & 18 deletions src/bin/io/prompt.rs
Expand Up @@ -14,8 +14,9 @@ use crossterm::{
};

use anyhow::Result;
use bip39::{Language, Mnemonic};
use bip39::{ErrorKind, Language, Mnemonic};
use blake3::Hash;
use dusk_wallet::Error;
use requestty::Question;

use dusk_wallet::{Address, Dusk, Lux};
Expand Down Expand Up @@ -112,25 +113,32 @@ where
}

/// Request the user to input the recovery phrase
pub(crate) fn request_recovery_phrase() -> String {
pub(crate) fn request_recovery_phrase() -> anyhow::Result<String> {
// let the user input the recovery phrase
let q = Question::input("phrase")
.message("Please enter the recovery phrase:")
.validate_on_key(|phrase, _| {
Mnemonic::from_phrase(phrase, Language::English).is_ok()
})
.validate(|phrase, _| {
if Mnemonic::from_phrase(phrase, Language::English).is_ok() {
Ok(())
} else {
Err("Please enter a valid recovery phrase".to_string())
}
})
.build();
let mut attempt = 1;
loop {
let q = Question::input("phrase")
.message("Please enter the recovery phrase:")
.build();

let a = requestty::prompt_one(q).expect("recovery phrase");
let phrase = a.as_string().unwrap_or_default();

let a = requestty::prompt_one(q).expect("recovery phrase");
let phrase = a.as_string().unwrap().to_string();
phrase
match Mnemonic::from_phrase(phrase, Language::English) {
Ok(phrase) => break Ok(phrase.to_string()),

Err(err) if attempt > 2 => match err.downcast_ref::<ErrorKind>() {
Some(ErrorKind::InvalidWord) => {
return Err(Error::AttemptsExhausted)?
}
_ => return Err(err),
},
Err(_) => {
println!("Invalid recovery phrase please try again");
attempt += 1;
}
}
}
}

fn is_valid_dir(dir: &str) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion src/bin/main.rs
Expand Up @@ -207,7 +207,7 @@ async fn exec() -> anyhow::Result<()> {
}
None => {
// ask user for 12-word recovery phrase
let phrase = prompt::request_recovery_phrase();
let phrase = prompt::request_recovery_phrase()?;
// ask user for a password to secure the wallet
let pwd = prompt::create_password(password);
// create wallet
Expand Down
7 changes: 5 additions & 2 deletions src/error.rs
Expand Up @@ -119,8 +119,11 @@ pub enum Error {
#[error("Wallet file is missing")]
WalletFileMissing,
/// Wrong wallet password
#[error("Wrong password")]
InvalidPassword(#[from] block_modes::BlockModeError),
#[error("Block Mode Error")]
BlockMode(#[from] block_modes::BlockModeError),
/// Reached the maximum number of attempts
#[error("Reached the maximum number of attempts")]
AttemptsExhausted,
/// Socket connection is not available on Windows
#[error("Socket connection to {0} is not available on Windows")]
SocketsNotSupported(String),
Expand Down
3 changes: 1 addition & 2 deletions src/wallet.rs
Expand Up @@ -475,7 +475,6 @@ impl<F: SecureWalletFile + Debug> Wallet<F> {
pub async fn withdraw_reward(
&self,
addr: &Address,
refund_addr: &Address,
gas: Gas,
) -> Result<Transaction, Error> {
if let Some(wallet) = &self.wallet {
Expand All @@ -491,7 +490,7 @@ impl<F: SecureWalletFile + Debug> Wallet<F> {
&mut rng,
index,
index,
refund_addr.psk(),
addr.psk(),
gas.limit,
gas.price,
)?;
Expand Down

0 comments on commit b3aab8d

Please sign in to comment.