Merge rust-bitcoin/rust-bitcoin#3559: Replace `String` with `InputString`

3f2e760d1f Replace String with InputString in ParseIntError (Tobin C. Harding)
aa5c78430c Replace invalidInteger with ParseIntError (Tobin C. Harding)
9b7a706bfd Remove From<ParseIntError> (Tobin C. Harding)
c90f4b6033 Fix bug in error output (Tobin C. Harding)
c986b2f620 internals: Move error.rs to error/mod.rs (Tobin C. Harding)

Pull request description:

  This PR hopefully clears the way for removing many of the `alloc` feature gates in `units` and `primitives`

  The three final patches were tested by adding the following test to `units::locktime::absolute`:
  ```rust
      #[test]
      pub fn debug_absolute_error_conversion_height() {
          let invalid_height = LOCK_TIME_THRESHOLD + 1;
          let err = Height::from_consensus(invalid_height).unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);

          let invalid_time =  LOCK_TIME_THRESHOLD - 1;
          let err = Time::from_consensus(invalid_time).unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);

          let invalid_height = std::format!("{:x}", LOCK_TIME_THRESHOLD + 1);
          let err = Height::from_hex(&invalid_height).unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);

          let invalid_time = std::format!("{:x}", LOCK_TIME_THRESHOLD - 1);
          let err = Time::from_hex(&invalid_time).unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);

          let err = Height::from_hex("somerandomshit").unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);

          let err = Time::from_hex("somerandomshit").unwrap_err();
          std::println!("{:?}", err);
          std::println!("{}", err);
      }
  ```

  Gives the following output (the last four lines is the bit that changes, the rest just proves we don't break other variants)

  On commit: `d47ff1c25 Remove From<ParseIntError>`

  ConversionError { unit: Blocks, input: 500000001 }
  invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
  ConversionError { unit: Seconds, input: 499999999 }
  invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
  ParseHeightError(Conversion(500000001))
  block height 500000001 is above limit 499999999
  ParseTimeError(Conversion(499999999))
  block height 499999999 is below limit 500000000
  ParseHeightError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
  failed to parse somerandomshit as block height
  ParseTimeError(InvalidInteger { source: ParseIntError { kind: InvalidDigit }, input: "somerandomshit" })
  failed to parse somerandomshit as block time

  On commit: `0155a0d9a Replace invalidInteger with ParseIntError`

  ConversionError { unit: Blocks, input: 500000001 }
  invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
  ConversionError { unit: Seconds, input: 499999999 }
  invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
  ParseHeightError(Conversion(500000001))
  block height 500000001 is above limit 499999999
  ParseTimeError(Conversion(499999999))
  block height 499999999 is below limit 500000000
  ParseHeightError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
  failed to parse somerandomshit as block height
  ParseTimeError(ParseInt(ParseIntError { input: "somerandomshit", bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
  failed to parse somerandomshit as block time

  On Commit: `3f2e760d1 Replace String with InputString in ParseIntError`

  ConversionError { unit: Blocks, input: 500000001 }
  invalid lock time value 500000001, expected lock-by-blockheight (must be < 500000000)
  ConversionError { unit: Seconds, input: 499999999 }
  invalid lock time value 499999999, expected lock-by-blocktime (must be >= 500000000)
  ParseHeightError(Conversion(500000001))
  block height 500000001 is above limit 499999999
  ParseTimeError(Conversion(499999999))
  block time 499999999 is below limit 500000000
  ParseHeightError(ParseInt(ParseIntError { input: InputString("somerandomshit"), bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
  failed to parse 'somerandomshit' as absolute Height/Time (block height)
  ParseTimeError(ParseInt(ParseIntError { input: InputString("somerandomshit"), bits: 32, is_signed: true, source: ParseIntError { kind: InvalidDigit } }))
  failed to parse 'somerandomshit' as absolute Height/Time (block time)

ACKs for top commit:
  apoelstra:
    ACK 3f2e760d1fef2951f93a2554cd53340b0d7a6e0b; successfully ran local tests; nice!

Tree-SHA512: f7fd55acfb83082419db22c24a6b375c20e2631263401e500410c5b5659463f06dc4bdb145621e475dc15d75e764668cdcbf8f88006a487248a05fdb237ad136

units/src/locktime/absolute.rs

@@ -6,11 +6,11 @@
 use alloc::{boxed::Box, string::String};
 use core::fmt;
 
-use internals::write_err;
+use internals::error::InputString;
 
+use crate::parse::ParseIntError;
 #[cfg(feature = "alloc")]
 use crate::parse;
-use crate::parse::ParseIntError;
 
 /// The Threshold for deciding whether a lock time value is a height or a time (see [Bitcoin Core]).
 ///
@@ -204,7 +204,7 @@ pub struct ParseTimeError(ParseError);
 
 impl fmt::Display for ParseTimeError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.display(f, "block height", LOCK_TIME_THRESHOLD, u32::MAX)
+        self.0.display(f, "block time", LOCK_TIME_THRESHOLD, u32::MAX)
     }
 }
 
@@ -221,7 +221,7 @@ impl From<ParseError> for ParseTimeError {
 fn parser<T, E, S, F>(f: F) -> impl FnOnce(S) -> Result<T, E>
 where
     E: From<ParseError>,
-    S: AsRef<str> + Into<String>,
+    S: AsRef<str> + Into<InputString>,
     F: FnOnce(u32) -> Result<T, ConversionError>,
 {
     move |s| {
@@ -234,7 +234,7 @@ where
 fn parse_hex<T, E, S, F>(s: S, f: F) -> Result<T, E>
 where
     E: From<ParseError>,
-    S: AsRef<str> + Into<String>,
+    S: AsRef<str> + Into<InputString>,
     F: FnOnce(u32) -> Result<T, ConversionError>,
 {
     let n = i64::from_str_radix(parse::hex_remove_optional_prefix(s.as_ref()), 16)
@@ -301,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,8 +310,8 @@ enum ParseError {
 internals::impl_from_infallible!(ParseError);
 
 impl ParseError {
-    fn invalid_int<S: Into<String>>(s: S) -> impl FnOnce(core::num::ParseIntError) -> Self {
+    fn invalid_int<S: Into<InputString>>(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(
@@ -326,14 +326,16 @@ 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)
+                // Outputs "failed to parse <input_string> as absolute Height/Time (<subject> is above limit <upper_bound>)"
+                write!(f, "{} ({} is above limit {})", input.display_cannot_parse("absolute Height/Time"), subject, 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)
+                // Outputs "failed to parse <input_string> as absolute Height/Time (<subject> is below limit <lower_bound>)"
+                write!(f, "{} ({} is below limit {})", input.display_cannot_parse("absolute Height/Time"), subject, lower_bound)
             }
-            InvalidInteger { source, input } => {
+            ParseInt(ParseIntError { input, bits: _, is_signed: _, source: _ }) => {
-                write_err!(f, "failed to parse {} as {}", input, subject; source)
+                write!(f, "{} ({})", input.display_cannot_parse("absolute Height/Time"), subject)
             }
             Conversion(value) if *value < i64::from(lower_bound) => {
                 write!(f, "{} {} is below limit {}", subject, value, lower_bound)
@@ -352,20 +354,14 @@ impl ParseError {
         use ParseError::*;
 
         match self {
-            InvalidInteger { source, .. } if *source.kind() == IntErrorKind::PosOverflow => None,
+            ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::PosOverflow => None,
-            InvalidInteger { source, .. } if *source.kind() == IntErrorKind::NegOverflow => None,
+            ParseInt(ParseIntError { source, .. }) if *source.kind() == IntErrorKind::NegOverflow => None,
-            InvalidInteger { source, .. } => Some(source),
+            ParseInt(ParseIntError { source, .. }) => Some(source),
             Conversion(_) => None,
         }
     }
 }
 
-impl From<ParseIntError> for ParseError {
-    fn from(value: ParseIntError) -> Self {
-        Self::InvalidInteger { source: value.source, input: value.input }
-    }
-}
-
 impl From<ConversionError> for ParseError {
     fn from(value: ConversionError) -> Self { Self::Conversion(value.input.into()) }
 }

units/src/parse.rs

@@ -7,6 +7,7 @@ use core::fmt;
 use core::str::FromStr;
 
 use internals::write_err;
+use internals::error::InputString;
 
 /// Error with rich context returned when a string can't be parsed as an integer.
 ///
@@ -20,26 +21,20 @@ use internals::write_err;
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub struct ParseIntError {
-    pub(crate) input: String,
+    pub(crate) input: InputString,
     // 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,
 }
 
-impl ParseIntError {
-    /// Returns the input that was attempted to be parsed.
-    pub fn input(&self) -> &str { &self.input }
-}
-
 impl fmt::Display for ParseIntError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let signed = if self.is_signed { "signed" } else { "unsigned" };
-        let n = if self.bits == 8 { "n" } else { "" };
+        write_err!(f, "{} ({}, {}-bit)", self.input.display_cannot_parse("integer"), signed, self.bits; self.source)
-        write_err!(f, "failed to parse '{}' as a{} {}-bit {} integer", self.input, n, self.bits, signed; self.source)
     }
 }
 
@@ -74,7 +69,7 @@ impl_integer!(u8, i8, u16, i16, u32, i32, u64, i64, u128, i128);
 ///
 /// If the caller owns `String` or `Box<str>` which is not used later it's better to pass it as
 /// owned since it avoids allocation in error case.
-pub fn int<T: Integer, S: AsRef<str> + Into<String>>(s: S) -> Result<T, ParseIntError> {
+pub fn int<T: Integer, S: AsRef<str> + Into<InputString>>(s: S) -> Result<T, ParseIntError> {
     s.as_ref().parse().map_err(|error| {
         ParseIntError {
             input: s.into(),