From b7689a7d6004e1b2d4bb0b6e83d0c27a91fd894f Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 19 Feb 2024 13:33:40 +0100 Subject: [PATCH 1/4] Split up `ParseAmountError::InvalidFormat` The `InvalidFormat` variant was pretty bad: it didn't make it clear what exactly is wrong with the input string, especially since it was used when the denomination was missing. Not only was this unhelpful to the users who don't know they have to write the denomination when the amount was otherwise correct but it was also attributed to a problem with the amount rather than a problem with denomination. To fix this the variant is replaced by `MissingDigitsError`, `MissingError` and `InvalidCharError` - all the cases `InvalidFormat` was originally used in. `InvalidCharError` is effectively the same as the existing variant but it was made into a separate error to enable special casing `.` and make extensions possible. Further this opportunity was used to add a special case for `-` as well. `MissingDigitsError` currently contains special casing for empty string and a string only containing minus sign. It's currently unclear if it's useful so this change makes this distinction private and only makes it affect error messages. As opposed to the previous two variants, `MissingDenominationError` was added to `ParseError`. The struct is currenly empty and may be extended in the future with e.g. span information. --- units/src/amount.rs | 129 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 104 insertions(+), 25 deletions(-) diff --git a/units/src/amount.rs b/units/src/amount.rs index 3e83f4c6..61b20467 100644 --- a/units/src/amount.rs +++ b/units/src/amount.rs @@ -156,6 +156,9 @@ pub enum ParseError { /// Invalid denomination. Denomination(ParseDenominationError), + + /// The denomination was not identified. + MissingDenomination(MissingDenominationError), } impl From for ParseError { @@ -170,11 +173,22 @@ impl From for ParseError { fn from(e: OutOfRangeError) -> Self { Self::Amount(e.into()) } } +impl From for ParseError { + fn from(e: MissingDigitsError) -> Self { Self::Amount(e.into()) } +} + +impl From for ParseError { + fn from(e: InvalidCharacterError) -> Self { Self::Amount(e.into()) } +} + impl fmt::Display for ParseError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { ParseError::Amount(error) => write_err!(f, "invalid amount"; error), ParseError::Denomination(error) => write_err!(f, "invalid denomination"; error), + // We consider this to not be a source because it currently doesn't contain useful + // information + ParseError::MissingDenomination(_) => f.write_str("the input doesn't contain a denomination"), } } } @@ -185,6 +199,9 @@ impl std::error::Error for ParseError { match self { ParseError::Amount(error) => Some(error), ParseError::Denomination(error) => Some(error), + // We consider this to not be a source because it currently doesn't contain useful + // information + ParseError::MissingDenomination(_) => None, } } } @@ -197,12 +214,24 @@ pub enum ParseAmountError { OutOfRange(OutOfRangeError), /// Amount has higher precision than supported by the type. TooPrecise, - /// Invalid number format. - InvalidFormat, + /// A digit was expected but not found. + MissingDigits(MissingDigitsError), /// Input string was too large. InputTooLarge, /// Invalid character in input. - InvalidCharacter(char), + InvalidCharacter(InvalidCharacterError), +} + +impl From for ParseAmountError { + fn from(value: MissingDigitsError) -> Self { + Self::MissingDigits(value) + } +} + +impl From for ParseAmountError { + fn from(value: InvalidCharacterError) -> Self { + Self::InvalidCharacter(value) + } } impl fmt::Display for ParseAmountError { @@ -210,11 +239,11 @@ impl fmt::Display for ParseAmountError { use ParseAmountError::*; match *self { - OutOfRange(error) => write_err!(f, "amount out of range"; error), + OutOfRange(ref error) => write_err!(f, "amount out of range"; error), TooPrecise => f.write_str("amount has a too high precision"), - InvalidFormat => f.write_str("invalid number format"), + MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), InputTooLarge => f.write_str("input string was too large"), - InvalidCharacter(c) => write!(f, "invalid character in input: {}", c), + InvalidCharacter(ref error) => write_err!(f, "invalid character in the input"; error), } } } @@ -225,9 +254,10 @@ impl std::error::Error for ParseAmountError { use ParseAmountError::*; match *self { - TooPrecise | InvalidFormat | InputTooLarge - | InvalidCharacter(_) => None, + TooPrecise | InputTooLarge => None, OutOfRange(ref error) => Some(error), + MissingDigits(ref error) => Some(error), + InvalidCharacter(ref error) => Some(error), } } } @@ -303,6 +333,50 @@ impl From for ParseAmountError { } } +/// Error returned when digits were expected in the input but there were none. +/// +/// In particular, this is currently returned when the string is empty or only contains the minus sign. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct MissingDigitsError { + kind: MissingDigitsKind, +} + +impl fmt::Display for MissingDigitsError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self.kind { + MissingDigitsKind::Empty => f.write_str("the input is empty"), + MissingDigitsKind::OnlyMinusSign => f.write_str("there are no digits following the minus (-) sign"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for MissingDigitsError {} + +#[derive(Debug, Clone, Eq, PartialEq)] +enum MissingDigitsKind { + Empty, + OnlyMinusSign, +} + +/// Returned when the input contains an invalid character. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InvalidCharacterError { + invalid_char: char, +} + +impl fmt::Display for InvalidCharacterError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self.invalid_char { + '.' => f.write_str("there is more than one decimal separator (dot) in the input"), + '-' => f.write_str("there is more than one minus sign (-) in the input"), + c => write!(f, "the character '{}' is not a valid digit", c), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidCharacterError {} /// An error during amount parsing. #[derive(Debug, Clone, PartialEq, Eq)] @@ -336,6 +410,11 @@ impl std::error::Error for ParseDenominationError { } } +/// Error returned when the denomination is empty. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct MissingDenominationError; + /// Parsing error, unknown denomination. #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -385,7 +464,7 @@ fn parse_signed_to_satoshi( denom: Denomination, ) -> Result<(bool, u64), InnerParseError> { if s.is_empty() { - return Err(InnerParseError::InvalidFormat); + return Err(InnerParseError::MissingDigits(MissingDigitsError { kind: MissingDigitsKind::Empty })); } if s.len() > 50 { return Err(InnerParseError::InputTooLarge); @@ -394,7 +473,7 @@ fn parse_signed_to_satoshi( let is_negative = s.starts_with('-'); if is_negative { if s.len() == 1 { - return Err(InnerParseError::InvalidFormat); + return Err(InnerParseError::MissingDigits(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign })); } s = &s[1..]; } @@ -446,9 +525,9 @@ fn parse_signed_to_satoshi( None if max_decimals <= 0 => break, None => decimals = Some(0), // Double decimal dot. - _ => return Err(InnerParseError::InvalidFormat), + _ => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: '.' })), }, - c => return Err(InnerParseError::InvalidCharacter(c)), + c => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: c })), } } @@ -467,9 +546,9 @@ fn parse_signed_to_satoshi( enum InnerParseError { Overflow { is_negative: bool }, TooPrecise, - InvalidFormat, + MissingDigits(MissingDigitsError), InputTooLarge, - InvalidCharacter(char), + InvalidCharacter(InvalidCharacterError), } impl InnerParseError { @@ -477,9 +556,9 @@ impl InnerParseError { match self { Self::Overflow { is_negative } => OutOfRangeError { is_signed, is_greater_than_max: !is_negative }.into(), Self::TooPrecise => ParseAmountError::TooPrecise, - Self::InvalidFormat => ParseAmountError::InvalidFormat, + Self::MissingDigits(error) => ParseAmountError::MissingDigits(error), Self::InputTooLarge => ParseAmountError::InputTooLarge, - Self::InvalidCharacter(c) => ParseAmountError::InvalidCharacter(c), + Self::InvalidCharacter(error) => ParseAmountError::InvalidCharacter(error), } } } @@ -488,7 +567,7 @@ fn split_amount_and_denomination(s: &str) -> Result<(&str, Denomination), ParseE let (i, j) = if let Some(i) = s.find(' ') { (i, i + 1) } else { - let i = s.find(|c: char| c.is_alphabetic()).ok_or(ParseAmountError::InvalidFormat)?; + let i = s.find(|c: char| c.is_alphabetic()).ok_or(ParseError::MissingDenomination(MissingDenominationError))?; (i, i) }; Ok((&s[..i], s[j..].parse()?)) @@ -2042,12 +2121,12 @@ mod tests { let p = Amount::from_str_in; let sp = SignedAmount::from_str_in; - assert_eq!(p("x", btc), Err(E::InvalidCharacter('x'))); - assert_eq!(p("-", btc), Err(E::InvalidFormat)); - assert_eq!(sp("-", btc), Err(E::InvalidFormat)); - assert_eq!(p("-1.0x", btc), Err(E::InvalidCharacter('x'))); - assert_eq!(p("0.0 ", btc), Err(ParseAmountError::InvalidCharacter(' '))); - assert_eq!(p("0.000.000", btc), Err(E::InvalidFormat)); + assert_eq!(p("x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x' }))); + assert_eq!(p("-", btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign }))); + assert_eq!(sp("-", btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign }))); + assert_eq!(p("-1.0x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x' }))); + assert_eq!(p("0.0 ", btc), Err(E::from(InvalidCharacterError { invalid_char: ' ' }))); + assert_eq!(p("0.000.000", btc), Err(E::from(InvalidCharacterError { invalid_char: '.' }))); #[cfg(feature = "alloc")] let more_than_max = format!("1{}", Amount::MAX); #[cfg(feature = "alloc")] @@ -2332,7 +2411,7 @@ mod tests { use super::ParseAmountError as E; - assert_eq!(Amount::from_str("x BTC"), Err(E::InvalidCharacter('x').into())); + assert_eq!(Amount::from_str("x BTC"), Err(E::from(E::from(InvalidCharacterError { invalid_char: 'x' })).into())); assert_eq!( Amount::from_str("xBTC"), Err(Unknown(UnknownDenominationError("xBTC".into())).into()), @@ -2341,7 +2420,7 @@ mod tests { Amount::from_str("5 BTC BTC"), Err(Unknown(UnknownDenominationError("BTC BTC".into())).into()), ); - assert_eq!(Amount::from_str("5BTC BTC"), Err(E::InvalidCharacter('B').into())); + assert_eq!(Amount::from_str("5BTC BTC"), Err(E::from(InvalidCharacterError { invalid_char: 'B' }).into())); assert_eq!( Amount::from_str("5 5 BTC"), Err(Unknown(UnknownDenominationError("5 BTC".into())).into()), From 28d83551eb1db4fe34a7eb2d86aab5aa51266264 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 19 Feb 2024 14:01:11 +0100 Subject: [PATCH 2/4] Improve `ParseAmountError::InputTooLarge` This variant lacked pretty important information: what is the limit. We could just write it in the error message but it's even better to move it to a separate struct which also says how many characters exceed the limit - this helps users know how many need to be deleted. --- units/src/amount.rs | 48 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/units/src/amount.rs b/units/src/amount.rs index 61b20467..b4ee153a 100644 --- a/units/src/amount.rs +++ b/units/src/amount.rs @@ -177,6 +177,10 @@ impl From for ParseError { fn from(e: MissingDigitsError) -> Self { Self::Amount(e.into()) } } +impl From for ParseError { + fn from(e: InputTooLargeError) -> Self { Self::Amount(e.into()) } +} + impl From for ParseError { fn from(e: InvalidCharacterError) -> Self { Self::Amount(e.into()) } } @@ -217,7 +221,7 @@ pub enum ParseAmountError { /// A digit was expected but not found. MissingDigits(MissingDigitsError), /// Input string was too large. - InputTooLarge, + InputTooLarge(InputTooLargeError), /// Invalid character in input. InvalidCharacter(InvalidCharacterError), } @@ -228,6 +232,13 @@ impl From for ParseAmountError { } } +impl From for ParseAmountError { + fn from(value: InputTooLargeError) -> Self { + Self::InputTooLarge(value) + } +} + + impl From for ParseAmountError { fn from(value: InvalidCharacterError) -> Self { Self::InvalidCharacter(value) @@ -242,7 +253,7 @@ impl fmt::Display for ParseAmountError { OutOfRange(ref error) => write_err!(f, "amount out of range"; error), TooPrecise => f.write_str("amount has a too high precision"), MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), - InputTooLarge => f.write_str("input string was too large"), + InputTooLarge(ref error) => write_err!(f, "the input is too large"; error), InvalidCharacter(ref error) => write_err!(f, "invalid character in the input"; error), } } @@ -254,7 +265,8 @@ impl std::error::Error for ParseAmountError { use ParseAmountError::*; match *self { - TooPrecise | InputTooLarge => None, + TooPrecise => None, + InputTooLarge(ref error) => Some(error), OutOfRange(ref error) => Some(error), MissingDigits(ref error) => Some(error), InvalidCharacter(ref error) => Some(error), @@ -333,6 +345,24 @@ impl From for ParseAmountError { } } +/// Error returned when the input string is too large. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct InputTooLargeError { + len: usize, +} + +impl fmt::Display for InputTooLargeError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self.len - INPUT_STRING_LEN_LIMIT { + 1 => write!(f, "the input is one character longer than the maximum allowed length ({})", INPUT_STRING_LEN_LIMIT), + n => write!(f, "the input is {} characters longer than the maximum allowed length ({})", n, INPUT_STRING_LEN_LIMIT), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InputTooLargeError {} + /// Error returned when digits were expected in the input but there were none. /// /// In particular, this is currently returned when the string is empty or only contains the minus sign. @@ -457,6 +487,8 @@ fn is_too_precise(s: &str, precision: usize) -> bool { } } +const INPUT_STRING_LEN_LIMIT: usize = 50; + /// Parse decimal string in the given denomination into a satoshi value and a /// bool indicator for a negative amount. fn parse_signed_to_satoshi( @@ -466,8 +498,8 @@ fn parse_signed_to_satoshi( if s.is_empty() { return Err(InnerParseError::MissingDigits(MissingDigitsError { kind: MissingDigitsKind::Empty })); } - if s.len() > 50 { - return Err(InnerParseError::InputTooLarge); + if s.len() > INPUT_STRING_LEN_LIMIT { + return Err(InnerParseError::InputTooLarge(s.len())); } let is_negative = s.starts_with('-'); @@ -547,7 +579,7 @@ enum InnerParseError { Overflow { is_negative: bool }, TooPrecise, MissingDigits(MissingDigitsError), - InputTooLarge, + InputTooLarge(usize), InvalidCharacter(InvalidCharacterError), } @@ -557,7 +589,7 @@ impl InnerParseError { Self::Overflow { is_negative } => OutOfRangeError { is_signed, is_greater_than_max: !is_negative }.into(), Self::TooPrecise => ParseAmountError::TooPrecise, Self::MissingDigits(error) => ParseAmountError::MissingDigits(error), - Self::InputTooLarge => ParseAmountError::InputTooLarge, + Self::InputTooLarge(len) => ParseAmountError::InputTooLarge(InputTooLargeError { len }), Self::InvalidCharacter(error) => ParseAmountError::InvalidCharacter(error), } } @@ -2177,7 +2209,7 @@ mod tests { // more than 50 chars. assert_eq!( p("100000000000000.00000000000000000000000000000000000", Denomination::Bitcoin), - Err(E::InputTooLarge) + Err(E::InputTooLarge(InputTooLargeError { len: 51 })) ); } From 73b325aec5e548855ce68b31fd1733bf2b251f31 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 19 Feb 2024 19:01:35 +0100 Subject: [PATCH 3/4] Report position of the first "too precise" digit Sometimes people don't remember the exact number of decimal places supported by denomination or don't want to count (e.g. when converting fiat to BTC the calculator may yield too precise value). It's helpful to say in error message at which digit the precision is too high. This adds `TooPreciseError` struct containing the information and improves the error message to display it. --- units/src/amount.rs | 113 +++++++++++++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 38 deletions(-) diff --git a/units/src/amount.rs b/units/src/amount.rs index b4ee153a..b20e74fa 100644 --- a/units/src/amount.rs +++ b/units/src/amount.rs @@ -173,6 +173,10 @@ impl From for ParseError { fn from(e: OutOfRangeError) -> Self { Self::Amount(e.into()) } } +impl From for ParseError { + fn from(e: TooPreciseError) -> Self { Self::Amount(e.into()) } +} + impl From for ParseError { fn from(e: MissingDigitsError) -> Self { Self::Amount(e.into()) } } @@ -217,7 +221,7 @@ pub enum ParseAmountError { /// The amount is too big or too small. OutOfRange(OutOfRangeError), /// Amount has higher precision than supported by the type. - TooPrecise, + TooPrecise(TooPreciseError), /// A digit was expected but not found. MissingDigits(MissingDigitsError), /// Input string was too large. @@ -226,6 +230,12 @@ pub enum ParseAmountError { InvalidCharacter(InvalidCharacterError), } +impl From for ParseAmountError { + fn from(value: TooPreciseError) -> Self { + Self::TooPrecise(value) + } +} + impl From for ParseAmountError { fn from(value: MissingDigitsError) -> Self { Self::MissingDigits(value) @@ -251,7 +261,7 @@ impl fmt::Display for ParseAmountError { match *self { OutOfRange(ref error) => write_err!(f, "amount out of range"; error), - TooPrecise => f.write_str("amount has a too high precision"), + TooPrecise(ref error) => write_err!(f, "amount has a too high precision"; error), MissingDigits(ref error) => write_err!(f, "the input has too few digits"; error), InputTooLarge(ref error) => write_err!(f, "the input is too large"; error), InvalidCharacter(ref error) => write_err!(f, "invalid character in the input"; error), @@ -265,7 +275,7 @@ impl std::error::Error for ParseAmountError { use ParseAmountError::*; match *self { - TooPrecise => None, + TooPrecise(ref error) => Some(error), InputTooLarge(ref error) => Some(error), OutOfRange(ref error) => Some(error), MissingDigits(ref error) => Some(error), @@ -345,6 +355,24 @@ impl From for ParseAmountError { } } +/// Error returned when the input string has higher precision than satoshis. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct TooPreciseError { + position: usize, +} + +impl fmt::Display for TooPreciseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self.position { + 0 => f.write_str("the amount is less than 1 satoshi but it's not zero"), + pos => write!(f, "the digits starting from position {} represent a sub-satoshi amount", pos), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for TooPreciseError {} + /// Error returned when the input string is too large. #[derive(Debug, Clone, Eq, PartialEq)] pub struct InputTooLargeError { @@ -477,13 +505,22 @@ impl std::error::Error for PossiblyConfusingDenominationError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } } -fn is_too_precise(s: &str, precision: usize) -> bool { +/// Returns `Some(position)` if the precision is not supported. +/// +/// The position indicates the first digit that is too precise. +fn is_too_precise(s: &str, precision: usize) -> Option { 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'), + Some(pos) if precision >= pos => { Some(0) }, + Some(pos) => { + s[..pos].char_indices().rev().take(precision).find(|(_, d)| *d != '0').map(|(i, _)| i) + .or_else(|| { + s[(pos + 1)..].char_indices().find(|(_, d)| *d != '0').map(|(i, _)| i + pos + 1) + }) + }, + None if precision >= s.len() => { Some(0) }, + None => { + s.char_indices().rev().take(precision).find(|(_, d)| *d != '0').map(|(i, _)| i) + }, } } @@ -520,10 +557,10 @@ fn parse_signed_to_satoshi( // there are no decimals and the last digits are zeroes as // many as the difference in precision. let last_n = precision_diff.unsigned_abs().into(); - if is_too_precise(s, last_n) { + if let Some(position) = is_too_precise(s, last_n) { match s.parse::() { Ok(0) => return Ok((is_negative, 0)), - _ => return Err(InnerParseError::TooPrecise), + _ => return Err(InnerParseError::TooPrecise(TooPreciseError { position: position + is_negative as usize })), } } s = &s[0..s.find('.').unwrap_or(s.len()) - last_n]; @@ -535,7 +572,7 @@ fn parse_signed_to_satoshi( let mut decimals = None; let mut value: u64 = 0; // as satoshis - for c in s.chars() { + for (i, c) in s.char_indices() { match c { '0'..='9' => { // Do `value = 10 * value + digit`, catching overflows. @@ -550,7 +587,7 @@ fn parse_signed_to_satoshi( decimals = match decimals { None => None, Some(d) if d < max_decimals => Some(d + 1), - _ => return Err(InnerParseError::TooPrecise), + _ => return Err(InnerParseError::TooPrecise(TooPreciseError { position: i + is_negative as usize, })), }; } '.' => match decimals { @@ -577,7 +614,7 @@ fn parse_signed_to_satoshi( enum InnerParseError { Overflow { is_negative: bool }, - TooPrecise, + TooPrecise(TooPreciseError), MissingDigits(MissingDigitsError), InputTooLarge(usize), InvalidCharacter(InvalidCharacterError), @@ -587,7 +624,7 @@ impl InnerParseError { fn convert(self, is_signed: bool) -> ParseAmountError { match self { Self::Overflow { is_negative } => OutOfRangeError { is_signed, is_greater_than_max: !is_negative }.into(), - Self::TooPrecise => ParseAmountError::TooPrecise, + Self::TooPrecise(error) => ParseAmountError::TooPrecise(error), Self::MissingDigits(error) => ParseAmountError::MissingDigits(error), Self::InputTooLarge(len) => ParseAmountError::InputTooLarge(InputTooLargeError { len }), Self::InvalidCharacter(error) => ParseAmountError::InvalidCharacter(error), @@ -2114,9 +2151,9 @@ mod tests { assert_eq!(sf(-0.00012345, D::Bitcoin), Ok(ssat(-12345))); assert_eq!(f(-100.0, D::MilliSatoshi), Err(OutOfRangeError::negative().into())); - assert_eq!(f(11.22, D::Satoshi), Err(ParseAmountError::TooPrecise)); - assert_eq!(sf(-100.0, D::MilliSatoshi), Err(ParseAmountError::TooPrecise)); - assert_eq!(f(42.123456781, D::Bitcoin), Err(ParseAmountError::TooPrecise)); + assert_eq!(f(11.22, D::Satoshi), Err(TooPreciseError { position: 3 }.into())); + assert_eq!(sf(-100.0, D::MilliSatoshi), Err(TooPreciseError { position: 1 }.into())); + assert_eq!(f(42.123456781, D::Bitcoin), Err(TooPreciseError { position: 11 }.into())); assert_eq!(sf(-184467440738.0, D::Bitcoin), Err(OutOfRangeError::too_small().into())); assert_eq!(f(18446744073709551617.0, D::Satoshi), Err(OutOfRangeError::too_big(false).into())); @@ -2163,16 +2200,16 @@ mod tests { let more_than_max = format!("1{}", Amount::MAX); #[cfg(feature = "alloc")] assert_eq!(p(&more_than_max, btc), Err(OutOfRangeError::too_big(false).into())); - 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("0.000000042", btc), Err(TooPreciseError { position: 10 }.into())); + assert_eq!(p("999.0000000", msat), Err(TooPreciseError { position: 0 }.into())); + assert_eq!(p("1.0000000", msat), Err(TooPreciseError { position: 0 }.into())); + assert_eq!(p("1.1", msat), Err(TooPreciseError { position: 0 }.into())); + assert_eq!(p("1000.1", msat), Err(TooPreciseError { position: 5 }.into())); + assert_eq!(p("1001.0000000", msat), Err(TooPreciseError { position: 3 }.into())); + assert_eq!(p("1000.0000001", msat), Err(TooPreciseError { position: 11 }.into())); + assert_eq!(p("1000.1000000", msat), Err(TooPreciseError { position: 5 }.into())); + assert_eq!(p("1100.0000000", msat), Err(TooPreciseError { position: 1 }.into())); + assert_eq!(p("10001.0000000", msat), Err(TooPreciseError { position: 4 }.into())); 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))); @@ -2200,7 +2237,7 @@ mod tests { assert!(Amount::from_str_in(&(amount + Amount(1)).to_string_in(sat), sat).is_ok()); } - assert_eq!(p("12.000", Denomination::MilliSatoshi), Err(E::TooPrecise)); + assert_eq!(p("12.000", Denomination::MilliSatoshi), Err(TooPreciseError { position: 0 }.into())); // exactly 50 chars. assert_eq!( p("100000000000000.0000000000000000000000000000000000", Denomination::Bitcoin), @@ -2488,10 +2525,10 @@ mod tests { case("-1 BTC", Err(OutOfRangeError::negative())); case("-0.0 BTC", Err(OutOfRangeError::negative())); - case("0.123456789 BTC", Err(E::TooPrecise)); - scase("-0.1 satoshi", Err(E::TooPrecise)); - case("0.123456 mBTC", Err(E::TooPrecise)); - scase("-1.001 bits", Err(E::TooPrecise)); + case("0.123456789 BTC", Err(TooPreciseError { position: 10 })); + scase("-0.1 satoshi", Err(TooPreciseError { position: 3 })); + case("0.123456 mBTC", Err(TooPreciseError { position: 7 })); + scase("-1.001 bits", Err(TooPreciseError { position: 5 })); scase("-200000000000 BTC", Err(OutOfRangeError::too_small())); case("18446744073709551616 sat", Err(OutOfRangeError::too_big(false))); @@ -2571,11 +2608,11 @@ mod tests { ); assert_eq!( sa_str(&sa_sat(i64::MAX).to_string_in(D::Satoshi), D::NanoBitcoin), - Err(ParseAmountError::TooPrecise) + Err(TooPreciseError { position: 18 }.into()) ); assert_eq!( sa_str(&sa_sat(i64::MIN).to_string_in(D::Satoshi), D::NanoBitcoin), - Err(ParseAmountError::TooPrecise) + Err(TooPreciseError { position: 19 }.into()) ); assert_eq!( @@ -2584,11 +2621,11 @@ mod tests { ); assert_eq!( sa_str(&sa_sat(i64::MAX).to_string_in(D::Satoshi), D::PicoBitcoin), - Err(ParseAmountError::TooPrecise) + Err(TooPreciseError { position: 18 }.into()) ); assert_eq!( sa_str(&sa_sat(i64::MIN).to_string_in(D::Satoshi), D::PicoBitcoin), - Err(ParseAmountError::TooPrecise) + Err(TooPreciseError { position: 19 }.into()) ); } @@ -2675,7 +2712,7 @@ mod tests { // errors let t: Result = serde_json::from_str("{\"amt\": 1000000.000000001, \"samt\": 1}"); - assert!(t.unwrap_err().to_string().contains(&ParseAmountError::TooPrecise.to_string())); + assert!(t.unwrap_err().to_string().contains(&ParseAmountError::TooPrecise(TooPreciseError { position: 16 }).to_string())); let t: Result = serde_json::from_str("{\"amt\": -1, \"samt\": 1}"); assert!(t.unwrap_err().to_string().contains(&OutOfRangeError::negative().to_string())); } From 08f83898a34f1cb03bd3803f313f2aab70e16e5e Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 24 Feb 2024 09:37:00 +0100 Subject: [PATCH 4/4] Report the position of an invalid char in amount It can be helpful to report the exact position where the invalid character was encountered. This change adds he feature. --- units/src/amount.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/units/src/amount.rs b/units/src/amount.rs index b20e74fa..a8988a49 100644 --- a/units/src/amount.rs +++ b/units/src/amount.rs @@ -421,6 +421,7 @@ enum MissingDigitsKind { #[derive(Debug, Clone, PartialEq, Eq)] pub struct InvalidCharacterError { invalid_char: char, + position: usize, } impl fmt::Display for InvalidCharacterError { @@ -428,7 +429,7 @@ impl fmt::Display for InvalidCharacterError { match self.invalid_char { '.' => f.write_str("there is more than one decimal separator (dot) in the input"), '-' => f.write_str("there is more than one minus sign (-) in the input"), - c => write!(f, "the character '{}' is not a valid digit", c), + c => write!(f, "the character '{}' at position {} is not a valid digit", c, self.position), } } } @@ -594,9 +595,9 @@ fn parse_signed_to_satoshi( None if max_decimals <= 0 => break, None => decimals = Some(0), // Double decimal dot. - _ => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: '.' })), + _ => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: '.', position: i + is_negative as usize })), }, - c => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: c })), + c => return Err(InnerParseError::InvalidCharacter(InvalidCharacterError { invalid_char: c, position: i + is_negative as usize })), } } @@ -2190,12 +2191,12 @@ mod tests { let p = Amount::from_str_in; let sp = SignedAmount::from_str_in; - assert_eq!(p("x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x' }))); + assert_eq!(p("x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 0 }))); assert_eq!(p("-", btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign }))); assert_eq!(sp("-", btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign }))); - assert_eq!(p("-1.0x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x' }))); - assert_eq!(p("0.0 ", btc), Err(E::from(InvalidCharacterError { invalid_char: ' ' }))); - assert_eq!(p("0.000.000", btc), Err(E::from(InvalidCharacterError { invalid_char: '.' }))); + assert_eq!(p("-1.0x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 4 }))); + assert_eq!(p("0.0 ", btc), Err(E::from(InvalidCharacterError { invalid_char: ' ', position: 3 }))); + assert_eq!(p("0.000.000", btc), Err(E::from(InvalidCharacterError { invalid_char: '.', position: 5 }))); #[cfg(feature = "alloc")] let more_than_max = format!("1{}", Amount::MAX); #[cfg(feature = "alloc")] @@ -2480,7 +2481,7 @@ mod tests { use super::ParseAmountError as E; - assert_eq!(Amount::from_str("x BTC"), Err(E::from(E::from(InvalidCharacterError { invalid_char: 'x' })).into())); + assert_eq!(Amount::from_str("x BTC"), Err(E::from(E::from(InvalidCharacterError { invalid_char: 'x', position: 0 })).into())); assert_eq!( Amount::from_str("xBTC"), Err(Unknown(UnknownDenominationError("xBTC".into())).into()), @@ -2489,7 +2490,7 @@ mod tests { Amount::from_str("5 BTC BTC"), Err(Unknown(UnknownDenominationError("BTC BTC".into())).into()), ); - assert_eq!(Amount::from_str("5BTC BTC"), Err(E::from(InvalidCharacterError { invalid_char: 'B' }).into())); + assert_eq!(Amount::from_str("5BTC BTC"), Err(E::from(InvalidCharacterError { invalid_char: 'B', position: 1 }).into())); assert_eq!( Amount::from_str("5 5 BTC"), Err(Unknown(UnknownDenominationError("5 BTC".into())).into()),