Merge rust-bitcoin/rust-bitcoin#2581: Implement ArbitraryOrd for relative::LockTime

d91cdd20bf docs: Document ordered feature (Tobin C. Harding)
3520f550f0 Implement ArbitraryOrd for relative::LockTime (Tobin C. Harding)

Pull request description:

  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.

  Fix: #2566

ACKs for top commit:
  sanket1729:
    ACK d91cdd20bf
  apoelstra:
    ACK d91cdd20bf

Tree-SHA512: 52ace9222e765dfa266d003b4aff3e93e35d1414c9fd579c4a4a36998d6d1b08bf6d4964a6f1c1d769068d65e47a882495daa4aacf254909a35dce8e01c99a9e
This commit is contained in:
Andrew Poelstra 2024-04-02 00:26:31 +00:00
commit 6a2fd96ff6
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
3 changed files with 24 additions and 5 deletions

View File

@ -33,11 +33,12 @@ pub use units::locktime::absolute::{
/// ///
/// ### Note on ordering /// ### Note on ordering
/// ///
/// Because locktimes may be height- or time-based, and these metrics are incommensurate, there /// Locktimes may be height- or time-based, and these metrics are incommensurate; there is no total
/// is no total ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. /// 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 /// 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 /// 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 /// ### Relevant BIPs
/// ///

View File

@ -6,6 +6,8 @@
//! whether bit 22 of the `u32` consensus value is set. //! whether bit 22 of the `u32` consensus value is set.
//! //!
#[cfg(feature = "ordered")]
use core::cmp::Ordering;
use core::{cmp, convert, fmt}; use core::{cmp, convert, fmt};
#[cfg(all(test, mutate))] #[cfg(all(test, mutate))]
@ -26,8 +28,9 @@ pub use units::locktime::relative::{Height, Time, TimeOverflowError};
/// ///
/// ### Note on ordering /// ### Note on ordering
/// ///
/// Because locktimes may be height- or time-based, and these metrics are incommensurate, there /// Locktimes may be height- or time-based, and these metrics are incommensurate; there is no total
/// is no total ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. /// ordering on locktimes. We therefore have implemented [`PartialOrd`] but not [`Ord`]. We also
/// implement [`ordered::ArbitraryOrd`] if the "ordered" feature is enabled.
/// ///
/// ### Relevant BIPs /// ### 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<Sequence> for LockTime { impl convert::TryFrom<Sequence> for LockTime {
type Error = DisabledLockTimeError; type Error = DisabledLockTimeError;
fn try_from(seq: Sequence) -> Result<LockTime, DisabledLockTimeError> { fn try_from(seq: Sequence) -> Result<LockTime, DisabledLockTimeError> {

View File

@ -27,6 +27,7 @@
//! `std::error::Error`. At this time there's a hack to //! `std::error::Error`. At this time there's a hack to
//! achieve the same without this feature but it could //! achieve the same without this feature but it could
//! happen the implementations diverge one day. //! 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)] #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
// Experimental features we need. // Experimental features we need.