Merge #2581: Implement ArbitraryOrd for relative::LockTime

d91cdd2 docs: Document ordered feature (Tobin C. Harding)
3520f55 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 d91cdd2
  apoelstra:
    ACK d91cdd2

Tree-SHA512: 52ace9222e765dfa266d003b4aff3e93e35d1414c9fd579c4a4a36998d6d1b08bf6d4964a6f1c1d769068d65e47a882495daa4aacf254909a35dce8e01c99a9e

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
 ///

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<Sequence> for LockTime {
     type Error = DisabledLockTimeError;
     fn try_from(seq: Sequence) -> Result<LockTime, DisabledLockTimeError> {

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)]
 // Experimental features we need.