From aa5c78430c5197bf2511857aa0c020b8cc7d4cd0 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 1 Nov 2024 16:54:45 +1100 Subject: [PATCH] Replace invalidInteger with ParseIntError We have a special type for wrapping integer parsing errors, use it. To test this I added the following tests: #[test] pub fn debug_absolute_error_conversion_height() { let invalid_height = LOCK_TIME_THRESHOLD + 1; let _ = Height::from_consensus(invalid_height).unwrap(); } #[test] pub fn debug_absolute_error_conversion_time() { let invalid_time = LOCK_TIME_THRESHOLD - 1; let _ = Time::from_consensus(invalid_time).unwrap(); } #[test] #[cfg(feature = "std")] pub fn debug_absolute_error_conversion_height_string() { let invalid_height = std::format!("{:x}", LOCK_TIME_THRESHOLD + 1); let _ = Height::from_hex(&invalid_height).unwrap(); } #[test] #[cfg(feature = "std")] pub fn debug_absolute_error_conversion_time_string() { let invalid_time = std::format!("{:x}", LOCK_TIME_THRESHOLD - 1); let _ = Time::from_hex(&invalid_time).unwrap(); } #[test] #[cfg(feature = "std")] pub fn debug_absolute_error_height_invalid_hex_string() { let _ = Height::from_hex("somerandomshit").unwrap(); } #[test] #[cfg(feature = "std")] pub fn debug_absolute_error_time_invalid_hex_string() { let _ = Time::from_hex("somerandomshit").unwrap(); } Which resulted in the following output Before: ---- locktime::absolute::tests::debug_absolute_error_conversion_height stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_height' panicked at units/src/locktime/absolute.rs:431:56: called `Result::unwrap()` on an `Err` value: ConversionError { unit: Blocks, input: 500000001 } note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ---- locktime::absolute::tests::debug_absolute_error_conversion_height_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_height_string' panicked at units/src/locktime/absolute.rs:444:51: called `Result::unwrap()` on an `Err` value: ParseHeightError(Conversion(500000001)) ---- locktime::absolute::tests::debug_absolute_error_conversion_time stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_time' panicked at units/src/locktime/absolute.rs:437:52: called `Result::unwrap()` on an `Err` value: ConversionError { unit: Seconds, input: 499999999 } ---- locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string' panicked at units/src/locktime/absolute.rs:457:52: called `Result::unwrap()` on an `Err` value: ParseHeightError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" }) ---- locktime::absolute::tests::debug_absolute_error_conversion_time_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_time_string' panicked at units/src/locktime/absolute.rs:451:47: called `Result::unwrap()` on an `Err` value: ParseTimeError(Conversion(499999999)) ---- locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string' panicked at units/src/locktime/absolute.rs:464:50: called `Result::unwrap()` on an `Err` value: ParseTimeError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" }) After: ---- locktime::absolute::tests::debug_absolute_error_conversion_height stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_height' panicked at units/src/locktime/absolute.rs:432:56: called `Result::unwrap()` on an `Err` value: ConversionError { unit: Blocks, input: 500000001 } ---- locktime::absolute::tests::debug_absolute_error_conversion_height_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_height_string' panicked at units/src/locktime/absolute.rs:445:51: called `Result::unwrap()` on an `Err` value: ParseHeightError(Conversion(500000001)) ---- locktime::absolute::tests::debug_absolute_error_conversion_time stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_time' panicked at units/src/locktime/absolute.rs:438:52: called `Result::unwrap()` on an `Err` value: ConversionError { unit: Seconds, input: 499999999 } ---- locktime::absolute::tests::debug_absolute_error_conversion_time_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_conversion_time_string' panicked at units/src/locktime/absolute.rs:452:47: called `Result::unwrap()` on an `Err` value: ParseTimeError(Conversion(499999999)) ---- locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_height_invalid_hex_string' panicked at units/src/locktime/absolute.rs:458:52: called `Result::unwrap()` on an `Err` value: ParseHeightError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: false, source: ParseIntError { kind: InvalidDigit } })) ---- locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string stdout ---- thread 'locktime::absolute::tests::debug_absolute_error_time_invalid_hex_string' panicked at units/src/locktime/absolute.rs:465:50: called `Result::unwrap()` on an `Err` value: ParseTimeError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: false, source: ParseIntError { kind: InvalidDigit } })) --- units/src/locktime/absolute.rs | 17 +++++++++-------- units/src/parse.rs | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/units/src/locktime/absolute.rs b/units/src/locktime/absolute.rs index 60a99da21..484439724 100644 --- a/units/src/locktime/absolute.rs +++ b/units/src/locktime/absolute.rs @@ -8,6 +8,7 @@ use core::fmt; use internals::write_err; +use crate::parse::ParseIntError; #[cfg(feature = "alloc")] use crate::parse; @@ -300,7 +301,7 @@ impl fmt::Display for LockTimeUnit { /// Internal - common representation for height and time. #[derive(Debug, Clone, Eq, PartialEq)] enum ParseError { - InvalidInteger { source: core::num::ParseIntError, input: String }, + ParseInt(ParseIntError), // unit implied by outer type // we use i64 to have nicer messages for negative values Conversion(i64), @@ -310,7 +311,7 @@ internals::impl_from_infallible!(ParseError); impl ParseError { fn invalid_int>(s: S) -> impl FnOnce(core::num::ParseIntError) -> Self { - move |source| Self::InvalidInteger { source, input: s.into() } + move |source| Self::ParseInt(ParseIntError { input: s.into(), bits: 32, is_signed: true , source }) } fn display( @@ -325,13 +326,13 @@ impl ParseError { use ParseError::*; match self { - InvalidInteger { source, input } if *source.kind() == IntErrorKind::PosOverflow => { + ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) if *source.kind() == IntErrorKind::PosOverflow => { write!(f, "{} {} is above limit {}", subject, input, upper_bound) } - InvalidInteger { source, input } if *source.kind() == IntErrorKind::NegOverflow => { + ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) if *source.kind() == IntErrorKind::NegOverflow => { write!(f, "{} {} is below limit {}", subject, input, lower_bound) } - InvalidInteger { source, input } => { + ParseInt(ParseIntError { input, bits: _, is_signed: _, source }) => { write_err!(f, "failed to parse {} as {}", input, subject; source) } Conversion(value) if *value < i64::from(lower_bound) => { @@ -351,9 +352,9 @@ impl ParseError { use ParseError::*; match self { - InvalidInteger { source, .. } if *source.kind() == IntErrorKind::PosOverflow => None, - InvalidInteger { source, .. } if *source.kind() == IntErrorKind::NegOverflow => None, - InvalidInteger { source, .. } => Some(source), + ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::PosOverflow => None, + ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::NegOverflow => None, + ParseInt(ParseIntError { source, .. }) => Some(source), Conversion(_) => None, } } diff --git a/units/src/parse.rs b/units/src/parse.rs index 1ba8fbb3a..02df36f6a 100644 --- a/units/src/parse.rs +++ b/units/src/parse.rs @@ -22,11 +22,11 @@ use internals::write_err; pub struct ParseIntError { pub(crate) input: String, // for displaying - see Display impl with nice error message below - bits: u8, + pub(crate) bits: u8, // We could represent this as a single bit but it wouldn't actually derease the cost of moving // the struct because String contains pointers so there will be padding of bits at least // pointer_size - 1 bytes: min 1B in practice. - is_signed: bool, + pub(crate) is_signed: bool, pub(crate) source: core::num::ParseIntError, }