From e80de8b1ee8d611aef378484b405cdd2c818870c Mon Sep 17 00:00:00 2001 From: KaFai Choi Date: Mon, 10 Jan 2022 21:30:28 +0700 Subject: [PATCH 1/2] add nano and pico BTC to Donomination enum --- src/util/amount.rs | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index c628e742..c51a022d 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -28,6 +28,10 @@ pub enum Denomination { MilliBitcoin, /// uBTC MicroBitcoin, + /// nBTC + NanoBitcoin, + /// pBTC + PicoBitcoin, /// bits Bit, /// satoshi @@ -43,6 +47,8 @@ impl Denomination { Denomination::Bitcoin => -8, Denomination::MilliBitcoin => -5, Denomination::MicroBitcoin => -2, + Denomination::NanoBitcoin => 1, + Denomination::PicoBitcoin => 4, Denomination::Bit => -2, Denomination::Satoshi => 0, Denomination::MilliSatoshi => 3, @@ -56,6 +62,8 @@ impl fmt::Display for Denomination { Denomination::Bitcoin => "BTC", Denomination::MilliBitcoin => "mBTC", Denomination::MicroBitcoin => "uBTC", + Denomination::NanoBitcoin => "nBTC", + Denomination::PicoBitcoin => "pBTC", Denomination::Bit => "bits", Denomination::Satoshi => "satoshi", Denomination::MilliSatoshi => "msat", @@ -68,15 +76,15 @@ impl FromStr for Denomination { /// Convert from a str to Denomination. /// - /// Any combination of upper and/or lower case, excluding uppercase 'M' is considered valid. - /// - Singular: BTC, mBTC, uBTC + /// Any combination of upper and/or lower case, excluding uppercase 'M', 'P' is considered valid. + /// - Singular: BTC, mBTC, uBTC, nBTC, pBTC /// - Plural or singular: sat, satoshi, bit, msat /// /// Due to ambiguity between mega and milli we prohibit usage of leading capital 'M'. fn from_str(s: &str) -> Result { use self::ParseAmountError::*; - if s.starts_with('M') { + if s.starts_with(|ch| ch == 'M' || ch == 'P') { return Err(denomination_from_str(s).map_or_else( || UnknownDenomination(s.to_owned()), |_| PossiblyConfusingDenomination(s.to_owned()) @@ -100,6 +108,14 @@ fn denomination_from_str(mut s: &str) -> Option { return Some(Denomination::MicroBitcoin); } + if s.eq_ignore_ascii_case("nBTC") { + return Some(Denomination::NanoBitcoin); + } + + if s.eq_ignore_ascii_case("pBTC") { + return Some(Denomination::PicoBitcoin); + } + if s.ends_with('s') || s.ends_with('S') { s = &s[..(s.len() - 1)]; } @@ -1439,6 +1455,12 @@ mod tests { assert_eq!("-5", SignedAmount::from_sat(-5).to_string_in(D::Satoshi)); assert_eq!("0.10000000", Amount::from_sat(100_000_00).to_string_in(D::Bitcoin)); assert_eq!("-100.00", SignedAmount::from_sat(-10_000).to_string_in(D::Bit)); + assert_eq!("2535830", Amount::from_sat(253583).to_string_in(D::NanoBitcoin)); + assert_eq!("-100000", SignedAmount::from_sat(-10_000).to_string_in(D::NanoBitcoin)); + assert_eq!("2535830000", Amount::from_sat(253583).to_string_in(D::PicoBitcoin)); + assert_eq!("-100000000", SignedAmount::from_sat(-10_000).to_string_in(D::PicoBitcoin)); + + assert_eq!(ua_str(&ua_sat(0).to_string_in(D::Satoshi), D::Satoshi), Ok(ua_sat(0))); assert_eq!(ua_str(&ua_sat(500).to_string_in(D::Bitcoin), D::Bitcoin), Ok(ua_sat(500))); @@ -1453,6 +1475,15 @@ mod tests { // Test an overflow bug in `abs()` assert_eq!(sa_str(&sa_sat(i64::min_value()).to_string_in(D::Satoshi), D::MicroBitcoin), Err(ParseAmountError::TooBig)); + assert_eq!(sa_str(&sa_sat(-1).to_string_in(D::NanoBitcoin), D::NanoBitcoin), Ok(sa_sat(-1))); + assert_eq!(sa_str(&sa_sat(i64::max_value()).to_string_in(D::Satoshi), D::NanoBitcoin), Err(ParseAmountError::TooPrecise)); + assert_eq!(sa_str(&sa_sat(i64::min_value()).to_string_in(D::Satoshi), D::NanoBitcoin), Err(ParseAmountError::TooPrecise)); + + assert_eq!(sa_str(&sa_sat(-1).to_string_in(D::PicoBitcoin), D::PicoBitcoin), Ok(sa_sat(-1))); + assert_eq!(sa_str(&sa_sat(i64::max_value()).to_string_in(D::Satoshi), D::PicoBitcoin), Err(ParseAmountError::TooPrecise)); + assert_eq!(sa_str(&sa_sat(i64::min_value()).to_string_in(D::Satoshi), D::PicoBitcoin), Err(ParseAmountError::TooPrecise)); + + } #[test] @@ -1465,7 +1496,9 @@ mod tests { assert_eq!(Amount::from_str(&denom(amt, D::MicroBitcoin)), Ok(amt)); assert_eq!(Amount::from_str(&denom(amt, D::Bit)), Ok(amt)); assert_eq!(Amount::from_str(&denom(amt, D::Satoshi)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::NanoBitcoin)), Ok(amt)); assert_eq!(Amount::from_str(&denom(amt, D::MilliSatoshi)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::PicoBitcoin)), Ok(amt)); assert_eq!(Amount::from_str("42 satoshi BTC"), Err(ParseAmountError::InvalidFormat)); assert_eq!(SignedAmount::from_str("-42 satoshi BTC"), Err(ParseAmountError::InvalidFormat)); @@ -1693,7 +1726,7 @@ mod tests { #[test] fn denomination_string_acceptable_forms() { // Non-exhaustive list of valid forms. - let valid = vec!["BTC", "btc", "mBTC", "mbtc", "uBTC", "ubtc", "SATOSHI","Satoshi", "Satoshis", "satoshis", "SAT", "Sat", "sats", "bit", "bits"]; + let valid = vec!["BTC", "btc", "mBTC", "mbtc", "uBTC", "ubtc", "SATOSHI","Satoshi", "Satoshis", "satoshis", "SAT", "Sat", "sats", "bit", "bits", "nBTC", "pBTC"]; for denom in valid.iter() { assert!(Denomination::from_str(denom).is_ok()); } @@ -1702,7 +1735,7 @@ mod tests { #[test] fn disallow_confusing_forms() { // Non-exhaustive list of confusing forms. - let confusing = vec!["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MBTC", "Mbtc"]; + let confusing = vec!["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MBTC", "Mbtc", "PBTC"]; for denom in confusing.iter() { match Denomination::from_str(denom) { Ok(_) => panic!("from_str should error for {}", denom), From 40f38b3edc9bcfe95231125f854efaca55641987 Mon Sep 17 00:00:00 2001 From: KaFai Choi Date: Tue, 11 Jan 2022 21:49:58 +0700 Subject: [PATCH 2/2] enforce strict SI(treat capital of m, u, n, p as invalid) in parsing amount denomiation. add disallow_unknown_denomination test --- src/util/amount.rs | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index c51a022d..0992d843 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -76,22 +76,26 @@ impl FromStr for Denomination { /// Convert from a str to Denomination. /// - /// Any combination of upper and/or lower case, excluding uppercase 'M', 'P' is considered valid. + /// Any combination of upper and/or lower case, excluding uppercase of SI(m, u, n, p) is considered valid. /// - Singular: BTC, mBTC, uBTC, nBTC, pBTC /// - Plural or singular: sat, satoshi, bit, msat /// - /// Due to ambiguity between mega and milli we prohibit usage of leading capital 'M'. + /// Due to ambiguity between mega and milli, pico and peta we prohibit usage of leading capital 'M', 'P'. fn from_str(s: &str) -> Result { use self::ParseAmountError::*; + use self::Denomination as D; - if s.starts_with(|ch| ch == 'M' || ch == 'P') { - return Err(denomination_from_str(s).map_or_else( - || UnknownDenomination(s.to_owned()), - |_| PossiblyConfusingDenomination(s.to_owned()) - )); + let starts_with_uppercase = || s.starts_with(|ch: char| ch.is_uppercase()); + match denomination_from_str(s) { + None => Err(UnknownDenomination(s.to_owned())), + Some(D::MilliBitcoin) | Some(D::PicoBitcoin) | Some(D::MilliSatoshi) if starts_with_uppercase() => { + Err(PossiblyConfusingDenomination(s.to_owned())) + } + Some(D::NanoBitcoin) | Some(D::MicroBitcoin) if starts_with_uppercase() => { + Err(UnknownDenomination(s.to_owned())) + } + Some(d) => Ok(d), } - - denomination_from_str(s).ok_or_else(|| UnknownDenomination(s.to_owned())) } } @@ -169,7 +173,13 @@ impl fmt::Display for ParseAmountError { ParseAmountError::InvalidCharacter(c) => write!(f, "invalid character in input: {}", c), ParseAmountError::UnknownDenomination(ref d) => write!(f, "unknown denomination: {}", d), ParseAmountError::PossiblyConfusingDenomination(ref d) => { - write!(f, "the 'M' at the beginning of {} should technically mean 'Mega' but that denomination is uncommon and maybe 'milli' was intended", d) + let (letter, upper, lower) = match d.chars().next() { + Some('M') => ('M', "Mega", "milli"), + Some('P') => ('P',"Peta", "pico"), + // This panic could be avoided by adding enum ConfusingDenomination { Mega, Peta } but is it worth it? + _ => panic!("invalid error information"), + }; + write!(f, "the '{}' at the beginning of {} should technically mean '{}' but that denomination is uncommon and maybe '{}' was intended", letter, d, upper, lower) } } } @@ -1744,5 +1754,18 @@ mod tests { } } } + + #[test] + fn disallow_unknown_denomination() { + // Non-exhaustive list of unknown forms. + let unknown = vec!["NBTC", "UBTC", "ABC", "abc"]; + for denom in unknown.iter() { + match Denomination::from_str(denom) { + Ok(_) => panic!("from_str should error for {}", denom), + Err(ParseAmountError::UnknownDenomination(_)) => {}, + Err(e) => panic!("unexpected error: {}", e), + } + } + } }