From 026537807fa16488024ab3b44c7297c709e5364b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 21 Feb 2024 14:53:10 +1100 Subject: [PATCH 1/3] Remove mention of packed We removed the `PackedLockTime`, remove all mentions of the word packed. --- bitcoin/src/blockdata/locktime/absolute.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bitcoin/src/blockdata/locktime/absolute.rs b/bitcoin/src/blockdata/locktime/absolute.rs index 96615b43..eb37de29 100644 --- a/bitcoin/src/blockdata/locktime/absolute.rs +++ b/bitcoin/src/blockdata/locktime/absolute.rs @@ -330,8 +330,8 @@ impl FromHexStr for LockTime { #[inline] fn from_hex_str_no_prefix + Into>(s: S) -> Result { - let packed_lock_time = crate::parse::hex_u32(s)?; - Ok(Self::from_consensus(packed_lock_time)) + let lock_time = crate::parse::hex_u32(s)?; + Ok(Self::from_consensus(lock_time)) } } @@ -840,20 +840,20 @@ mod tests { } #[test] - fn packed_lock_time_from_str_hex_happy_path() { + fn lock_time_from_str_hex_happy_path() { let actual = LockTime::from_hex_str("0xBA70D").unwrap(); let expected = LockTime::from_consensus(0xBA70D); assert_eq!(actual, expected); } #[test] - fn packed_lock_time_from_str_hex_no_prefix_happy_path() { + fn lock_time_from_str_hex_no_prefix_happy_path() { let lock_time = LockTime::from_hex_str_no_prefix("BA70D").unwrap(); assert_eq!(lock_time, LockTime::from_consensus(0xBA70D)); } #[test] - fn packed_lock_time_from_str_hex_invalid_hex_should_ergr() { + fn lock_time_from_str_hex_invalid_hex_should_ergr() { let hex = "0xzb93"; let result = LockTime::from_hex_str(hex); assert!(result.is_err()); From 4d762cb08c8e1d13cf715a6072792f0b96238fbc Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 21 Feb 2024 15:11:48 +1100 Subject: [PATCH 2/3] Remove the FromHexStr trait The `FromHexStr` trait is used to parse integer-like types, however we can achieve the same using inherent methods. Move the hex parsing functionality to inherent methods, keeping the same behaviour in regard to the `0x` prefix. --- bitcoin/src/blockdata/locktime/absolute.rs | 69 +++++++++----------- bitcoin/src/blockdata/transaction.rs | 26 ++++---- bitcoin/src/lib.rs | 1 - bitcoin/src/parse.rs | 13 +++- bitcoin/src/pow.rs | 26 ++++---- bitcoin/src/string.rs | 76 ---------------------- 6 files changed, 65 insertions(+), 146 deletions(-) delete mode 100644 bitcoin/src/string.rs diff --git a/bitcoin/src/blockdata/locktime/absolute.rs b/bitcoin/src/blockdata/locktime/absolute.rs index eb37de29..fa7be4ef 100644 --- a/bitcoin/src/blockdata/locktime/absolute.rs +++ b/bitcoin/src/blockdata/locktime/absolute.rs @@ -18,9 +18,8 @@ use mutagen::mutate; use crate::absolute; use crate::consensus::encode::{self, Decodable, Encodable}; use crate::error::ParseIntError; -use crate::parse::{impl_parse_str, impl_parse_str_from_int_infallible}; +use crate::parse::{self, impl_parse_str, impl_parse_str_from_int_infallible}; use crate::prelude::*; -use crate::string::FromHexStr; /// The Threshold for deciding whether a lock time value is a height or a time (see [Bitcoin Core]). /// @@ -102,6 +101,14 @@ 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 { + let lock_time = parse::hex_u32(s)?; + Ok(Self::from_consensus(lock_time)) + } + /// Constructs a `LockTime` from an nLockTime value or the argument to OP_CHEKCLOCKTIMEVERIFY. /// /// # Examples @@ -325,16 +332,6 @@ impl fmt::Display for LockTime { } } -impl FromHexStr for LockTime { - type Error = ParseIntError; - - #[inline] - fn from_hex_str_no_prefix + Into>(s: S) -> Result { - let lock_time = crate::parse::hex_u32(s)?; - Ok(Self::from_consensus(lock_time)) - } -} - impl Encodable for LockTime { #[inline] fn consensus_encode(&self, w: &mut W) -> Result { @@ -419,6 +416,11 @@ impl Height { /// The maximum absolute block height. pub const MAX: Self = Height(LOCK_TIME_THRESHOLD - 1); + /// Creates a `Height` 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 { parse_hex(s, Self::from_consensus) } + /// Constructs a new block height. /// /// # Errors @@ -460,15 +462,6 @@ impl fmt::Display for Height { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } } -impl FromHexStr for Height { - type Error = ParseHeightError; - - #[inline] - fn from_hex_str_no_prefix + Into>(s: S) -> Result { - parse_hex(s, Self::from_consensus) - } -} - impl_parse_str!(Height, ParseHeightError, parser(Height::from_consensus)); /// Error returned when parsing block height fails. @@ -508,6 +501,11 @@ impl Time { /// The maximum absolute block time (Sun Feb 07 2106 06:28:15 GMT+0000). pub const MAX: Self = Time(u32::max_value()); + /// Creates a `Time` 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 { parse_hex(s, Self::from_consensus) } + /// Constructs a new block time. /// /// # Errors @@ -549,15 +547,6 @@ impl fmt::Display for Time { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } } -impl FromHexStr for Time { - type Error = ParseTimeError; - - #[inline] - fn from_hex_str_no_prefix + Into>(s: S) -> Result { - parse_hex(s, Self::from_consensus) - } -} - impl_parse_str!(Time, ParseTimeError, parser(Time::from_consensus)); /// Error returned when parsing block time fails. @@ -599,7 +588,7 @@ where S: AsRef + Into, F: FnOnce(u32) -> Result, { - let n = i64::from_str_radix(s.as_ref(), 16).map_err(ParseError::invalid_int(s))?; + let n = i64::from_str_radix(parse::strip_hex_prefix(s.as_ref()), 16).map_err(ParseError::invalid_int(s))?; let n = u32::try_from(n).map_err(|_| ParseError::Conversion(n))?; f(n).map_err(ParseError::from).map_err(Into::into) } @@ -821,61 +810,61 @@ mod tests { #[test] fn time_from_str_hex_happy_path() { - let actual = Time::from_hex_str("0x6289C350").unwrap(); + let actual = Time::from_hex("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(); + let time = Time::from_hex("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); + let result = Time::from_hex(hex); assert!(result.is_err()); } #[test] fn lock_time_from_str_hex_happy_path() { - let actual = LockTime::from_hex_str("0xBA70D").unwrap(); + 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_str_no_prefix("BA70D").unwrap(); + 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_str(hex); + let result = LockTime::from_hex(hex); assert!(result.is_err()); } #[test] fn height_from_str_hex_happy_path() { - let actual = Height::from_hex_str("0xBA70D").unwrap(); + let actual = Height::from_hex("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(); + let height = Height::from_hex("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); + let result = Height::from_hex(hex); assert!(result.is_err()); } diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 7674824c..c0a7c023 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -25,10 +25,9 @@ use crate::blockdata::witness::Witness; use crate::blockdata::FeeRate; use crate::consensus::{encode, Decodable, Encodable}; use crate::internal_macros::{impl_consensus_encoding, impl_hashencode}; -use crate::parse::impl_parse_str_from_int_infallible; +use crate::parse::{self, impl_parse_str_from_int_infallible, ParseIntError}; #[cfg(doc)] use crate::sighash::{EcdsaSighashType, TapSighashType}; -use crate::string::FromHexStr; use crate::prelude::*; use crate::{Amount, SignedAmount, VarInt}; @@ -402,6 +401,14 @@ 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 { + let lock_time = parse::hex_u32(s)?; + Ok(Self::from_consensus(lock_time)) + } + /// Creates a relative lock-time using block height. #[inline] pub fn from_height(height: u16) -> Self { Sequence(u32::from(height)) } @@ -473,15 +480,6 @@ impl Sequence { fn low_u16(&self) -> u16 { self.0 as u16 } } -impl FromHexStr for Sequence { - type Error = crate::parse::ParseIntError; - - fn from_hex_str_no_prefix + Into>(s: S) -> Result { - 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 { Sequence::MAX } @@ -2120,20 +2118,20 @@ mod tests { #[test] fn sequence_from_str_hex_happy_path() { - let sequence = Sequence::from_hex_str("0xFFFFFFFF").unwrap(); + 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_str_no_prefix("FFFFFFFF").unwrap(); + let sequence = Sequence::from_hex("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); + let result = Sequence::from_hex(hex); assert!(result.is_err()); } diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index c9f92e6b..23524672 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -108,7 +108,6 @@ pub mod policy; pub mod pow; pub mod psbt; pub mod sign_message; -pub mod string; pub mod taproot; #[rustfmt::skip] // Keep public re-exports separate. diff --git a/bitcoin/src/parse.rs b/bitcoin/src/parse.rs index 35eb4764..f8c40400 100644 --- a/bitcoin/src/parse.rs +++ b/bitcoin/src/parse.rs @@ -90,8 +90,19 @@ pub(crate) fn int + Into>(s: S) -> Result &str { + if let Some(stripped) = s.strip_prefix("0x") { + stripped + } else if let Some(stripped) = s.strip_prefix("0X") { + stripped + } else { + s + } +} + pub(crate) fn hex_u32 + Into>(s: S) -> Result { - u32::from_str_radix(s.as_ref(), 16).map_err(|error| ParseIntError { + u32::from_str_radix(strip_hex_prefix(s.as_ref()), 16).map_err(|error| ParseIntError { input: s.into(), bits: u8::try_from(core::mem::size_of::() * 8).expect("max is 32 bits for u32"), is_signed: u32::try_from(-1i8).is_ok(), diff --git a/bitcoin/src/pow.rs b/bitcoin/src/pow.rs index e6870677..e098cbb7 100644 --- a/bitcoin/src/pow.rs +++ b/bitcoin/src/pow.rs @@ -17,8 +17,7 @@ use crate::blockdata::block::BlockHash; use crate::consensus::encode::{self, Decodable, Encodable}; #[cfg(doc)] use crate::consensus::Params; -use crate::string::FromHexStr; -use crate::prelude::*; +use crate::parse::{self, ParseIntError}; use crate::Network; /// Implement traits and methods shared by `Target` and `Work`. @@ -272,6 +271,14 @@ 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 { + let lock_time = parse::hex_u32(s)?; + Ok(Self::from_consensus(lock_time)) + } + /// Creates a [`CompactTarget`] from a consensus encoded `u32`. pub fn from_consensus(bits: u32) -> Self { Self(bits) } @@ -283,15 +290,6 @@ impl From 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 + Into>(s: S) -> Result { - let compact_target = crate::parse::hex_u32(s)?; - Ok(Self::from_consensus(compact_target)) - } -} - impl Encodable for CompactTarget { #[inline] fn consensus_encode(&self, w: &mut W) -> Result { @@ -1525,14 +1523,14 @@ mod tests { #[test] fn compact_target_from_hex_str_happy_path() { - let actual = CompactTarget::from_hex_str("0x01003456").unwrap(); + let actual = CompactTarget::from_hex("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 actual = CompactTarget::from_hex("01003456").unwrap(); let expected = CompactTarget(0x01003456); assert_eq!(actual, expected); } @@ -1540,7 +1538,7 @@ mod tests { #[test] fn compact_target_from_hex_invalid_hex_should_err() { let hex = "0xzbf9"; - let result = CompactTarget::from_hex_str(hex); + let result = CompactTarget::from_hex(hex); assert!(result.is_err()); } diff --git a/bitcoin/src/string.rs b/bitcoin/src/string.rs deleted file mode 100644 index c8bad0ba..00000000 --- a/bitcoin/src/string.rs +++ /dev/null @@ -1,76 +0,0 @@ -// 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 internals::write_err; - -use crate::prelude::*; - -/// 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 + Into>(s: S) -> Result> { - 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 + Into>(s: S) -> Result; -} - -/// Hex parsing error -#[derive(Debug, Clone, PartialEq, Eq)] -#[non_exhaustive] -pub enum FromHexError { - /// 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 From for FromHexError { - fn from(e: E) -> Self { FromHexError::ParseHex(e) } -} - -impl fmt::Display for FromHexError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use 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")] -impl std::error::Error for FromHexError -where - E: std::error::Error + 'static, -{ - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use FromHexError::*; - - match *self { - ParseHex(ref e) => Some(e), - MissingPrefix(_) => None, - } - } -} From b873a3cd44a34bc24b2971cb97bff6a3a8060ebb Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Feb 2024 10:03:58 +1100 Subject: [PATCH 3/3] 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 | 76 ++++++++----- bitcoin/src/blockdata/transaction.rs | 46 ++++++-- bitcoin/src/error.rs | 125 +++++++++++++++++++++ bitcoin/src/pow.rs | 53 ++++++--- 4 files changed, 252 insertions(+), 48 deletions(-) diff --git a/bitcoin/src/blockdata/locktime/absolute.rs b/bitcoin/src/blockdata/locktime/absolute.rs index fa7be4ef..ec1451a0 100644 --- a/bitcoin/src/blockdata/locktime/absolute.rs +++ b/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 { + /// Creates a `LockTime` from an prefixed hex string. + pub fn from_hex(s: &str) -> Result { + 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 { + 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(); diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index c0a7c023..94e3131a 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/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 { + /// Creates a `Sequence` from an prefixed hex string. + pub fn from_hex(s: &str) -> Result { + 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 { + 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); } diff --git a/bitcoin/src/error.rs b/bitcoin/src/error.rs index 6adcbbb0..da9d6074 100644 --- a/bitcoin/src/error.rs +++ b/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 for PrefixedHexError { + fn from(e: MissingPrefixError) -> Self { Self::MissingPrefix(e) } +} + +impl From 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 for UnprefixedHexError { + fn from(e: ContainsPrefixError) -> Self { Self::ContainsPrefix(e) } +} + +impl From 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 {} diff --git a/bitcoin/src/pow.rs b/bitcoin/src/pow.rs index e098cbb7..3053d077 100644 --- a/bitcoin/src/pow.rs +++ b/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 { + /// Creates a `CompactTarget` from an prefixed hex string. + pub fn from_hex(s: &str) -> Result { + 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 { + 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]