units: Change hex parsing on Height and MedianTimePast
What the developer wrote
Authored by Mitchell Bagot
units: Change hex parsing on Height and MedianTimePast
Height and MedianTimePast both have an existing from_hex function that
parses from an optionally prefixed hex string. In order to match the
other integer wrapper types in units, this should be split to from_hex
and from_unprefixed_hex.
Change from_hex to only parse prefixed hex in Height and
MedianTimePast. Introduce from_unprefixed_hex in Height and
MedianTimePast to parse unprefixed hex. Adjust ParseTimeError and
ParseHeightError accordingly.
What changed, and why it matters
This commit changes how two Bitcoin locktime types (block height and median-time-past) parse hexadecimal strings. Previously, a single function accepted hex with or without a '0x' prefix. Now there are two separate functions: one that requires the '0x' prefix and one that rejects it. This is an API-consistency cleanup, not a fix for an exploitable vulnerability. Existing callers that relied on the old flexible behavior could break at compile time or runtime if they still pass unprefixed hex to from_hex, but the change is intentional and documented.
Treat as a normal API-breaking refactor. Downstream users should audit calls to Height::from_hex and MedianTimePast::from_hex: any caller passing unprefixed hex must switch to from_unprefixed_hex. No urgent security patch is indicated by the diff alone.
Security signals we found
API behavior change: from_hex now rejects unprefixed hex strings
New from_unprefixed_hex function introduced for explicit unprefixed parsing
Error type expanded with prefixed/unprefixed hex error variants
No bounds-checking or overflow weakness visible in the new parsing path
No vendor security disclosure or CVE references present in commit or supplied materials
Evidence from the diff
The patch refactors Height::from_hex and MedianTimePast::from_hex in rust-bitcoin/units to match other integer wrapper types. It replaces the shared parse_hex helper (which used parse_int::hex_remove_optional_prefix and i64::from_str_radix) with explicit parse_int::hex_u32_prefixed and parse_int::hex_u32_unprefixed calls. New from_unprefixed_hex methods are added, and from_hex now requires a ‘0x’ prefix. Error variants PrefixedHexError and UnprefixedHexError are added to ParseError. Tests are updated to call from_unprefixed_hex for unprefixed inputs. No memory-safety bug, overflow, or cryptographic issue is evident in the diff.
Changed components
units/src/locktime/absolute/mod.rsunits/src/locktime/absolute/error.rsHeight::from_hexHeight::from_unprefixed_hexMedianTimePast::from_hexMedianTimePast::from_unprefixed_hexParseErrorInspect captured patch +70 / −23
diff --git a/units/src/locktime/absolute/error.rs b/units/src/locktime/absolute/error.rs
index fa266e49..6b5e35ff 100644
--- a/units/src/locktime/absolute/error.rs
+++ b/units/src/locktime/absolute/error.rs
@@ -10,7 +10,7 @@ use internals::error::InputString;
use internals::write_err;
use super::{Height, MedianTimePast, LOCK_TIME_THRESHOLD};
-use crate::parse_int::ParseIntError;
+use crate::parse_int::{ParseIntError, PrefixedHexError, UnprefixedHexError};
/// An error consensus decoding an `LockTime`.
#[cfg(feature = "encoding")]
@@ -139,6 +139,11 @@ impl From<ParseError> for ParseTimeError {
/// Internal - common representation for height and time.
#[derive(Debug, Clone, Eq, PartialEq)]
pub(super) enum ParseError {
+ /// Error parsing prefixed hex
+ PrefixedHex(PrefixedHexError),
+ /// Error parsing unprefixed hex
+ UnprefixedHex(UnprefixedHexError),
+ // Error parsing decimal
ParseInt(ParseIntError),
// unit implied by outer type
// we use i64 to have nicer messages for negative values
@@ -168,6 +173,12 @@ impl ParseError {
use core::num::IntErrorKind;
match self {
+ Self::PrefixedHex(ref err) => {
+ fmt::Display::fmt(err, f)
+ },
+ Self::UnprefixedHex(ref err) => {
+ fmt::Display::fmt(err, f)
+ },
Self::ParseInt(ParseIntError { input, bits: _, is_signed: _, source })
if *source.kind() == IntErrorKind::PosOverflow =>
{
@@ -215,6 +226,8 @@ impl ParseError {
use core::num::IntErrorKind;
match self {
+ Self::PrefixedHex(ref err) => Some(err),
+ Self::UnprefixedHex(ref err) => Some(err),
Self::ParseInt(ParseIntError { source, .. })
if *source.kind() == IntErrorKind::PosOverflow =>
None,
@@ -231,6 +244,14 @@ impl From<ConversionError> for ParseError {
fn from(value: ConversionError) -> Self { Self::Conversion(value.input.into()) }
}
+impl From<PrefixedHexError> for ParseError {
+ fn from(value: PrefixedHexError) -> Self { Self::PrefixedHex(value) }
+}
+
+impl From<UnprefixedHexError> for ParseError {
+ fn from(value: UnprefixedHexError) -> Self { Self::UnprefixedHex(value) }
+}
+
/// Error returned when converting a `u32` to a lock time variant fails.
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
diff --git a/units/src/locktime/absolute/mod.rs b/units/src/locktime/absolute/mod.rs
index 084883a1..00517ca5 100644
--- a/units/src/locktime/absolute/mod.rs
+++ b/units/src/locktime/absolute/mod.rs
@@ -524,14 +524,33 @@ impl Height {
/// The maximum absolute block height.
pub const MAX: Self = Self(LOCK_TIME_THRESHOLD - 1);
- /// Constructs a new [`Height`] from a hex string.
+ /// Constructs a new [`Height`] from a prefixed hex string.
///
- /// The input string may or may not contain a typical hex prefix e.g., `0x`.
+ /// # Errors
+ ///
+ /// If the input string is not a valid hex representation of a block height or it does not
+ /// include the `0x` prefix.
+ #[inline]
+ pub fn from_hex(s: &str) -> Result<Self, ParseHeightError> {
+ let height = parse_int::hex_u32_prefixed(s)
+ .map_err(ParseError::PrefixedHex)?;
+ Ok(Self::from_u32(height)
+ .map_err(|_| ParseError::Conversion(height.into()))?)
+ }
+
+ /// Constructs a new [`Height`] from an unprefixed hex string.
///
/// # Errors
///
- /// If the input string is not a valid hex representation of a block height.
- pub fn from_hex(s: &str) -> Result<Self, ParseHeightError> { parse_hex(s, Self::from_u32) }
+ /// If the input string is not a valid hex representation of a block height or if it
+ /// includes the `0x` prefix.
+ #[inline]
+ pub fn from_unprefixed_hex(s: &str) -> Result<Self, ParseHeightError> {
+ let height = parse_int::hex_u32_unprefixed(s)
+ .map_err(ParseError::UnprefixedHex)?;
+ Ok(Self::from_u32(height)
+ .map_err(|_| ParseError::Conversion(height.into()))?)
+ }
#[deprecated(since = "1.0.0-rc.0", note = "use `from_u32` instead")]
#[doc(hidden)]
@@ -638,14 +657,33 @@ impl MedianTimePast {
crate::BlockMtp::new(timestamps).try_into()
}
- /// Constructs a new [`MedianTimePast`] from a big-endian hex-encoded `u32`.
+ /// Constructs a new [`MedianTimePast`] from a prefixed hex string.
///
- /// The input string may or may not contain a typical hex prefix e.g., `0x`.
+ /// # Errors
+ ///
+ /// If the input string is not a valid hex representation of a block time or it does not
+ /// include the `0x` prefix.
+ #[inline]
+ pub fn from_hex(s: &str) -> Result<Self, ParseTimeError> {
+ let height = parse_int::hex_u32_prefixed(s)
+ .map_err(ParseError::PrefixedHex)?;
+ Ok(Self::from_u32(height)
+ .map_err(|_| ParseError::Conversion(height.into()))?)
+ }
+
+ /// Constructs a new [`MedianTimePast`] from an unprefixed hex string.
///
/// # Errors
///
- /// If the input string is not a valid hex representation of a block time.
- pub fn from_hex(s: &str) -> Result<Self, ParseTimeError> { parse_hex(s, Self::from_u32) }
+ /// If the input string is not a valid hex representation of a block time or if it
+ /// includes the `0x` prefix.
+ #[inline]
+ pub fn from_unprefixed_hex(s: &str) -> Result<Self, ParseTimeError> {
+ let height = parse_int::hex_u32_unprefixed(s)
+ .map_err(ParseError::UnprefixedHex)?;
+ Ok(Self::from_u32(height)
+ .map_err(|_| ParseError::Conversion(height.into()))?)
+ }
#[deprecated(since = "1.0.0-rc.0", note = "use `from_u32` instead")]
#[doc(hidden)]
@@ -729,18 +767,6 @@ where
}
}
-fn parse_hex<T, E, S, F>(s: S, f: F) -> Result<T, E>
-where
- E: From<ParseError>,
- S: AsRef<str> + Into<InputString>,
- F: FnOnce(u32) -> Result<T, ConversionError>,
-{
- let n = i64::from_str_radix(parse_int::hex_remove_optional_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)
-}
-
/// Returns true if `n` is a block height i.e., less than 500,000,000.
pub const fn is_block_height(n: u32) -> bool { n < LOCK_TIME_THRESHOLD }
@@ -946,7 +972,7 @@ mod tests {
#[test]
fn time_from_str_hex_no_prefix_happy_path() {
- let time = MedianTimePast::from_hex("6289C350").unwrap();
+ let time = MedianTimePast::from_unprefixed_hex("6289C350").unwrap();
assert_eq!(time, MedianTimePast(0x6289_C350));
}
@@ -966,7 +992,7 @@ mod tests {
#[test]
fn height_from_str_hex_no_prefix_happy_path() {
- let height = Height::from_hex("BA70D").unwrap();
+ let height = Height::from_unprefixed_hex("BA70D").unwrap();
assert_eq!(height, Height(0xBA70D));
}
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.