From b3d9a267ea903d331d15e4834689754248abefe7 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 1 Apr 2023 12:32:15 +0200 Subject: [PATCH 1/3] Add a few more amount parsing tests These tests try to stress various edge cases that should return `ParseAmountError::TooPrecise`. --- bitcoin/src/amount.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bitcoin/src/amount.rs b/bitcoin/src/amount.rs index 5f3b285b..d513feb4 100644 --- a/bitcoin/src/amount.rs +++ b/bitcoin/src/amount.rs @@ -1642,6 +1642,7 @@ mod tests { use super::ParseAmountError as E; let btc = Denomination::Bitcoin; let sat = Denomination::Satoshi; + let msat = Denomination::MilliSatoshi; let p = Amount::from_str_in; let sp = SignedAmount::from_str_in; @@ -1654,6 +1655,15 @@ mod tests { let more_than_max = format!("1{}", Amount::max_value()); assert_eq!(p(&more_than_max, btc), Err(E::TooBig)); assert_eq!(p("0.000000042", btc), Err(E::TooPrecise)); + assert_eq!(p("999.0000000", msat), Err(E::TooPrecise)); + assert_eq!(p("1.0000000", msat), Err(E::TooPrecise)); + assert_eq!(p("1.1", msat), Err(E::TooPrecise)); + assert_eq!(p("1000.1", msat), Err(E::TooPrecise)); + assert_eq!(p("1001.0000000", msat), Err(E::TooPrecise)); + assert_eq!(p("1000.0000001", msat), Err(E::TooPrecise)); + assert_eq!(p("1000.1000000", msat), Err(E::TooPrecise)); + assert_eq!(p("1100.0000000", msat), Err(E::TooPrecise)); + assert_eq!(p("10001.0000000", msat), Err(E::TooPrecise)); assert_eq!(p("1", btc), Ok(Amount::from_sat(1_000_000_00))); assert_eq!(sp("-.5", btc), Ok(SignedAmount::from_sat(-500_000_00))); From f1a3dc67197990865f4c2c5d18e8ecbb645f63e7 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sun, 7 May 2023 08:45:25 +0200 Subject: [PATCH 2/3] Allow parsing sub-sat denoms with decimal points Numbers with only zeros after decimal points are valid if they are also multiples of `10^precision` (e.g. 1000 for msats). These were artificially disallowed as "too precise" which was at least misleading. This change allows parsing such numbers. --- bitcoin/src/amount.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/bitcoin/src/amount.rs b/bitcoin/src/amount.rs index d513feb4..80dc06ea 100644 --- a/bitcoin/src/amount.rs +++ b/bitcoin/src/amount.rs @@ -199,7 +199,13 @@ impl std::error::Error for ParseAmountError { } fn is_too_precise(s: &str, precision: usize) -> bool { - s.contains('.') || precision >= s.len() || s.chars().rev().take(precision).any(|d| d != '0') + match s.find('.') { + Some(pos) => + s[(pos + 1)..].chars().any(|d| d != '0') + || precision >= pos + || s[..pos].chars().rev().take(precision).any(|d| d != '0'), + None => precision >= s.len() || s.chars().rev().take(precision).any(|d| d != '0'), + } } /// Parse decimal string in the given denomination into a satoshi value and a @@ -227,7 +233,7 @@ fn parse_signed_to_satoshi( // The difference in precision between native (satoshi) // and desired denomination. let precision_diff = -denom.precision(); - if precision_diff < 0 { + if precision_diff <= 0 { // If precision diff is negative, this means we are parsing // into a less precise amount. That is not allowed unless // there are no decimals and the last digits are zeroes as @@ -239,7 +245,7 @@ fn parse_signed_to_satoshi( _ => return Err(ParseAmountError::TooPrecise), } } - s = &s[0..s.len() - last_n]; + s = &s[0..s.find('.').unwrap_or(s.len()) - last_n]; 0 } else { precision_diff @@ -267,6 +273,7 @@ fn parse_signed_to_satoshi( }; } '.' => match decimals { + None if max_decimals <= 0 => break, None => decimals = Some(0), // Double decimal dot. _ => return Err(ParseAmountError::InvalidFormat), From 6c6a89b1d13fa2a5200318c943404d2bf673d94a Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sun, 7 May 2023 08:47:53 +0200 Subject: [PATCH 3/3] Add sub-sat fractions parsing regression test This test triggers the bug fixed in previous commit. --- bitcoin/src/amount.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bitcoin/src/amount.rs b/bitcoin/src/amount.rs index 80dc06ea..4bb62d92 100644 --- a/bitcoin/src/amount.rs +++ b/bitcoin/src/amount.rs @@ -1684,6 +1684,8 @@ mod tests { p("12345678901.12345678", btc), Ok(Amount::from_sat(12_345_678_901__123_456_78)) ); + assert_eq!(p("1000.0", msat), Ok(Amount::from_sat(1))); + assert_eq!(p("1000.000000000000000000000000000", msat), Ok(Amount::from_sat(1))); // make sure satoshi > i64::max_value() is checked. let amount = Amount::from_sat(i64::max_value() as u64);