Merge rust-bitcoin/rust-bitcoin#1400: Add trait `FromStrHex` for tuple structs with single `u32` member

e00dfa9806 impl FromHexStr for structs with single u32 member (connormullett)

Pull request description:

  Closes: #1112

   - Adds new trait `FromStrHex` with 2 methods: `from_hex_str` and `from_hex_str_no_prefix`
   -  Impl new trait on each tuple struct with single u32 member. eg `Time(u32)`

  As stated in the issue, grep through codebase with `\(u32\)` and `\(pub u32\)` to see all implementations and verify none were missed.

  NonStandardSighashType is an error type and should never be constructed from a hex string. Therefore, it has been omitted from this change.

  Tests are somewhat redundant, but cover 4 cases each. 2 happy paths, 1 for each function. 1 case for malformed/invalid hex input, and 1 for calling no_prefix without a prefix

ACKs for top commit:
  Kixunil:
    ACK e00dfa9806
  apoelstra:
    ACK e00dfa9806

Tree-SHA512: 221faef7fc1fa8fdb4cba79cfae317a0b63984937c345c6ca2287123a078f38911cdc07db7589a88b7bc6fbecf389e9bcff47952728410510ffcfc1857e0f91f

bitcoin/src/amount.rs

@@ -1403,6 +1403,7 @@ pub mod serde {
 mod verification {
     use std::cmp;
     use std::convert::TryInto;
+
     use super::*;
 
     // Note regarding the `unwind` parameter: this defines how many iterations

bitcoin/src/blockdata/locktime/absolute.rs

@@ -19,6 +19,7 @@ use crate::error::ParseIntError;
 use crate::io::{self, Read, Write};
 use crate::prelude::*;
 use crate::parse::{self, impl_parse_str_through_int};
+use crate::string::FromHexStr;
 
 #[cfg(docsrs)]
 use crate::absolute;
@@ -86,6 +87,15 @@ impl fmt::Display for PackedLockTime {
     }
 }
 
+impl FromHexStr for PackedLockTime {
+    type Error = Error;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let packed_lock_time = crate::parse::hex_u32(s)?;
+        Ok(Self(packed_lock_time))
+    }
+}
+
 impl Encodable for PackedLockTime {
     #[inline]
     fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
@@ -482,6 +492,16 @@ impl fmt::Display for Height {
     }
 }
 
+impl FromHexStr for Height {
+    type Error = Error;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let height = crate::parse::hex_u32(s)?;
+        Self::from_consensus(height)
+    }
+}
+
+
 impl FromStr for Height {
     type Err = Error;
 
@@ -565,6 +585,15 @@ impl fmt::Display for Time {
     }
 }
 
