diff --git a/clap-utils/src/input_parsers.rs b/clap-utils/src/input_parsers.rs index 203ebb5060..47b9f8de3f 100644 --- a/clap-utils/src/input_parsers.rs +++ b/clap-utils/src/input_parsers.rs @@ -216,11 +216,18 @@ mod tests { .clone() .get_matches_from(vec!["test", "--single", "50", "--unit", "lamports"]); assert_eq!(amount_of(&matches, "single", "unit"), Some(50)); + assert_eq!(amount_of(&matches, "multiple", "unit"), None); let matches = app() .clone() .get_matches_from(vec!["test", "--single", "50", "--unit", "SOL"]); assert_eq!(amount_of(&matches, "single", "unit"), Some(50000000000)); - assert_eq!(amount_of(&matches, "multiple", "unit"), None); - assert_eq!(amount_of(&matches, "multiple", "unit"), None); + let matches = app() + .clone() + .get_matches_from(vec!["test", "--single", "1.5", "--unit", "SOL"]); + assert_eq!(amount_of(&matches, "single", "unit"), Some(1500000000)); + let matches = app() + .clone() + .get_matches_from(vec!["test", "--single", "1.5", "--unit", "lamports"]); + assert_eq!(amount_of(&matches, "single", "unit"), None); } } diff --git a/clap-utils/src/input_validators.rs b/clap-utils/src/input_validators.rs index 8121e06e56..2b29a6c323 100644 --- a/clap-utils/src/input_validators.rs +++ b/clap-utils/src/input_validators.rs @@ -120,8 +120,12 @@ pub fn is_valid_percentage(percentage: String) -> Result<(), String> { } pub fn is_amount(amount: String) -> Result<(), String> { - amount - .parse::() - .map(|_| ()) - .map_err(|e| format!("{:?}", e)) + if amount.parse::().is_ok() || amount.parse::().is_ok() { + Ok(()) + } else { + Err(format!( + "Unable to parse input amount as integer or float, provided: {}", + amount + )) + } } diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 064d193e98..a2b19cfe67 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -406,7 +406,7 @@ pub fn parse_command(matches: &ArgMatches<'_>) -> Result) -> Result { - let lamports = amount_of(matches, "amount", "unit").expect("Invalid amount"); + let lamports = required_lamports_from(matches, "amount", "unit")?; let to = pubkey_of(&matches, "to").unwrap(); let timestamp = if matches.is_present("timestamp") { // Parse input for serde_json @@ -1449,6 +1449,22 @@ where } } +// If clap arg `name` is_required, and specifies an amount of either lamports or SOL, the only way +// `amount_of()` can return None is if `name` is an f64 and `unit`== "lamports". This method +// catches that case and converts it to an Error. +pub(crate) fn required_lamports_from( + matches: &ArgMatches<'_>, + name: &str, + unit: &str, +) -> Result { + amount_of(matches, name, unit).ok_or_else(|| { + CliError::BadParameter(format!( + "Lamports cannot be fractional: {}", + matches.value_of("amount").unwrap() + )) + }) +} + pub(crate) fn build_balance_message( lamports: u64, use_lamports_unit: bool, @@ -1516,6 +1532,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' .index(1) .value_name("AMOUNT") .takes_value(true) + .validator(is_amount) .required(true) .help("The airdrop amount to request (default unit SOL)"), ) @@ -1588,6 +1605,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' .index(2) .value_name("AMOUNT") .takes_value(true) + .validator(is_amount) .required(true) .help("The amount to send (default unit SOL)"), ) @@ -1759,14 +1777,6 @@ mod tests { path } - #[test] - #[should_panic] - fn test_bad_amount() { - let test_commands = app("test", "desc", "version"); - let test_bad_airdrop = test_commands.get_matches_from(vec!["test", "airdrop", "notint"]); - let _ignored = parse_command(&test_bad_airdrop).unwrap(); - } - #[test] fn test_cli_parse_command() { let test_commands = app("test", "desc", "version"); diff --git a/cli/src/cluster_query.rs b/cli/src/cluster_query.rs index 0872db0f40..7ef5c4bef6 100644 --- a/cli/src/cluster_query.rs +++ b/cli/src/cluster_query.rs @@ -127,6 +127,7 @@ impl ClusterQuerySubCommands for App<'_, '_> { .value_name("NUMBER") .takes_value(true) .default_value("1") + .validator(is_amount) .help("Number of lamports to transfer for each transaction"), ) .arg( diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index fea194aaa1..1418af1ff0 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -1,6 +1,7 @@ use crate::cli::{ build_balance_message, check_account_for_fee, check_unique_pubkeys, - log_instruction_custom_error, CliCommand, CliCommandInfo, CliConfig, CliError, ProcessResult, + log_instruction_custom_error, required_lamports_from, CliCommand, CliCommandInfo, CliConfig, + CliError, ProcessResult, }; use clap::{App, Arg, ArgMatches, SubCommand}; use solana_clap_utils::{input_parsers::*, input_validators::*}; @@ -142,7 +143,7 @@ impl NonceSubCommands for App<'_, '_> { pub fn parse_nonce_create_account(matches: &ArgMatches<'_>) -> Result { let nonce_account = keypair_of(matches, "nonce_account_keypair").unwrap(); - let lamports = amount_of(matches, "amount", "unit").unwrap(); + let lamports = required_lamports_from(matches, "amount", "unit")?; Ok(CliCommandInfo { command: CliCommand::CreateNonceAccount { @@ -189,7 +190,7 @@ pub fn parse_withdraw_from_nonce_account( ) -> Result { let nonce_account = keypair_of(matches, "nonce_account_keypair").unwrap(); let destination_account_pubkey = pubkey_of(matches, "destination_account_pubkey").unwrap(); - let lamports = amount_of(matches, "amount", "unit").unwrap(); + let lamports = required_lamports_from(matches, "amount", "unit")?; Ok(CliCommandInfo { command: CliCommand::WithdrawFromNonceAccount { diff --git a/cli/src/stake.rs b/cli/src/stake.rs index 4eac69351d..bdb686b0ce 100644 --- a/cli/src/stake.rs +++ b/cli/src/stake.rs @@ -1,7 +1,8 @@ use crate::cli::{ build_balance_message, check_account_for_fee, check_unique_pubkeys, - get_blockhash_fee_calculator, log_instruction_custom_error, replace_signatures, return_signers, - CliCommand, CliCommandInfo, CliConfig, CliError, ProcessResult, + get_blockhash_fee_calculator, log_instruction_custom_error, replace_signatures, + required_lamports_from, return_signers, CliCommand, CliCommandInfo, CliConfig, CliError, + ProcessResult, }; use clap::{App, Arg, ArgMatches, SubCommand}; use console::style; @@ -51,6 +52,7 @@ impl StakeSubCommands for App<'_, '_> { .index(2) .value_name("AMOUNT") .takes_value(true) + .validator(is_amount) .required(true) .help("The amount of send to the vote account (default unit SOL)") ) @@ -251,6 +253,7 @@ impl StakeSubCommands for App<'_, '_> { .index(3) .value_name("AMOUNT") .takes_value(true) + .validator(is_amount) .required(true) .help("The amount to withdraw from the stake account (default unit SOL)") ) @@ -323,7 +326,7 @@ pub fn parse_stake_create_account(matches: &ArgMatches<'_>) -> Result) -> Result) -> Result { let stake_account_pubkey = pubkey_of(matches, "stake_account_pubkey").unwrap(); let destination_account_pubkey = pubkey_of(matches, "destination_account_pubkey").unwrap(); - let lamports = amount_of(matches, "amount", "unit").expect("Invalid amount"); + let lamports = required_lamports_from(matches, "amount", "unit")?; Ok(CliCommandInfo { command: CliCommand::WithdrawStake(