From 39b4f7670d2cf7b05d802e0cfd9c2d69131cd81f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 2 May 2025 17:44:02 +0000 Subject: [PATCH] units: rename relative::Height to HeightInterval This is disruptive, but makes the type name consistent with `MtpInterval` and also greatly improves clarity, helping to distinguish between absolute and relative locktimes and reminding the author (and reviewer) of locktime code that this needs to be a diff. --- bitcoin/src/blockdata/block.rs | 2 +- bitcoin/src/blockdata/mod.rs | 6 ++- primitives/src/locktime/relative.rs | 61 ++++++++++++++++------------- primitives/src/sequence.rs | 8 ++-- units/src/block.rs | 31 ++++++++------- units/src/locktime/relative.rs | 44 +++++++++++---------- units/tests/api.rs | 6 ++- 7 files changed, 88 insertions(+), 70 deletions(-) diff --git a/bitcoin/src/blockdata/block.rs b/bitcoin/src/blockdata/block.rs index b88f2a197..5fd59aa31 100644 --- a/bitcoin/src/blockdata/block.rs +++ b/bitcoin/src/blockdata/block.rs @@ -30,7 +30,7 @@ use crate::transaction::{Transaction, TransactionExt as _, Wtxid}; #[doc(inline)] pub use primitives::block::{Block, Checked, Unchecked, Validation, Version, BlockHash, Header, WitnessCommitment}; #[doc(inline)] -pub use units::block::{BlockHeight, BlockInterval, TooBigForRelativeBlockHeightError}; +pub use units::block::{BlockHeight, BlockInterval, TooBigForRelativeBlockHeightIntervalError}; impl_hashencode!(BlockHash); diff --git a/bitcoin/src/blockdata/mod.rs b/bitcoin/src/blockdata/mod.rs index ee1c770f8..8255e3cc5 100644 --- a/bitcoin/src/blockdata/mod.rs +++ b/bitcoin/src/blockdata/mod.rs @@ -71,10 +71,14 @@ pub mod locktime { /// Re-export everything from the `primitives::locktime::relative` module. pub use primitives::locktime::relative::{ - DisabledLockTimeError, Height, IncompatibleHeightError, IncompatibleTimeError, + DisabledLockTimeError, HeightInterval, IncompatibleHeightError, IncompatibleTimeError, LockTime, MtpInterval, TimeOverflowError, }; + #[deprecated(since = "TBD", note = "use `Mtp` instead")] + #[doc(hidden)] + pub type Height = HeightInterval; + #[deprecated(since = "TBD", note = "use `Mtp` instead")] #[doc(hidden)] pub type Time = MtpInterval; diff --git a/primitives/src/locktime/relative.rs b/primitives/src/locktime/relative.rs index 0bfefc83f..386c7c2ed 100644 --- a/primitives/src/locktime/relative.rs +++ b/primitives/src/locktime/relative.rs @@ -13,9 +13,13 @@ use crate::{relative, TxIn}; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] -pub use units::locktime::relative::{Height, MtpInterval, TimeOverflowError}; +pub use units::locktime::relative::{HeightInterval, MtpInterval, TimeOverflowError}; use units::mtp_height::MtpAndHeight; +#[deprecated(since = "TBD", note = "use `Mtp` instead")] +#[doc(hidden)] +pub type Height = HeightInterval; + #[deprecated(since = "TBD", note = "use `Mtp` instead")] #[doc(hidden)] pub type Time = MtpInterval; @@ -74,7 +78,7 @@ pub type Time = MtpInterval; #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum LockTime { /// A block height lock time value. - Blocks(Height), + Blocks(HeightInterval), /// A 512 second time interval value. Time(MtpInterval), } @@ -82,7 +86,7 @@ pub enum LockTime { impl LockTime { /// A relative locktime of 0 is always valid, and is assumed valid for inputs that /// are not yet confirmed. - pub const ZERO: LockTime = LockTime::Blocks(Height::ZERO); + pub const ZERO: LockTime = LockTime::Blocks(HeightInterval::ZERO); /// The number of bytes that the locktime contributes to the size of a transaction. pub const SIZE: usize = 4; // Serialized length of a u32. @@ -161,7 +165,7 @@ impl LockTime { /// Constructs a new `LockTime` from `n`, expecting `n` to be a 16-bit count of blocks. #[inline] - pub const fn from_height(n: u16) -> Self { LockTime::Blocks(Height::from_height(n)) } + pub const fn from_height(n: u16) -> Self { LockTime::Blocks(HeightInterval::from_height(n)) } /// Constructs a new `LockTime` from `n`, expecting `n` to be a count of 512-second intervals. /// @@ -221,7 +225,7 @@ impl LockTime { /// # Examples /// /// ```rust - /// # use bitcoin_primitives::locktime::relative::Height; + /// # use bitcoin_primitives::locktime::relative::HeightInterval; /// # use bitcoin_primitives::relative::Time; /// # use units::mtp_height::MtpAndHeight; /// # use bitcoin_primitives::BlockHeight; @@ -325,7 +329,7 @@ impl LockTime { } } - /// Returns true if this [`relative::LockTime`] is satisfied by [`Height`]. + /// Returns true if this [`relative::LockTime`] is satisfied by [`HeightInterval`]. /// /// # Errors /// @@ -339,10 +343,13 @@ impl LockTime { /// /// let required_height: u16 = 100; /// let lock = Sequence::from_height(required_height).to_relative_lock_time().expect("valid height"); - /// assert!(lock.is_satisfied_by_height(relative::Height::from(required_height + 1)).expect("a height")); + /// assert!(lock.is_satisfied_by_height(relative::HeightInterval::from(required_height + 1)).expect("a height")); /// ``` #[inline] - pub fn is_satisfied_by_height(self, height: Height) -> Result { + pub fn is_satisfied_by_height( + self, + height: HeightInterval, + ) -> Result { use LockTime as L; match self { @@ -378,9 +385,9 @@ impl LockTime { } } -impl From for LockTime { +impl From for LockTime { #[inline] - fn from(h: Height) -> Self { LockTime::Blocks(h) } + fn from(h: HeightInterval) -> Self { LockTime::Blocks(h) } } impl From for LockTime { @@ -445,14 +452,14 @@ impl std::error::Error for DisabledLockTimeError {} #[derive(Debug, Clone, PartialEq, Eq)] pub struct IncompatibleHeightError { /// Attempted to satisfy a lock-by-blocktime lock with this height. - height: Height, + height: HeightInterval, /// The inner time value of the lock-by-blocktime lock. time: MtpInterval, } impl IncompatibleHeightError { /// Returns the height that was erroneously used to try and satisfy a lock-by-blocktime lock. - pub fn incompatible(&self) -> Height { self.height } + pub fn incompatible(&self) -> HeightInterval { self.height } /// Returns the time value of the lock-by-blocktime lock. pub fn expected(&self) -> MtpInterval { self.time } @@ -478,7 +485,7 @@ pub struct IncompatibleTimeError { /// Attempted to satisfy a lock-by-blockheight lock with this time. time: MtpInterval, /// The inner height value of the lock-by-blockheight lock. - height: Height, + height: HeightInterval, } impl IncompatibleTimeError { @@ -486,7 +493,7 @@ impl IncompatibleTimeError { pub fn incompatible(&self) -> MtpInterval { self.time } /// Returns the height value of the lock-by-blockheight lock. - pub fn expected(&self) -> Height { self.height } + pub fn expected(&self) -> HeightInterval { self.height } } impl fmt::Display for IncompatibleTimeError { @@ -541,8 +548,8 @@ mod tests { #[test] fn parses_correctly_to_height_or_time() { - let height1 = Height::from(10); - let height2 = Height::from(11); + let height1 = HeightInterval::from(10); + let height2 = HeightInterval::from(11); let time1 = MtpInterval::from_512_second_intervals(70); let time2 = MtpInterval::from_512_second_intervals(71); @@ -566,12 +573,12 @@ mod tests { #[test] fn height_correctly_implies() { - let height = Height::from(10); + let height = HeightInterval::from(10); let lock_by_height = LockTime::from(height); - assert!(!lock_by_height.is_implied_by(LockTime::from(Height::from(9)))); - assert!(lock_by_height.is_implied_by(LockTime::from(Height::from(10)))); - assert!(lock_by_height.is_implied_by(LockTime::from(Height::from(11)))); + assert!(!lock_by_height.is_implied_by(LockTime::from(HeightInterval::from(9)))); + assert!(lock_by_height.is_implied_by(LockTime::from(HeightInterval::from(10)))); + assert!(lock_by_height.is_implied_by(LockTime::from(HeightInterval::from(11)))); } #[test] @@ -592,7 +599,7 @@ mod tests { #[test] fn sequence_correctly_implies() { - let height = Height::from(10); + let height = HeightInterval::from(10); let time = MtpInterval::from_512_second_intervals(70); let lock_by_height = LockTime::from(height); @@ -615,7 +622,7 @@ mod tests { #[test] fn incorrect_units_do_not_imply() { let time = MtpInterval::from_512_second_intervals(70); - let height = Height::from(10); + let height = HeightInterval::from(10); let lock_by_time = LockTime::from(time); assert!(!lock_by_time.is_implied_by(LockTime::from(height))); @@ -654,7 +661,7 @@ mod tests { #[test] fn incompatible_height_error() { - let height = Height::from(10); + let height = HeightInterval::from(10); let time = MtpInterval::from_512_second_intervals(70); let lock_by_time = LockTime::from(time); let err = lock_by_time.is_satisfied_by_height(height).unwrap_err(); @@ -666,7 +673,7 @@ mod tests { #[test] fn incompatible_time_error() { - let height = Height::from(10); + let height = HeightInterval::from(10); let time = MtpInterval::from_512_second_intervals(70); let lock_by_height = LockTime::from(height); let err = lock_by_height.is_satisfied_by_time(time).unwrap_err(); @@ -692,10 +699,10 @@ mod tests { let chain_tip = MtpAndHeight::new(BlockHeight::from_u32(100), timestamps); let utxo_mined_at = MtpAndHeight::new(BlockHeight::from_u32(80), utxo_timestamps); - let lock1 = LockTime::Blocks(Height::from(10)); + let lock1 = LockTime::Blocks(HeightInterval::from(10)); assert!(lock1.is_satisfied_by(chain_tip, utxo_mined_at)); - let lock2 = LockTime::Blocks(Height::from(21)); + let lock2 = LockTime::Blocks(HeightInterval::from(21)); assert!(!lock2.is_satisfied_by(chain_tip, utxo_mined_at)); let lock3 = LockTime::Time(MtpInterval::from_512_second_intervals(10)); @@ -710,7 +717,7 @@ mod tests { let lock6 = LockTime::from_seconds_floor(5000).unwrap(); assert!(lock6.is_satisfied_by(chain_tip, utxo_mined_at)); - let max_height_lock = LockTime::Blocks(Height::MAX); + let max_height_lock = LockTime::Blocks(HeightInterval::MAX); assert!(!max_height_lock.is_satisfied_by(chain_tip, utxo_mined_at)); let max_time_lock = LockTime::Time(MtpInterval::MAX); diff --git a/primitives/src/sequence.rs b/primitives/src/sequence.rs index 98e42b50f..f24381575 100644 --- a/primitives/src/sequence.rs +++ b/primitives/src/sequence.rs @@ -187,7 +187,7 @@ impl Sequence { /// Constructs a new [`relative::LockTime`] from this [`Sequence`] number. #[inline] pub fn to_relative_lock_time(self) -> Option { - use crate::locktime::relative::{Height, LockTime, MtpInterval}; + use crate::locktime::relative::{HeightInterval, LockTime, MtpInterval}; if !self.is_relative_lock_time() { return None; @@ -198,7 +198,7 @@ impl Sequence { if self.is_time_locked() { Some(LockTime::from(MtpInterval::from_512_second_intervals(lock_value))) } else { - Some(LockTime::from(Height::from(lock_value))) + Some(LockTime::from(HeightInterval::from(lock_value))) } } @@ -261,8 +261,8 @@ impl<'a> Arbitrary<'a> for Sequence { 1 => Ok(Sequence::ZERO), 2 => Ok(Sequence::MIN_NO_RBF), 3 => Ok(Sequence::ENABLE_LOCKTIME_AND_RBF), - 4 => Ok(Sequence::from_consensus(relative::Height::MIN.to_consensus_u32())), - 5 => Ok(Sequence::from_consensus(relative::Height::MAX.to_consensus_u32())), + 4 => Ok(Sequence::from_consensus(relative::HeightInterval::MIN.to_consensus_u32())), + 5 => Ok(Sequence::from_consensus(relative::HeightInterval::MAX.to_consensus_u32())), 6 => Ok(Sequence::from_consensus( Sequence::LOCK_TYPE_MASK | u32::from(relative::MtpInterval::MIN.to_512_second_intervals()), diff --git a/units/src/block.rs b/units/src/block.rs index b28c475e1..8c60e0d58 100644 --- a/units/src/block.rs +++ b/units/src/block.rs @@ -129,47 +129,48 @@ impl From for u32 { fn from(height: BlockInterval) -> Self { height.to_u32() } } -impl From for BlockInterval { - /// Converts a [`locktime::relative::Height`] to a [`BlockInterval`]. +impl From for BlockInterval { + /// Converts a [`locktime::relative::HeightInterval`] to a [`BlockInterval`]. /// /// A relative locktime block height has a maximum value of `u16::MAX` where as a /// [`BlockInterval`] is a thin wrapper around a `u32`, the two types are not interchangeable. - fn from(h: relative::Height) -> Self { Self::from_u32(h.value().into()) } + fn from(h: relative::HeightInterval) -> Self { Self::from_u32(h.value().into()) } } -impl TryFrom for relative::Height { - type Error = TooBigForRelativeBlockHeightError; +impl TryFrom for relative::HeightInterval { + type Error = TooBigForRelativeBlockHeightIntervalError; - /// Converts a [`BlockInterval`] to a [`locktime::relative::Height`]. + /// Converts a [`BlockInterval`] to a [`locktime::relative::HeightInterval`]. /// /// A relative locktime block height has a maximum value of `u16::MAX` where as a /// [`BlockInterval`] is a thin wrapper around a `u32`, the two types are not interchangeable. fn try_from(h: BlockInterval) -> Result { let h = h.to_u32(); + if h > u32::from(u16::MAX) { - return Err(TooBigForRelativeBlockHeightError(h)); + return Err(TooBigForRelativeBlockHeightIntervalError(h)); } - Ok(relative::Height::from(h as u16)) // Cast ok, value checked above. + Ok(relative::HeightInterval::from(h as u16)) // Cast ok, value checked above. } } /// Error returned when the block interval is too big to be used as a relative lock time. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct TooBigForRelativeBlockHeightError(u32); +pub struct TooBigForRelativeBlockHeightIntervalError(u32); -impl fmt::Display for TooBigForRelativeBlockHeightError { +impl fmt::Display for TooBigForRelativeBlockHeightIntervalError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, "block interval is too big to be used as a relative lock time: {} (max: {})", self.0, - relative::Height::MAX + relative::HeightInterval::MAX ) } } #[cfg(feature = "std")] -impl std::error::Error for TooBigForRelativeBlockHeightError {} +impl std::error::Error for TooBigForRelativeBlockHeightIntervalError {} crate::internal_macros::impl_op_for_references! { // height - height = interval @@ -279,14 +280,14 @@ mod tests { let interval: u32 = BlockInterval(100).into(); assert_eq!(interval, 100); - let interval_from_height: BlockInterval = relative::Height::from(10u16).into(); + let interval_from_height: BlockInterval = relative::HeightInterval::from(10u16).into(); assert_eq!(interval_from_height.to_u32(), 10u32); let invalid_height_greater = - relative::Height::try_from(BlockInterval(u32::from(u16::MAX) + 1)); + relative::HeightInterval::try_from(BlockInterval(u32::from(u16::MAX) + 1)); assert!(invalid_height_greater.is_err()); - let valid_height = relative::Height::try_from(BlockInterval(u32::from(u16::MAX))); + let valid_height = relative::HeightInterval::try_from(BlockInterval(u32::from(u16::MAX))); assert!(valid_height.is_ok()); } diff --git a/units/src/locktime/relative.rs b/units/src/locktime/relative.rs index f027acbca..96dfc4fa6 100644 --- a/units/src/locktime/relative.rs +++ b/units/src/locktime/relative.rs @@ -11,24 +11,28 @@ use serde::{Deserialize, Serialize}; use crate::mtp_height::MtpAndHeight; +#[deprecated(since = "TBD", note = "use `HeightIterval` instead")] +#[doc(hidden)] +pub type Height = HeightInterval; + /// A relative lock time lock-by-blockheight value. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -pub struct Height(u16); +pub struct HeightInterval(u16); -impl Height { +impl HeightInterval { /// Relative block height 0, can be included in any block. - pub const ZERO: Self = Height(0); + pub const ZERO: Self = Self(0); /// The minimum relative block height (0), can be included in any block. pub const MIN: Self = Self::ZERO; /// The maximum relative block height. - pub const MAX: Self = Height(u16::MAX); + pub const MAX: Self = Self(u16::MAX); - /// Constructs a new [`Height`] using a count of blocks. + /// Constructs a new [`HeightInterval`] using a count of blocks. #[inline] - pub const fn from_height(blocks: u16) -> Self { Height(blocks) } + pub const fn from_height(blocks: u16) -> Self { Self(blocks) } /// Returns the inner `u16` value. #[inline] @@ -63,14 +67,14 @@ impl Height { } } -impl From for Height { +impl From for HeightInterval { #[inline] - fn from(value: u16) -> Self { Height(value) } + fn from(value: u16) -> Self { HeightInterval(value) } } -crate::impl_parse_str_from_int_infallible!(Height, u16, from); +crate::impl_parse_str_from_int_infallible!(HeightInterval, u16, from); -impl fmt::Display for Height { +impl fmt::Display for HeightInterval { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } } @@ -219,14 +223,14 @@ impl fmt::Display for TimeOverflowError { impl std::error::Error for TimeOverflowError {} #[cfg(feature = "arbitrary")] -impl<'a> Arbitrary<'a> for Height { +impl<'a> Arbitrary<'a> for HeightInterval { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let choice = u.int_in_range(0..=2)?; match choice { - 0 => Ok(Height::MIN), - 1 => Ok(Height::MAX), - _ => Ok(Height::from_height(u16::arbitrary(u)?)), + 0 => Ok(HeightInterval::MIN), + 1 => Ok(HeightInterval::MAX), + _ => Ok(HeightInterval::from_height(u16::arbitrary(u)?)), } } } @@ -257,7 +261,7 @@ mod tests { #[test] #[allow(deprecated_in_future)] fn sanity_check() { - assert_eq!(Height::MAX.to_consensus_u32(), u32::from(u16::MAX)); + assert_eq!(HeightInterval::MAX.to_consensus_u32(), u32::from(u16::MAX)); assert_eq!(MtpInterval::from_512_second_intervals(100).value(), 100u16); assert_eq!(MtpInterval::from_512_second_intervals(100).to_consensus_u32(), 4_194_404u32); // 0x400064 } @@ -312,9 +316,9 @@ mod tests { #[test] #[cfg(feature = "serde")] pub fn encode_decode_height() { - serde_round_trip!(Height::ZERO); - serde_round_trip!(Height::MIN); - serde_round_trip!(Height::MAX); + serde_round_trip!(HeightInterval::ZERO); + serde_round_trip!(HeightInterval::MIN); + serde_round_trip!(HeightInterval::MAX); } #[test] @@ -374,7 +378,7 @@ mod tests { let timestamps: [BlockTime; 11] = generate_timestamps(1_600_000_000, 200); let utxo_timestamps: [BlockTime; 11] = generate_timestamps(1_599_000_000, 200); - let height_lock = Height(10); + let height_lock = HeightInterval(10); // Test case 1: Satisfaction (current_height >= utxo_height + required) let chain_state1 = MtpAndHeight::new(BlockHeight::from_u32(100), timestamps); @@ -387,7 +391,7 @@ mod tests { assert!(!height_lock.is_satisfied_by(chain_state2, utxo_state2)); // Test case 3: Overflow handling - tests that is_satisfied_by handles overflow gracefully - let max_height_lock = Height::MAX; + let max_height_lock = HeightInterval::MAX; let chain_state3 = MtpAndHeight::new(BlockHeight::from_u32(1000), timestamps); let utxo_state3 = MtpAndHeight::new(BlockHeight::from_u32(80), utxo_timestamps); assert!(!max_height_lock.is_satisfied_by(chain_state3, utxo_state3)); diff --git a/units/tests/api.rs b/units/tests/api.rs index 475bd5fd2..41b39bb4b 100644 --- a/units/tests/api.rs +++ b/units/tests/api.rs @@ -128,7 +128,7 @@ struct Errors { t: amount::PossiblyConfusingDenominationError, u: amount::TooPreciseError, v: amount::UnknownDenominationError, - w: block::TooBigForRelativeBlockHeightError, + w: block::TooBigForRelativeBlockHeightIntervalError, x: locktime::absolute::ConversionError, y: locktime::absolute::Height, z: locktime::absolute::ParseHeightError, @@ -163,7 +163,9 @@ fn api_can_use_all_types_from_module_amount() { #[test] fn api_can_use_all_types_from_module_block() { - use bitcoin_units::block::{BlockHeight, BlockInterval, TooBigForRelativeBlockHeightError}; + use bitcoin_units::block::{ + BlockHeight, BlockInterval, TooBigForRelativeBlockHeightIntervalError, + }; } #[test]