From 3520f550f0d00c96bba7ab0f9912bb5bfd609edd Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 13 Mar 2024 12:31:37 +1100 Subject: [PATCH 1/2] Implement ArbitraryOrd for relative::LockTime TL;DR As we do for `absolute::LockTime` and for the same reasons; implement `ArbitraryOrd` for `relative::LockTime`. locktimes do not have a semantic ordering if they differ (blocks, time) so we do not derive `Ord` however it is useful for downstream to be able to order structs that contain lock times. This is exactly what the `ArbitraryOrd` trait is for. Update the rustdocs in `relative` and mirror the docs changes in `absolute`. Fix: #2566 --- bitcoin/src/blockdata/locktime/absolute.rs | 7 ++++--- bitcoin/src/blockdata/locktime/relative.rs | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/bitcoin/src/blockdata/locktime/absolute.rs b/bitcoin/src/blockdata/locktime/absolute.rs index 1c4b58f0..70d920f9 100644 --- a/bitcoin/src/blockdata/locktime/absolute.rs +++ b/bitcoin/src/blockdata/locktime/absolute.rs @@ -33,11 +33,12 @@ pub use units::locktime::absolute::{ /// /// ### Note on ordering /// -/// Because locktimes may be height- or time-based, and these metrics are incommensurate, there -/// is no total ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. +/// Locktimes may be height- or time-based, and these metrics are incommensurate; there is no total +/// ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. /// For [`crate::Transaction`], which has a locktime field, we implement a total ordering to make /// it easy to store transactions in sorted data structures, and use the locktime's 32-bit integer -/// consensus encoding to order it. +/// consensus encoding to order it. We also implement [`ordered::ArbitraryOrd`] if the "ordered" +/// feature is enabled. /// /// ### Relevant BIPs /// diff --git a/bitcoin/src/blockdata/locktime/relative.rs b/bitcoin/src/blockdata/locktime/relative.rs index f574f7fb..04fe4bda 100644 --- a/bitcoin/src/blockdata/locktime/relative.rs +++ b/bitcoin/src/blockdata/locktime/relative.rs @@ -6,6 +6,8 @@ //! whether bit 22 of the `u32` consensus value is set. //! +#[cfg(feature = "ordered")] +use core::cmp::Ordering; use core::{cmp, convert, fmt}; #[cfg(all(test, mutate))] @@ -26,8 +28,9 @@ pub use units::locktime::relative::{Height, Time, TimeOverflowError}; /// /// ### Note on ordering /// -/// Because locktimes may be height- or time-based, and these metrics are incommensurate, there -/// is no total ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. +/// Locktimes may be height- or time-based, and these metrics are incommensurate; there is no total +/// ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. We also +/// implement [`ordered::ArbitraryOrd`] if the "ordered" feature is enabled. /// /// ### Relevant BIPs /// @@ -338,6 +341,20 @@ impl fmt::Display for LockTime { } } +#[cfg(feature = "ordered")] +impl ordered::ArbitraryOrd for LockTime { + fn arbitrary_cmp(&self, other: &Self) -> Ordering { + use LockTime::*; + + match (self, other) { + (Blocks(_), Time(_)) => Ordering::Less, + (Time(_), Blocks(_)) => Ordering::Greater, + (Blocks(this), Blocks(that)) => this.cmp(that), + (Time(this), Time(that)) => this.cmp(that), + } + } +} + impl convert::TryFrom for LockTime { type Error = DisabledLockTimeError; fn try_from(seq: Sequence) -> Result { From d91cdd20bfc0eebce5a067d37d05381bac2845aa Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 2 Apr 2024 08:10:38 +1100 Subject: [PATCH 2/2] docs: Document ordered feature Add "ordered" to the list of features in the `bitcoin` crate level docs. --- bitcoin/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 7ae3eb76..c10c84b3 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -27,6 +27,7 @@ //! `std::error::Error`. At this time there's a hack to //! achieve the same without this feature but it could //! happen the implementations diverge one day. +//! * `ordered` - (dependency), adds implementations of `ArbitraryOrdOrd` to some structs. #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]