Do infallible int from hex conversions

We have three integer wrapping types that can be created from hex
strings where the conversion from an integer is infallible:

- `absolute::LockTime`
- `Sequence`
- `CompactTarget`

We would like to improve our handling of the two prefix characters (eg
0x) by making it explicit.

- Modify the inherent `from_hex` method on each type to error if the
input string does not contain a prefix.

- Add an additional inherent method on each type `from_unprefixed_hex`
that errors if the input string does contain a prefix.

This patch does not touch the wrapper types that cannot be infallibly
constructed from an integer (i.e. absolute `Height` and `Time`).

bitcoin/src/blockdata/locktime/absolute.rs

@@ -17,7 +17,7 @@ use mutagen::mutate;
 #[cfg(doc)]
 use crate::absolute;
 use crate::consensus::encode::{self, Decodable, Encodable};
-use crate::error::ParseIntError;
+use crate::error::{ParseIntError, PrefixedHexError, UnprefixedHexError, ContainsPrefixError, MissingPrefixError};
 use crate::parse::{self, impl_parse_str, impl_parse_str_from_int_infallible};
 use crate::prelude::*;
 
@@ -101,10 +101,25 @@ impl LockTime {
     /// The number of bytes that the locktime contributes to the size of a transaction.
     pub const SIZE: usize = 4; // Serialized length of a u32.
 
-    /// Creates a `LockTime` from a hex string.
-    ///
-    /// The input string is may or may not contain a typical hex prefix e.g., `0x`.
-    pub fn from_hex(s: &str) -> Result<Self, ParseIntError> {
+    /// Creates a `LockTime` from an prefixed hex string.
+    pub fn from_hex(s: &str) -> Result<Self, PrefixedHexError> {
+        let stripped = if let Some(stripped) = s.strip_prefix("0x") {
+            stripped
+        } else if let Some(stripped) = s.strip_prefix("0X") {
+            stripped
+        } else {
+            return Err(MissingPrefixError::new(s).into());
+        };
+
+        let lock_time = parse::hex_u32(stripped)?;
+        Ok(Self::from_consensus(lock_time))
+    }
+
+    /// Creates a `LockTime` from an unprefixed hex string.
+    pub fn from_unprefixed_hex(s: &str) -> Result<Self, UnprefixedHexError> {
+        if s.starts_with("0x") || s.starts_with("0X") {
+            return Err(ContainsPrefixError::new(s).into());
+        }
         let lock_time = parse::hex_u32(s)?;
         Ok(Self::from_consensus(lock_time))
     }
@@ -808,6 +823,37 @@ mod tests {
         assert_eq!(got, "block-height 741521");
     }
 
+    #[test]
+    fn lock_time_from_hex_lower() {
+        let lock = LockTime::from_hex("0x6289c350").unwrap();
+        assert_eq!(lock, LockTime::from_consensus(0x6289C350));
+    }
+
+    #[test]
+    fn lock_time_from_hex_upper() {
+        let lock = LockTime::from_hex("0X6289C350").unwrap();
+        assert_eq!(lock, LockTime::from_consensus(0x6289C350));
+    }
+
+    #[test]
+    fn lock_time_from_unprefixed_hex_lower() {
+        let lock = LockTime::from_unprefixed_hex("6289c350").unwrap();
+        assert_eq!(lock, LockTime::from_consensus(0x6289C350));
+    }
+
+    #[test]
+    fn lock_time_from_unprefixed_hex_upper() {
+        let lock = LockTime::from_unprefixed_hex("6289C350").unwrap();
+        assert_eq!(lock, LockTime::from_consensus(0x6289C350));
+    }
+
+    #[test]
+    fn lock_time_from_invalid_hex_should_err() {
+        let hex = "0xzb93";
+        let result = LockTime::from_hex(hex);
+        assert!(result.is_err());
+    }
+
     #[test]
     fn time_from_str_hex_happy_path() {
         let actual = Time::from_hex("0x6289C350").unwrap();
@@ -828,26 +874,6 @@ mod tests {
         assert!(result.is_err());
     }
 
-    #[test]
-    fn lock_time_from_str_hex_happy_path() {
-        let actual = LockTime::from_hex("0xBA70D").unwrap();
-        let expected = LockTime::from_consensus(0xBA70D);
-        assert_eq!(actual, expected);
-    }
-
-    #[test]
-    fn lock_time_from_str_hex_no_prefix_happy_path() {
-        let lock_time = LockTime::from_hex("BA70D").unwrap();
-        assert_eq!(lock_time, LockTime::from_consensus(0xBA70D));
-    }
-
-    #[test]
-    fn lock_time_from_str_hex_invalid_hex_should_ergr() {
-        let hex = "0xzb93";
-        let result = LockTime::from_hex(hex);
-        assert!(result.is_err());
-    }
-
     #[test]
     fn height_from_str_hex_happy_path() {
         let actual = Height::from_hex("0xBA70D").unwrap();

bitcoin/src/blockdata/transaction.rs

@@ -24,8 +24,9 @@ use crate::blockdata::script::{Script, ScriptBuf};
 use crate::blockdata::witness::Witness;
 use crate::blockdata::FeeRate;
 use crate::consensus::{encode, Decodable, Encodable};
+use crate::error::{PrefixedHexError, UnprefixedHexError, ContainsPrefixError, MissingPrefixError};
 use crate::internal_macros::{impl_consensus_encoding, impl_hashencode};
-use crate::parse::{self, impl_parse_str_from_int_infallible, ParseIntError};
+use crate::parse::{self, impl_parse_str_from_int_infallible};
 #[cfg(doc)]
 use crate::sighash::{EcdsaSighashType, TapSighashType};
 use crate::prelude::*;
@@ -401,10 +402,25 @@ impl Sequence {
         self.is_relative_lock_time() & (self.0 & Sequence::LOCK_TYPE_MASK > 0)
     }
 
-    /// Creates a `Sequence` number from a hex string.
-    ///
-    /// The input string may or may not contain a typical hex prefix e.g., `0x`.
-    pub fn from_hex(s: &str) -> Result<Self, ParseIntError> {
+    /// Creates a `Sequence` from an prefixed hex string.
+    pub fn from_hex(s: &str) -> Result<Self, PrefixedHexError> {
+        let stripped = if let Some(stripped) = s.strip_prefix("0x") {
+            stripped
+        } else if let Some(stripped) = s.strip_prefix("0X") {
+            stripped
+        } else {
+            return Err(MissingPrefixError::new(s).into());
+        };
+
+        let sequence = parse::hex_u32(stripped)?;
+        Ok(Self::from_consensus(sequence))
+    }
+
+    /// Creates a `Sequence` from an unprefixed hex string.
+    pub fn from_unprefixed_hex(s: &str) -> Result<Self, UnprefixedHexError> {
+        if s.starts_with("0x") || s.starts_with("0X") {
+            return Err(ContainsPrefixError::new(s).into());
+        }
         let lock_time = parse::hex_u32(s)?;
         Ok(Self::from_consensus(lock_time))
     }
@@ -2117,14 +2133,26 @@ mod tests {
     }
 
     #[test]
-    fn sequence_from_str_hex_happy_path() {
-        let sequence = Sequence::from_hex("0xFFFFFFFF").unwrap();
+    fn sequence_from_hex_lower() {
+        let sequence = Sequence::from_hex("0xffffffff").unwrap();
         assert_eq!(sequence, Sequence::MAX);
     }
 
     #[test]
-    fn sequence_from_str_hex_no_prefix_happy_path() {
-        let sequence = Sequence::from_hex("FFFFFFFF").unwrap();
+    fn sequence_from_hex_upper() {
+        let sequence = Sequence::from_hex("0XFFFFFFFF").unwrap();
+        assert_eq!(sequence, Sequence::MAX);
+    }
+
+    #[test]
+    fn sequence_from_unprefixed_hex_lower() {
+        let sequence = Sequence::from_unprefixed_hex("ffffffff").unwrap();
+        assert_eq!(sequence, Sequence::MAX);
+    }
+
+    #[test]
+    fn sequence_from_unprefixed_hex_upper() {
+        let sequence = Sequence::from_unprefixed_hex("FFFFFFFF").unwrap();
         assert_eq!(sequence, Sequence::MAX);
     }
 

bitcoin/src/error.rs

@@ -2,6 +2,131 @@
 
 //! Contains error types and other error handling tools.
 
+use core::fmt;
+
+use internals::write_err;
+
+use crate::prelude::*;
+
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
 pub use crate::parse::ParseIntError;
+
+/// Error returned when parsing integer from an supposedly prefixed hex string for
+/// a type that can be created infallibly from an integer.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum PrefixedHexError {
+    /// Hex string is missing prefix.
+    MissingPrefix(MissingPrefixError),
+    /// Error parsing integer from hex string.
+    ParseInt(ParseIntError),
+}
+
+impl fmt::Display for PrefixedHexError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use PrefixedHexError::*;
+
+        match *self {
+            MissingPrefix(ref e) => write_err!(f, "hex string is missing prefix"; e),
+            ParseInt(ref e) => write_err!(f, "prefixed hex string invalid int"; e),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PrefixedHexError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use PrefixedHexError::*;
+
+        match *self {
+            MissingPrefix(ref e) => Some(e),
+            ParseInt(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<MissingPrefixError> for PrefixedHexError {
+    fn from(e: MissingPrefixError) -> Self { Self::MissingPrefix(e) }
+}
+
+impl From<ParseIntError> for PrefixedHexError {
+    fn from(e: ParseIntError) -> Self { Self::ParseInt(e) }
+}
+
+/// Error returned when parsing integer from an supposedly un-prefixed hex string.
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum UnprefixedHexError {
+    /// Hex string contains prefix.
+    ContainsPrefix(ContainsPrefixError),
+    /// Error parsing integer from string.
+    ParseInt(ParseIntError),
+}
+
+impl fmt::Display for UnprefixedHexError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use UnprefixedHexError::*;
+
+        match *self {
+            ContainsPrefix(ref e) => write_err!(f, "hex string is contains prefix"; e),
+            ParseInt(ref e) => write_err!(f, "hex string parse int"; e),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for UnprefixedHexError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use UnprefixedHexError::*;
+
+        match *self {
+            ContainsPrefix(ref e) => Some(e),
+            ParseInt(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<ContainsPrefixError> for UnprefixedHexError {
+    fn from(e: ContainsPrefixError) -> Self { Self::ContainsPrefix(e) }
+}
+
+impl From<ParseIntError> for UnprefixedHexError {
+    fn from(e: ParseIntError) -> Self { Self::ParseInt(e) }
+}
+
+/// Error when hex string is missing a prefix (e.g. 0x).
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct MissingPrefixError {
+    hex: String,
+}
+
+impl MissingPrefixError {
+    pub(crate) fn new(s: &str) -> Self { Self { hex: s.into() } }
+}
+
+impl fmt::Display for MissingPrefixError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "hex string is missing a prefix (e.g. 0x): {}", self.hex)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for MissingPrefixError {}
+
+/// Error when hex string contains a prefix (e.g. 0x).
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct ContainsPrefixError {
+    hex: String,
+}
+
+impl ContainsPrefixError {
+    pub(crate) fn new(s: &str) -> Self { Self { hex: s.into() } }
+}
+
+impl fmt::Display for ContainsPrefixError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "hex string contains a prefix: {}", self.hex)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for ContainsPrefixError {}

bitcoin/src/pow.rs

@@ -17,8 +17,8 @@ use crate::blockdata::block::BlockHash;
 use crate::consensus::encode::{self, Decodable, Encodable};
 #[cfg(doc)]
 use crate::consensus::Params;
-use crate::parse::{self, ParseIntError};
-use crate::Network;
+use crate::error::{PrefixedHexError, UnprefixedHexError, ContainsPrefixError, MissingPrefixError};
+use crate::{parse, Network};
 
 /// Implement traits and methods shared by `Target` and `Work`.
 macro_rules! do_impl {
@@ -271,10 +271,25 @@ do_impl!(Target);
 pub struct CompactTarget(u32);
 
 impl CompactTarget {
-    /// Creates a [`CompactTarget`] from a hex string.
-    ///
-    /// The input string is may or may not contain a typical hex prefix e.g., `0x`.
-    pub fn from_hex(s: &str) -> Result<Self, ParseIntError> {
+    /// Creates a `CompactTarget` from an prefixed hex string.
+    pub fn from_hex(s: &str) -> Result<Self, PrefixedHexError> {
+        let stripped = if let Some(stripped) = s.strip_prefix("0x") {
+            stripped
+        } else if let Some(stripped) = s.strip_prefix("0X") {
+            stripped
+        } else {
+            return Err(MissingPrefixError::new(s).into());
+        };
+
+        let target = parse::hex_u32(stripped)?;
+        Ok(Self::from_consensus(target))
+    }
+
+    /// Creates a `CompactTarget` from an unprefixed hex string.
+    pub fn from_unprefixed_hex(s: &str) -> Result<Self, UnprefixedHexError> {
+        if s.starts_with("0x") || s.starts_with("0X") {
+            return Err(ContainsPrefixError::new(s).into());
+        }
         let lock_time = parse::hex_u32(s)?;
         Ok(Self::from_consensus(lock_time))
     }
@@ -1522,17 +1537,27 @@ mod tests {
     }
 
     #[test]
-    fn compact_target_from_hex_str_happy_path() {
-        let actual = CompactTarget::from_hex("0x01003456").unwrap();
-        let expected = CompactTarget(0x01003456);
-        assert_eq!(actual, expected);
+    fn compact_target_from_hex_lower() {
+        let target = CompactTarget::from_hex("0x010034ab").unwrap();
+        assert_eq!(target, CompactTarget(0x010034ab));
     }
 
     #[test]
-    fn compact_target_from_hex_str_no_prefix_happy_path() {
-        let actual = CompactTarget::from_hex("01003456").unwrap();
-        let expected = CompactTarget(0x01003456);
-        assert_eq!(actual, expected);
+    fn compact_target_from_hex_upper() {
+        let target = CompactTarget::from_hex("0X010034AB").unwrap();
+        assert_eq!(target, CompactTarget(0x010034ab));
+    }
+
+    #[test]
+    fn compact_target_from_unprefixed_hex_lower() {
+        let target = CompactTarget::from_unprefixed_hex("010034ab").unwrap();
+        assert_eq!(target, CompactTarget(0x010034ab));
+    }
+
+    #[test]
+    fn compact_target_from_unprefixed_hex_upper() {
+        let target = CompactTarget::from_unprefixed_hex("010034AB").unwrap();
+        assert_eq!(target, CompactTarget(0x010034ab));
     }
 
     #[test]