Merge rust-bitcoin#5437: units: Improve and test API surface

7cef310 Update the API files (Tobin C. Harding)
3adb97f units: Update API test (Tobin C. Harding)
ce355b4 Implement Arbitrary for relative::LockTime (Tobin C. Harding)

Pull request description:

  Since we implemented it the API test has gotten out of sync, unfortunately there is no obvious way to prevent this.
    
  - Patch 1: Add missing `Arbitrary` impl
  - Patch 2: Clean the `api` test.
  - Patch 3: Update the API files.
  
  I just used grep on the `units` directory to do the fixes.


ACKs for top commit:
  apoelstra:
    ACK 7cef310; successfully ran local tests


Tree-SHA512: ec3252b2ea63976792730d421ca3373d7eb983175c2d6b05c1972402a85ee7ed5344c61e4467a9f8974c5712b2dce2c07bdec5c2a143a88566be023fa653144e

Author: apoelstra

api/units/all-features.txt

@@ -1261,6 +1261,7 @@ impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::block::BlockMtpInterval
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::absolute::Height
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::absolute::LockTime
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::absolute::MedianTimePast
+impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::relative::LockTime
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::relative::NumberOf512Seconds
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::locktime::relative::NumberOfBlocks
 impl<'a> arbitrary::Arbitrary<'a> for bitcoin_units::result::MathOp
