From cd2b0bb2cfbf85a7bebc854fe7a42b55f5563c3d Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Wed, 22 Nov 2023 10:04:31 +0100 Subject: [PATCH 1/5] wip --- src/dfx/src/util/clap/parsers.rs | 58 +++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/dfx/src/util/clap/parsers.rs b/src/dfx/src/util/clap/parsers.rs index 290e3da662..1af8df85f4 100644 --- a/src/dfx/src/util/clap/parsers.rs +++ b/src/dfx/src/util/clap/parsers.rs @@ -1,5 +1,7 @@ use byte_unit::{Byte, ByteUnit}; -use std::path::PathBuf; +use regex::Regex; +use rust_decimal::Decimal; +use std::{path::PathBuf, str::FromStr}; pub fn request_id_parser(v: &str) -> Result { // A valid Request Id starts with `0x` and is a series of 64 hexadecimals. @@ -29,9 +31,43 @@ pub fn memo_parser(memo: &str) -> Result { } pub fn cycle_amount_parser(cycles: &str) -> Result { - cycles - .parse::() - .map_err(|_| "Must be a non negative amount.".to_string()) + fn get_multiplier(input: &str) -> Result { + match input { + "k" | "kc" => Ok(1_000), + "m" | "mc" => Ok(1_000_000), + "b" | "bc" => Ok(1_000_000_000), + "t" | "tc" => Ok(1_000_000_000_000), + other => Err(format!("Unknown amount specifier: '{}'", other)), + } + } + + let input = &cycles.replace("_", "").to_lowercase(); + if let Ok(num) = input.parse::() { + Ok(num) + } else { + let re = Regex::new(r"^(.*?)([a-zA-Z]{1,2})$").unwrap(); + + if let Some(captures) = re.captures(input) { + println!("captures: {:?}", captures); + if let (Some(number), Some(multiplier)) = (captures.get(1), captures.get(2)) { + let multiplier = get_multiplier(multiplier.as_str())?; + let number = Decimal::from_str(number.as_str()) + .map_err(|_| "must specify a decimal amount of cycles.".to_string())?; + let amount = Decimal::from(multiplier) * number; + if amount >= 0.into() { + amount + .try_into() + .map_err(|_| "Too large amount of cycles.".to_string()) + } else { + Err("Must specify a non negative amount of cycles.".to_string()) + } + } else { + Err("Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) + } + } else { + Err("Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) + } + } } pub fn file_parser(path: &str) -> Result { @@ -130,3 +166,17 @@ pub fn hsm_key_id_parser(key_id: &str) -> Result { Ok(key_id.to_string()) } } + +#[test] +fn test_cycle_amount_parser() { + assert_eq!(cycle_amount_parser("10T"), Ok(10_000_000_000_000)); + assert_eq!(cycle_amount_parser("10TC"), Ok(10_000_000_000_000)); + assert_eq!(cycle_amount_parser("0.01b"), Ok(10_000_000)); + assert_eq!(cycle_amount_parser("1.23t"), Ok(1_230_000_000_000)); + assert_eq!(cycle_amount_parser("9_887K"), Ok(9_887_000)); + + assert!(matches!(cycle_amount_parser("1MT"), Err(_))); + assert!(matches!(cycle_amount_parser("-0.1m"), Err(_))); + assert!(matches!(cycle_amount_parser("T100"), Err(_))); + assert!(matches!(cycle_amount_parser("1.1k0"), Err(_))); +} From 8d9e51841a7e4fe721ad30e3d0cd7c4f7c64de1f Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Wed, 22 Nov 2023 13:59:30 +0100 Subject: [PATCH 2/5] feat: accept more ways to specify cycle and e8s amounts --- CHANGELOG.md | 12 +++ src/dfx/src/util/clap/parsers.rs | 126 +++++++++++++++++++------------ 2 files changed, 89 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d45ca3266..071e2d059f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,18 @@ call it as a query call. This resolves a potential security risk. The message "transaction is a duplicate of another transaction in block ...", previously printed to stdout, is now logged to stderr. This means that the output of `dfx ledger transfer` to stdout will contain only "Transfer sent at block height ". +### feat: accept more ways to specify cycle and e8s amounts + +`_` can now be to make large numbers more readable. For example: `dfx canister deposit-cycles 1_234_567 mycanister` + +Certain suffixes that replace a number of zeros are now supported. The (case insensiteve) suffixes are: +- `k` for `000`, e.g. `500k` +- `m` for `000_000`, e.g. `5m` +- `b` for `000_000_000`, e.g. `50B` +- `t` for `000_000_000_000`, e.g. `0.3T` + +For cycles (but not trillion cycles), an additional `c` or `C` is also acceptable. For example: `dfx canister deposit-cycles 3TC mycanister` + ### feat: added `dfx cycles` command This won't work on mainnet yet, but can work locally after installing the cycles ledger. diff --git a/src/dfx/src/util/clap/parsers.rs b/src/dfx/src/util/clap/parsers.rs index 1af8df85f4..0b9f918405 100644 --- a/src/dfx/src/util/clap/parsers.rs +++ b/src/dfx/src/util/clap/parsers.rs @@ -1,8 +1,34 @@ use byte_unit::{Byte, ByteUnit}; -use regex::Regex; use rust_decimal::Decimal; use std::{path::PathBuf, str::FromStr}; +/// Removes `_`, interprets `k`, `m`, `b`, `t` suffix (case insensitive) +fn decimal_with_suffix_parser(input: &str) -> Result { + let input = input.replace("_", "").to_lowercase(); + let (number, suffix) = if input + .chars() + .last() + .map(|char| char.is_alphabetic()) + .unwrap_or(false) + { + input.split_at(input.len() - 1) + } else { + (input.as_str(), "") + }; + let multiplier: u64 = match suffix { + "" => Ok(1), + "k" => Ok(1_000), + "m" => Ok(1_000_000), + "b" => Ok(1_000_000_000), + "t" => Ok(1_000_000_000_000), + other => Err(format!("Unknown amount specifier: '{}'", other)), + }?; + let number = Decimal::from_str(number).map_err(|err| err.to_string())?; + Decimal::from(multiplier) + .checked_mul(number) + .ok_or_else(|| "Amount too large.".to_string()) +} + pub fn request_id_parser(v: &str) -> Result { // A valid Request Id starts with `0x` and is a series of 64 hexadecimals. if !v.starts_with("0x") { @@ -20,8 +46,9 @@ pub fn request_id_parser(v: &str) -> Result { } } -pub fn e8s_parser(e8s: &str) -> Result { - e8s.parse::() +pub fn e8s_parser(input: &str) -> Result { + decimal_with_suffix_parser(input)? + .try_into() .map_err(|_| "Must specify a non negative whole number.".to_string()) } @@ -30,44 +57,14 @@ pub fn memo_parser(memo: &str) -> Result { .map_err(|_| "Must specify a non negative whole number.".to_string()) } -pub fn cycle_amount_parser(cycles: &str) -> Result { - fn get_multiplier(input: &str) -> Result { - match input { - "k" | "kc" => Ok(1_000), - "m" | "mc" => Ok(1_000_000), - "b" | "bc" => Ok(1_000_000_000), - "t" | "tc" => Ok(1_000_000_000_000), - other => Err(format!("Unknown amount specifier: '{}'", other)), - } - } - - let input = &cycles.replace("_", "").to_lowercase(); - if let Ok(num) = input.parse::() { - Ok(num) +pub fn cycle_amount_parser(input: &str) -> Result { + let removed_cycle_suffix = if input.to_lowercase().chars().last() == Some('c') { + &input[..input.len() - 1] } else { - let re = Regex::new(r"^(.*?)([a-zA-Z]{1,2})$").unwrap(); - - if let Some(captures) = re.captures(input) { - println!("captures: {:?}", captures); - if let (Some(number), Some(multiplier)) = (captures.get(1), captures.get(2)) { - let multiplier = get_multiplier(multiplier.as_str())?; - let number = Decimal::from_str(number.as_str()) - .map_err(|_| "must specify a decimal amount of cycles.".to_string())?; - let amount = Decimal::from(multiplier) * number; - if amount >= 0.into() { - amount - .try_into() - .map_err(|_| "Too large amount of cycles.".to_string()) - } else { - Err("Must specify a non negative amount of cycles.".to_string()) - } - } else { - Err("Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) - } - } else { - Err("Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) - } - } + &input[..] + }; + + decimal_with_suffix_parser(removed_cycle_suffix)?.try_into().map_err(|_| "Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) } pub fn file_parser(path: &str) -> Result { @@ -88,9 +85,15 @@ pub fn file_or_stdin_parser(path: &str) -> Result { } } -pub fn trillion_cycle_amount_parser(cycles: &str) -> Result { - format!("{}000000000000", cycles).parse::() - .map_err(|_| "Must be a non negative amount. Currently only accepts whole numbers. Use --cycles otherwise.".to_string()) +pub fn trillion_cycle_amount_parser(input: &str) -> Result { + if let Ok(cycles) = format!("{}000000000000", input.replace("_", "")).parse::() { + Ok(cycles) + } else { + decimal_with_suffix_parser(input)? + .checked_mul(1_000_000_000_000_u64.into()) + .and_then(|total| total.try_into().ok()) + .ok_or_else(|| "Amount too large.".to_string()) + } } pub fn compute_allocation_parser(compute_allocation: &str) -> Result { @@ -169,14 +172,39 @@ pub fn hsm_key_id_parser(key_id: &str) -> Result { #[test] fn test_cycle_amount_parser() { + assert_eq!(cycle_amount_parser("900c"), Ok(900)); + assert_eq!(cycle_amount_parser("9_887K"), Ok(9_887_000)); + assert_eq!(cycle_amount_parser("0.1M"), Ok(100_000)); + assert_eq!(cycle_amount_parser("0.01b"), Ok(10_000_000)); assert_eq!(cycle_amount_parser("10T"), Ok(10_000_000_000_000)); assert_eq!(cycle_amount_parser("10TC"), Ok(10_000_000_000_000)); - assert_eq!(cycle_amount_parser("0.01b"), Ok(10_000_000)); assert_eq!(cycle_amount_parser("1.23t"), Ok(1_230_000_000_000)); - assert_eq!(cycle_amount_parser("9_887K"), Ok(9_887_000)); - assert!(matches!(cycle_amount_parser("1MT"), Err(_))); - assert!(matches!(cycle_amount_parser("-0.1m"), Err(_))); - assert!(matches!(cycle_amount_parser("T100"), Err(_))); - assert!(matches!(cycle_amount_parser("1.1k0"), Err(_))); + assert!(cycle_amount_parser("1ffff").is_err()); + assert!(cycle_amount_parser("1MT").is_err()); + assert!(cycle_amount_parser("-0.1m").is_err()); + assert!(cycle_amount_parser("T100").is_err()); + assert!(cycle_amount_parser("1.1k0").is_err()); + assert!(cycle_amount_parser(&format!("{}0", u128::MAX)).is_err()); +} + +#[test] +fn test_trillion_cycle_amount_parser() { + const TRILLION: u128 = 1_000_000_000_000; + assert_eq!(trillion_cycle_amount_parser("1"), Ok(1 * TRILLION)); + assert_eq!(trillion_cycle_amount_parser("5_555"), Ok(5_555 * TRILLION)); + assert_eq!(trillion_cycle_amount_parser("1k"), Ok(1_000 * TRILLION)); + assert_eq!(trillion_cycle_amount_parser("0.3"), Ok(300_000_000_000)); + assert_eq!(trillion_cycle_amount_parser("0.3k"), Ok(300 * TRILLION)); + + assert!(trillion_cycle_amount_parser("-0.1m").is_err()); + assert!(trillion_cycle_amount_parser("1TC").is_err()); // ambiguous in combination with --t +} + +#[test] +fn test_e8s_parser() { + assert_eq!(e8s_parser("1"), Ok(1)); + assert_eq!(e8s_parser("1_000"), Ok(1_000)); + assert_eq!(e8s_parser("1k"), Ok(1_000)); + assert_eq!(e8s_parser("1M"), Ok(1_000_000)); } From 743e375f5b1a3923d83381147333654227c1dc9b Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Thu, 23 Nov 2023 11:43:54 +0100 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Thomas Locher --- CHANGELOG.md | 4 ++-- src/dfx/src/util/clap/parsers.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79df0c0463..a5a7d2f1ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,9 +38,9 @@ The message "transaction is a duplicate of another transaction in block ...", pr ### feat: accept more ways to specify cycle and e8s amounts -`_` can now be to make large numbers more readable. For example: `dfx canister deposit-cycles 1_234_567 mycanister` +Underscores (`_`) can now be used to make large numbers more readable. For example: `dfx canister deposit-cycles 1_234_567 mycanister` -Certain suffixes that replace a number of zeros are now supported. The (case insensiteve) suffixes are: +Certain suffixes that replace a number of zeros are now supported. The (case-insensitive) suffixes are: - `k` for `000`, e.g. `500k` - `m` for `000_000`, e.g. `5m` - `b` for `000_000_000`, e.g. `50B` diff --git a/src/dfx/src/util/clap/parsers.rs b/src/dfx/src/util/clap/parsers.rs index 0b9f918405..0dbe392116 100644 --- a/src/dfx/src/util/clap/parsers.rs +++ b/src/dfx/src/util/clap/parsers.rs @@ -2,7 +2,7 @@ use byte_unit::{Byte, ByteUnit}; use rust_decimal::Decimal; use std::{path::PathBuf, str::FromStr}; -/// Removes `_`, interprets `k`, `m`, `b`, `t` suffix (case insensitive) +/// Removes `_`, interprets `k`, `m`, `b`, `t` suffix (case-insensitive) fn decimal_with_suffix_parser(input: &str) -> Result { let input = input.replace("_", "").to_lowercase(); let (number, suffix) = if input @@ -49,7 +49,7 @@ pub fn request_id_parser(v: &str) -> Result { pub fn e8s_parser(input: &str) -> Result { decimal_with_suffix_parser(input)? .try_into() - .map_err(|_| "Must specify a non negative whole number.".to_string()) + .map_err(|_| "Must specify a non-negative whole number.".to_string()) } pub fn memo_parser(memo: &str) -> Result { From 206abdf496aae4a56936a00c34ea7565facf433d Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Thu, 23 Nov 2023 11:55:54 +0100 Subject: [PATCH 4/5] clippy --- src/dfx/src/util/clap/parsers.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/dfx/src/util/clap/parsers.rs b/src/dfx/src/util/clap/parsers.rs index 0dbe392116..9b7184dffc 100644 --- a/src/dfx/src/util/clap/parsers.rs +++ b/src/dfx/src/util/clap/parsers.rs @@ -4,7 +4,7 @@ use std::{path::PathBuf, str::FromStr}; /// Removes `_`, interprets `k`, `m`, `b`, `t` suffix (case-insensitive) fn decimal_with_suffix_parser(input: &str) -> Result { - let input = input.replace("_", "").to_lowercase(); + let input = input.replace('_', "").to_lowercase(); let (number, suffix) = if input .chars() .last() @@ -58,10 +58,10 @@ pub fn memo_parser(memo: &str) -> Result { } pub fn cycle_amount_parser(input: &str) -> Result { - let removed_cycle_suffix = if input.to_lowercase().chars().last() == Some('c') { + let removed_cycle_suffix = if input.to_lowercase().ends_with('c') { &input[..input.len() - 1] } else { - &input[..] + input }; decimal_with_suffix_parser(removed_cycle_suffix)?.try_into().map_err(|_| "Failed to parse amount. Please use digits only or something like 3.5TC, 2t, or 5_000_000.".to_string()) @@ -86,7 +86,7 @@ pub fn file_or_stdin_parser(path: &str) -> Result { } pub fn trillion_cycle_amount_parser(input: &str) -> Result { - if let Ok(cycles) = format!("{}000000000000", input.replace("_", "")).parse::() { + if let Ok(cycles) = format!("{}000000000000", input.replace('_', "")).parse::() { Ok(cycles) } else { decimal_with_suffix_parser(input)? @@ -191,7 +191,7 @@ fn test_cycle_amount_parser() { #[test] fn test_trillion_cycle_amount_parser() { const TRILLION: u128 = 1_000_000_000_000; - assert_eq!(trillion_cycle_amount_parser("1"), Ok(1 * TRILLION)); + assert_eq!(trillion_cycle_amount_parser("3"), Ok(3 * TRILLION)); assert_eq!(trillion_cycle_amount_parser("5_555"), Ok(5_555 * TRILLION)); assert_eq!(trillion_cycle_amount_parser("1k"), Ok(1_000 * TRILLION)); assert_eq!(trillion_cycle_amount_parser("0.3"), Ok(300_000_000_000)); From ed01157720e4d9c0ca22456acbff674c18233c47 Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Thu, 23 Nov 2023 16:35:31 +0100 Subject: [PATCH 5/5] clarify changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5a7d2f1ae..9fc52fba8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ Certain suffixes that replace a number of zeros are now supported. The (case-ins - `b` for `000_000_000`, e.g. `50B` - `t` for `000_000_000_000`, e.g. `0.3T` -For cycles (but not trillion cycles), an additional `c` or `C` is also acceptable. For example: `dfx canister deposit-cycles 3TC mycanister` +For cycles an additional `c` or `C` is also acceptable. For example: `dfx canister deposit-cycles 3TC mycanister` ### feat: added `dfx cycles` command