+impl FromHexStr for Time {
+    type Error = Error;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let time = crate::parse::hex_u32(s)?;
+        Time::from_consensus(time)
+    }
+}
+
 impl FromStr for Time {
     type Err = Error;
 
@@ -750,4 +779,64 @@ mod tests {
         let got = format!("{:#}", n);
         assert_eq!(got, "block-height 100");
     }
+
+    #[test]
+    fn time_from_str_hex_happy_path() {
+        let actual = Time::from_hex_str("0x6289C350").unwrap();
+        let expected = Time::from_consensus(0x6289C350).unwrap();
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn time_from_str_hex_no_prefix_happy_path() {
+        let time = Time::from_hex_str_no_prefix("6289C350").unwrap();
+        assert_eq!(time, Time(0x6289C350));
+    }
+
+    #[test]
+    fn time_from_str_hex_invalid_hex_should_err() {
+        let hex = "0xzb93";
+        let result = Time::from_hex_str(hex);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn packed_lock_time_from_str_hex_happy_path() {
+        let actual = PackedLockTime::from_hex_str("0xBA70D").unwrap();
+        let expected = PackedLockTime(0xBA70D);
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn packed_lock_time_from_str_hex_no_prefix_happy_path() {
+        let lock_time = PackedLockTime::from_hex_str_no_prefix("BA70D").unwrap();
+        assert_eq!(lock_time, PackedLockTime(0xBA70D));
+    }
+
+    #[test]
+    fn packed_lock_time_from_str_hex_invalid_hex_should_ergr() {
+        let hex = "0xzb93";
+        let result = PackedLockTime::from_hex_str(hex);
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn height_from_str_hex_happy_path() {
+        let actual = Height::from_hex_str("0xBA70D").unwrap();
+        let expected = Height(0xBA70D);
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn height_from_str_hex_no_prefix_happy_path() {
+        let height = Height::from_hex_str_no_prefix("BA70D").unwrap();
+        assert_eq!(height, Height(0xBA70D));
+    }
+
+    #[test]
+    fn height_from_str_hex_invalid_hex_should_err() {
+        let hex = "0xzb93";
+        let result = Height::from_hex_str(hex);
+        assert!(result.is_err());
+    }
 }

bitcoin/src/blockdata/transaction.rs

@@ -15,6 +15,7 @@
 use crate::prelude::*;
 
 use crate::io;
+use crate::string::FromHexStr;
 use core::{fmt, str, default::Default};
 use core::convert::TryFrom;
 
@@ -405,6 +406,15 @@ impl Sequence {
     }
 }
 
+impl FromHexStr for Sequence {
+    type Error = crate::parse::ParseIntError;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let sequence = crate::parse::hex_u32(s)?;
+        Ok(Self::from_consensus(sequence))
+    }
+}
+
 impl Default for Sequence {
     /// The default value of sequence is 0xffffffff.
     fn default() -> Self {
@@ -1463,6 +1473,25 @@ mod tests {
         assert!(unit_time_lock.is_rbf());
         assert!(!lock_time_disabled.is_relative_lock_time());
     }
+
+    #[test]
+    fn sequence_from_str_hex_happy_path() {
+        let sequence = Sequence::from_hex_str("0xFFFFFFFF").unwrap();
+        assert_eq!(sequence, Sequence::MAX);
+    }
+
+    #[test]
+    fn sequence_from_str_hex_no_prefix_happy_path() {
+        let sequence = Sequence::from_hex_str_no_prefix("FFFFFFFF").unwrap();
+        assert_eq!(sequence, Sequence::MAX);
+    }
+
+    #[test]
+    fn sequence_from_str_hex_invalid_hex_should_err() {
+        let hex = "0xzb93";
+        let result = Sequence::from_hex_str(hex);
+        assert!(result.is_err());
+    }
 }
 
 #[cfg(bench)]

bitcoin/src/lib.rs

@@ -107,6 +107,7 @@ pub mod pow;
 pub mod psbt;
 pub mod sighash;
 pub mod sign_message;
+pub mod string;
 pub mod taproot;
 pub mod util;
 

bitcoin/src/parse.rs

@@ -84,6 +84,15 @@ pub(crate) fn int<T: Integer, S: AsRef<str> + Into<String>>(s: S) -> Result<T, P
     })
 }
 
+pub(crate) fn hex_u32<S: AsRef<str> + Into<String>>(s: S) -> Result<u32, ParseIntError> {
+    u32::from_str_radix(s.as_ref(), 16).map_err(|error| ParseIntError {
+        input: s.into(),
+        bits: u8::try_from(core::mem::size_of::<u32>() * 8).expect("max is 32 bits for u32"),
+        is_signed: u32::try_from(-1i8).is_ok(),
+        source: error,
+    })
+}
+
 impl_std_error!(ParseIntError, source);
 
 /// Implements `TryFrom<$from> for $to` using `parse::int`, mapping the output using `fn`

bitcoin/src/pow.rs

@@ -15,6 +15,8 @@ use crate::consensus::encode::{self, Decodable, Encodable};
 use crate::consensus::Params;
 use crate::hash_types::BlockHash;
 use crate::io::{self, Read, Write};
+use crate::prelude::String;
+use crate::string::FromHexStr;
 
 /// Implements $int * $ty. Requires `u64::from($int)`.
 macro_rules! impl_int_mul {
@@ -283,6 +285,15 @@ impl From<CompactTarget> for Target {
     fn from(c: CompactTarget) -> Self { Target::from_compact(c) }
 }
 
+impl FromHexStr for CompactTarget {
+    type Error = crate::parse::ParseIntError;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let compact_target = crate::parse::hex_u32(s)?;
+        Ok(Self::from_consensus(compact_target))
+    }
+}
+
 impl Encodable for CompactTarget {
     #[inline]
     fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
@@ -1403,6 +1414,27 @@ mod tests {
         .is_err()); // invalid length
     }
 
+    #[test]
+    fn compact_target_from_hex_str_happy_path() {
+        let actual = CompactTarget::from_hex_str("0x01003456").unwrap();
+        let expected = CompactTarget(0x01003456);
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn compact_target_from_hex_str_no_prefix_happy_path() {
+        let actual = CompactTarget::from_hex_str_no_prefix("01003456").unwrap();
+        let expected = CompactTarget(0x01003456);
+        assert_eq!(actual, expected);
+    }
+
+    #[test]
+    fn compact_target_from_hex_invalid_hex_should_err() {
+        let hex = "0xzbf9";
+        let result = CompactTarget::from_hex_str(hex);
+        assert!(result.is_err());
+    }
+
     #[test]
     fn target_from_compact() {
         // (nBits, target)

bitcoin/src/string.rs

@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Bitcoin string parsing utilities.
+//!
+//! This module provides utility types and traits
+//! to support handling and parsing strings within `rust-bitcoin`.
+
+use core::fmt;
+
+use bitcoin_internals::write_err;
+
+use crate::prelude::String;
+
+/// Trait that allows types to be initialized from hex strings
+pub trait FromHexStr: Sized {
+    /// An error occurred while parsing the hex string.
+    type Error;
+
+    /// Parses provided string as hex requiring 0x prefix.
+    ///
+    /// This is intended for user-supplied inputs or already-existing protocols in which 0x prefix is used.
+    fn from_hex_str<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, FromHexError<Self::Error>> {
+        if !s.as_ref().starts_with("0x") {
+            Err(FromHexError::MissingPrefix(s.into()))
+        } else {
+            Ok(Self::from_hex_str_no_prefix(s.as_ref().trim_start_matches("0x"))?)
+        }
+    }
+
+    /// Parses provided string as hex without requiring 0x prefix.
+    ///
+    /// This is **not** recommended for user-supplied inputs because of possible confusion with decimals.
+    /// It should be only used for existing protocols which always encode values as hex without 0x prefix.
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error>;
+}
+
+/// Hex parsing error
+#[derive(Debug, Eq, PartialEq, Clone)]
+pub enum FromHexError<E> {
+    /// The input was not a valid hex string, contains the error that occurred while parsing.
+    ParseHex(E),
+    /// The input is missing `0x` prefix, contains the invalid input.
+    MissingPrefix(String),
+}
+
+impl<E> From<E> for FromHexError<E> {
+    fn from(e: E) -> Self { FromHexError::ParseHex(e) }
+}
+
+impl<E: fmt::Display> fmt::Display for FromHexError<E>
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use self::FromHexError::*;
+
+        match *self {
+            ParseHex(ref e) => write_err!(f, "failed to parse hex string"; e),
+            MissingPrefix(ref value) => write_err!(f, "the input value `{}` is missing the `0x` prefix", value; self),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl<E> std::error::Error for FromHexError<E>
+where
+    E: std::error::Error + 'static,
+{
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::FromHexError::*;
+
+        match *self {
+            ParseHex(ref e) => Some(e),
+            MissingPrefix(_) => None,
+        }
+    }
+}