@@ -2246,6 +2247,7 @@ pub fn bitcoin_units::locktime::absolute::error::ParseTimeError::clone(&self) ->
 pub fn bitcoin_units::locktime::absolute::error::ParseTimeError::eq(&self, other: &bitcoin_units::locktime::absolute::error::ParseTimeError) -> bool
 pub fn bitcoin_units::locktime::absolute::error::ParseTimeError::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin_units::locktime::absolute::error::ParseTimeError::source(&self) -> core::option::Option<&(dyn core::error::Error + 'static)>
+pub fn bitcoin_units::locktime::relative::LockTime::arbitrary(u: &mut arbitrary::unstructured::Unstructured<'a>) -> arbitrary::error::Result<Self>
 pub fn bitcoin_units::locktime::relative::LockTime::clone(&self) -> bitcoin_units::locktime::relative::LockTime
 pub fn bitcoin_units::locktime::relative::LockTime::deserialize<D>(deserializer: D) -> core::result::Result<Self, <D as serde::de::Deserializer>::Error> where D: serde::de::Deserializer<'de>
 pub fn bitcoin_units::locktime::relative::LockTime::eq(&self, other: &bitcoin_units::locktime::relative::LockTime) -> bool

units/src/locktime/relative/mod.rs

@@ -579,6 +579,18 @@ impl fmt::Display for NumberOf512Seconds {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
 }
 
+#[cfg(feature = "arbitrary")]
+impl<'a> Arbitrary<'a> for LockTime {
+    fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
+        let choice = u.int_in_range(0..=1)?;
+
+        match choice {
+            0 => Ok(Self::Blocks(NumberOfBlocks::arbitrary(u)?)),
+            _ => Ok(Self::Time(NumberOf512Seconds::arbitrary(u)?)),
+        }
+    }
+}
+
 #[cfg(feature = "arbitrary")]
 impl<'a> Arbitrary<'a> for NumberOfBlocks {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {

units/tests/api.rs

@@ -14,31 +14,36 @@ use arbitrary::{Arbitrary, Unstructured};
 // These imports test "typical" usage by user code.
 use bitcoin_units::locktime::{absolute, relative}; // Typical usage is `absolute::LockTime`.
 use bitcoin_units::{
-    amount, block, fee_rate, locktime, parse_int, result, time, weight, Amount, BlockHeight,
-    BlockHeightInterval, BlockMtp, BlockMtpInterval, BlockTime, FeeRate, NumOpResult, SignedAmount,
-    Weight,
+    amount, block, fee_rate, locktime, parse_int, result, sequence, time, weight, Amount,
+    BlockHeight, BlockHeightInterval, BlockMtp, BlockMtpInterval, BlockTime, FeeRate, NumOpResult,
+    Sequence, SignedAmount, Weight,
 };
 
 /// A struct that includes all public non-error enums.
 #[derive(Debug)] // All public types implement Debug (C-DEBUG).
 struct Enums {
     a: amount::Denomination,
-    b: NumOpResult<Amount>,
-    c: result::MathOp,
+    b: absolute::LockTime,
+    c: relative::LockTime,
+    d: result::MathOp,
+    e: result::NumOpResult<Amount>,
 }
 
 impl Enums {
     fn new() -> Self {
         Self {
             a: amount::Denomination::Bitcoin,
-            b: NumOpResult::Valid(Amount::MAX),
-            c: result::MathOp::Add,
+            b: absolute::LockTime::Blocks(absolute::Height::MAX),
+            c: relative::LockTime::Blocks(relative::NumberOfBlocks::MAX),
+            d: result::MathOp::Add,
+            e: result::NumOpResult::Valid(Amount::MAX),
         }
     }
 }
 
 /// A struct that includes all public non-error structs.
 #[derive(Debug)] // All public types implement Debug (C-DEBUG).
+                 // Does not include encoders and decoders.
 struct Structs {
     // Full path to show alphabetic sort order.
     a: amount::Amount,
@@ -53,8 +58,9 @@ struct Structs {
     j: locktime::absolute::MedianTimePast,
     k: locktime::relative::NumberOf512Seconds,
     l: locktime::relative::NumberOfBlocks,
-    m: time::BlockTime,
-    n: weight::Weight,
+    m: sequence::Sequence,
+    n: time::BlockTime,
+    o: weight::Weight,
 }
 
 impl Structs {
@@ -72,8 +78,9 @@ impl Structs {
             j: absolute::MedianTimePast::MAX,
             k: relative::NumberOf512Seconds::MAX,
             l: relative::NumberOfBlocks::MAX,
-            m: BlockTime::from_u32(u32::MAX),
-            n: Weight::MAX,
+            m: sequence::Sequence::MAX,
+            n: BlockTime::from_u32(u32::MAX),
+            o: Weight::MAX,
         }
     }
 }
@@ -119,9 +126,10 @@ struct Default {
     d: BlockMtpInterval,
     e: relative::NumberOf512Seconds,
     f: relative::NumberOfBlocks,
+    g: Sequence,
 }
 
-/// A struct that includes all public error types.
+/// A struct that includes all public error types (excl. decode errors).
 // These derives are the policy of `rust-bitcoin` not Rust API guidelines.
 #[derive(Debug, Clone, PartialEq, Eq)] // All public types implement Debug (C-DEBUG).
 struct Errors {
@@ -150,16 +158,30 @@ struct Errors {
     v: parse_int::UnprefixedHexError,
 }
 
+/// A struct that includes all public decoder error types.
+// These derives are the policy of `rust-bitcoin` not Rust API guidelines.
+#[derive(Debug, Clone, PartialEq, Eq)] // All public types implement Debug (C-DEBUG).
+#[cfg(feature = "encoding")]
+struct DecoderErrors {
+    a: amount::error::AmountDecoderError,
+    b: block::BlockHeightDecoderError,
+    c: locktime::absolute::LockTimeDecoderError,
+    d: sequence::SequenceDecoderError,
+    e: time::BlockTimeDecoderError,
+}
+
 #[test]
 fn api_can_use_modules_from_crate_root() {
-    use bitcoin_units::{amount, block, fee_rate, locktime, parse_int, time, weight};
+    use bitcoin_units::{
+        amount, block, fee_rate, locktime, parse_int, result, sequence, time, weight,
+    };
 }
 
 #[test]
 fn api_can_use_types_from_crate_root() {
     use bitcoin_units::{
         Amount, BlockHeight, BlockHeightInterval, BlockMtp, BlockMtpInterval, BlockTime, FeeRate,
-        NumOpResult, SignedAmount, Weight,
+        NumOpResult, Sequence, SignedAmount, Weight,
     };
 }
 
@@ -169,13 +191,15 @@ fn api_can_use_all_types_from_module_amount() {
         Amount, Denomination, Display, OutOfRangeError, ParseAmountError, ParseDenominationError,
         ParseError, SignedAmount,
     };
+    #[cfg(feature = "encoding")]
+    use bitcoin_units::amount::{AmountDecoder, AmountDecoderError, AmountEncoder};
 }
 
 #[test]
 fn api_can_use_all_types_from_module_amount_error() {
     use bitcoin_units::amount::error::{
-        InputTooLargeError, InvalidCharacterError, MissingDenominationError, MissingDigitsError,
-        OutOfRangeError, ParseAmountError, ParseDenominationError, ParseError,
+        BadPositionError, InputTooLargeError, InvalidCharacterError, MissingDenominationError,
+        MissingDigitsError, OutOfRangeError, ParseAmountError, ParseDenominationError, ParseError,
         PossiblyConfusingDenominationError, TooPreciseError, UnknownDenominationError,
     };
 }
@@ -185,6 +209,15 @@ fn api_can_use_all_types_from_module_block() {
     use bitcoin_units::block::{
         BlockHeight, BlockHeightInterval, BlockMtp, BlockMtpInterval, TooBigForRelativeHeightError,
     };
+    #[cfg(feature = "encoding")]
+    use bitcoin_units::block::{BlockHeightDecoder, BlockHeightDecoderError, BlockHeightEncoder};
+}
+
+#[test]
+fn api_can_use_all_types_from_module_sequence() {
+    use bitcoin_units::sequence::Sequence;
+    #[cfg(feature = "encoding")]
+    use bitcoin_units::sequence::{SequenceDecoder, SequenceDecoderError, SequenceEncoder};
 }
 
 #[test]
@@ -196,21 +229,31 @@ fn api_can_use_all_types_from_module_fee_rate() {
 
 #[test]
 fn api_can_use_all_types_from_module_locktime_absolute() {
+    #[cfg(feature = "encoding")]
+    use bitcoin_units::locktime::absolute::error::LockTimeDecoderError as _;
     use bitcoin_units::locktime::absolute::error::{
-        ConversionError as _, ParseHeightError as _, ParseTimeError as _,
+        ConversionError as _, IncompatibleHeightError as _, IncompatibleTimeError as _,
+        ParseHeightError as _, ParseTimeError as _,
+    };
+    use bitcoin_units::locktime::absolute::{
+        ConversionError, IncompatibleHeightError, IncompatibleTimeError, ParseHeightError,
+        ParseTimeError,
     };
+    #[cfg(feature = "encoding")]
     use bitcoin_units::locktime::absolute::{
-        ConversionError, Height, MedianTimePast, ParseHeightError, ParseTimeError,
+        LockTimeDecoder, LockTimeDecoderError, LockTimeEncoder,
     };
 }
 
 #[test]
 fn api_can_use_all_types_from_module_locktime_relative() {
     use bitcoin_units::locktime::relative::error::{
-        InvalidHeightError as _, InvalidTimeError as _, TimeOverflowError as _,
+        DisabledLockTimeError as _, InvalidHeightError as _, InvalidTimeError as _,
+        TimeOverflowError as _,
     };
     use bitcoin_units::locktime::relative::{
-        InvalidHeightError, InvalidTimeError, NumberOf512Seconds, NumberOfBlocks, TimeOverflowError,
+        DisabledLockTimeError, InvalidHeightError, InvalidTimeError, NumberOf512Seconds,
+        NumberOfBlocks, TimeOverflowError,
     };
 }
 
@@ -222,6 +265,8 @@ fn api_can_use_all_types_from_module_parse() {
 #[test]
 fn api_can_use_all_types_from_module_time() {
     use bitcoin_units::time::BlockTime;
+    #[cfg(feature = "encoding")]
+    use bitcoin_units::time::{BlockTimeDecoder, BlockTimeDecoderError, BlockTimeEncoder};
 }
 
 #[test]
@@ -236,6 +281,14 @@ fn api_all_non_error_types_have_non_empty_debug() {
 
     let debug = format!("{:?}", t.a.a);
     assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.a.b);
+    assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.a.c);
+    assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.a.d);
+    assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.a.e);
+    assert!(!debug.is_empty());
 
     let debug = format!("{:?}", t.b.a);
     assert!(!debug.is_empty());
@@ -261,6 +314,12 @@ fn api_all_non_error_types_have_non_empty_debug() {
     assert!(!debug.is_empty());
     let debug = format!("{:?}", t.b.l);
     assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.b.m);
+    assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.b.n);
+    assert!(!debug.is_empty());
+    let debug = format!("{:?}", t.b.o);
+    assert!(!debug.is_empty());
 }
 
 #[test]
@@ -287,6 +346,7 @@ fn regression_default() {
         d: BlockMtpInterval::ZERO,
         e: relative::NumberOf512Seconds::ZERO,
         f: relative::NumberOfBlocks::ZERO,
+        g: Sequence::MAX,
     };
     assert_eq!(got, want);
 }
@@ -327,8 +387,9 @@ impl<'a> Arbitrary<'a> for Structs {
             j: absolute::MedianTimePast::arbitrary(u)?,
             k: relative::NumberOf512Seconds::arbitrary(u)?,
             l: relative::NumberOfBlocks::arbitrary(u)?,
-            m: BlockTime::arbitrary(u)?,
-            n: Weight::arbitrary(u)?,
+            m: sequence::Sequence::arbitrary(u)?,
+            n: BlockTime::arbitrary(u)?,
+            o: Weight::arbitrary(u)?,
         };
         Ok(a)
     }
@@ -339,8 +400,10 @@ impl<'a> Arbitrary<'a> for Enums {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
         let a = Self {
             a: amount::Denomination::arbitrary(u)?,
-            b: NumOpResult::<Amount>::arbitrary(u)?,
-            c: result::MathOp::arbitrary(u)?,
+            b: absolute::LockTime::arbitrary(u)?,
+            c: relative::LockTime::arbitrary(u)?,
+            d: result::MathOp::arbitrary(u)?,
+            e: result::NumOpResult::<Amount>::arbitrary(u)?,
         };
         Ok(a)
     }