Modify locktime serde implemenations

The `units::locktime` types are used for two things:

- They are the inner types of `primitives` `LockTime`s
- They are used ephemerally for checking satisfaction

Neither of these use cases requires `serde` impls for the `units` types.
Since we are trying to release 1.0 with minimal amounts of code we
should remove them.

For `LockTime`s that need to be stored on disk or go over the wire we
can manually implement the `serde` traits. For `absolute::LockTime` this
is done already and there is no reason the `relative::LockTime` impl
cannot be the same [0]. This differs from the current `serde` trait impls
but we have already decided that in 0.33 we are going to accept breakage
and direct users to use 0.32 to handle it.

- Remove `serde` stuff from `units::locktime`
- Manually implement `serde` traits on `relative::LockTime`
- Fix the regression test to use the new format

While we are at it use a uniform terse call in `serialize`.

[0] This is because there is an unambiguous encoding for the whole set
of locktimes - consensus encoding.

primitives/src/locktime/absolute.rs

@@ -413,7 +413,7 @@ impl serde::Serialize for LockTime {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_u32(self.to_consensus_u32())
+        self.to_consensus_u32().serialize(serializer)
     }
 }
 
@@ -423,26 +423,7 @@ impl<'de> serde::Deserialize<'de> for LockTime {
     where
         D: serde::Deserializer<'de>,
     {
-        struct Visitor;
-        impl serde::de::Visitor<'_> for Visitor {
-            type Value = u32;
-            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("a u32") }
-            // We cannot just implement visit_u32 because JSON (among other things) always
-            // calls visit_u64, even when called from Deserializer::deserialize_u32. The
-            // other visit_u*s have default implementations that forward to visit_u64.
-            fn visit_u64<E: serde::de::Error>(self, v: u64) -> Result<u32, E> {
-                v.try_into().map_err(|_| {
-                    E::invalid_value(serde::de::Unexpected::Unsigned(v), &"a 32-bit number")
-                })
-            }
-            // Also do the signed version, just for good measure.
-            fn visit_i64<E: serde::de::Error>(self, v: i64) -> Result<u32, E> {
-                v.try_into().map_err(|_| {
-                    E::invalid_value(serde::de::Unexpected::Signed(v), &"a 32-bit number")
-                })
-            }
-        }
-        deserializer.deserialize_u32(Visitor).map(LockTime::from_consensus)
+        u32::deserialize(deserializer).map(Self::from_consensus)
     }
 }
 

primitives/src/locktime/relative.rs

@@ -42,7 +42,6 @@ pub type Time = NumberOf512Seconds;
 /// * [BIP 68 Relative lock-time using consensus-enforced sequence numbers](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki)
 /// * [BIP 112 CHECKSEQUENCEVERIFY](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki)
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum LockTime {
     /// A block height lock time value.
     Blocks(NumberOfBlocks),
@@ -372,6 +371,27 @@ impl From<LockTime> for Sequence {
     fn from(lt: LockTime) -> Sequence { lt.to_sequence() }
 }
 
+#[cfg(feature = "serde")]
+impl serde::Serialize for LockTime {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.to_consensus_u32().serialize(serializer)
+    }
+}
+
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for LockTime {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        u32::deserialize(deserializer)
+            .and_then(|n| Self::from_consensus(n).map_err(serde::de::Error::custom))
+    }
+}
+
 /// Error returned when a sequence number is parsed as a lock time, but its
 /// "disable" flag is set.
 #[derive(Debug, Clone, Eq, PartialEq)]

units/src/locktime/absolute.rs

@@ -122,27 +122,6 @@ impl From<ParseError> for ParseHeightError {
     fn from(value: ParseError) -> Self { Self(value) }
 }
 
-#[cfg(feature = "serde")]
-impl<'de> serde::Deserialize<'de> for Height {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        let u = u32::deserialize(deserializer)?;
-        Height::from_u32(u).map_err(serde::de::Error::custom)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl serde::Serialize for Height {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        self.to_u32().serialize(serializer)
-    }
-}
-
 #[deprecated(since = "TBD", note = "use `MedianTimePast` instead")]
 #[doc(hidden)]
 pub type Time = MedianTimePast;
@@ -248,27 +227,6 @@ impl fmt::Display for MedianTimePast {
 
 crate::impl_parse_str!(MedianTimePast, ParseTimeError, parser(MedianTimePast::from_u32));
 
-#[cfg(feature = "serde")]
-impl<'de> serde::Deserialize<'de> for MedianTimePast {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        let u = u32::deserialize(deserializer)?;
-        MedianTimePast::from_u32(u).map_err(serde::de::Error::custom)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl serde::Serialize for MedianTimePast {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        self.to_u32().serialize(serializer)
-    }
-}
-
 /// Error returned when parsing block time fails.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct ParseTimeError(ParseError);
@@ -500,9 +458,6 @@ impl<'a> Arbitrary<'a> for MedianTimePast {
 
 #[cfg(test)]
 mod tests {
-    #[cfg(feature = "serde")]
-    use internals::serde_round_trip;
-
     use super::*;
 
     #[test]
@@ -554,21 +509,6 @@ mod tests {
         assert!(is_block_time(500_000_000));
     }
 
-    #[test]
-    #[cfg(feature = "serde")]
-    pub fn encode_decode_height() {
-        serde_round_trip!(Height::ZERO);
-        serde_round_trip!(Height::MIN);
-        serde_round_trip!(Height::MAX);
-    }
-
-    #[test]
-    #[cfg(feature = "serde")]
-    pub fn encode_decode_time() {
-        serde_round_trip!(MedianTimePast::MIN);
-        serde_round_trip!(MedianTimePast::MAX);
-    }
-
     #[test]
     #[cfg(feature = "alloc")]
     fn locktime_unit_display() {

units/src/locktime/relative.rs

@@ -7,8 +7,6 @@ use core::fmt;
 
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
-#[cfg(feature = "serde")]
-use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 #[deprecated(since = "TBD", note = "use `NumberOfBlocks` instead")]
 #[doc(hidden)]
@@ -89,28 +87,6 @@ impl fmt::Display for NumberOfBlocks {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
 }
 
-#[cfg(feature = "serde")]
-impl Serialize for NumberOfBlocks {
-    #[inline]
-    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        u16::serialize(&self.to_height(), s)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl<'de> Deserialize<'de> for NumberOfBlocks {
-    #[inline]
-    fn deserialize<D>(d: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        Ok(Self::from_height(u16::deserialize(d)?))
-    }
-}
-
 #[deprecated(since = "TBD", note = "use `NumberOf512Seconds` instead")]
 #[doc(hidden)]
 pub type Time = NumberOf512Seconds;
@@ -227,28 +203,6 @@ impl fmt::Display for NumberOf512Seconds {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
 }
 
-#[cfg(feature = "serde")]
-impl Serialize for NumberOf512Seconds {
-    #[inline]
-    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        u16::serialize(&self.to_512_second_intervals(), s)
-    }
-}
-
-#[cfg(feature = "serde")]
-impl<'de> Deserialize<'de> for NumberOf512Seconds {
-    #[inline]
-    fn deserialize<D>(d: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        Ok(Self::from_512_second_intervals(u16::deserialize(d)?))
-    }
-}
-
 /// Error returned when the input time in seconds was too large to be encoded to a 16 bit 512 second interval.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct TimeOverflowError {
@@ -348,9 +302,6 @@ impl<'a> Arbitrary<'a> for NumberOf512Seconds {
 
 #[cfg(test)]
 mod tests {
-    #[cfg(feature = "serde")]
-    use internals::serde_round_trip;
-
     use super::*;
     use crate::BlockTime;
 
@@ -416,22 +367,6 @@ mod tests {
         assert!(result.is_err());
     }
 
-    #[test]
-    #[cfg(feature = "serde")]
-    pub fn encode_decode_height() {
-        serde_round_trip!(NumberOfBlocks::ZERO);
-        serde_round_trip!(NumberOfBlocks::MIN);
-        serde_round_trip!(NumberOfBlocks::MAX);
-    }
-
-    #[test]
-    #[cfg(feature = "serde")]
-    pub fn encode_decode_time() {
-        serde_round_trip!(NumberOf512Seconds::ZERO);
-        serde_round_trip!(NumberOf512Seconds::MIN);
-        serde_round_trip!(NumberOf512Seconds::MAX);
-    }
-
     fn generate_timestamps(start: u32, step: u16) -> [BlockTime; 11] {
         let mut timestamps = [BlockTime::from_u32(0); 11];
         for (i, ts) in timestamps.iter_mut().enumerate() {

units/tests/serde.rs

@@ -6,10 +6,7 @@
 #![cfg(feature = "serde")]
 
 use bincode::serialize;
-use bitcoin_units::locktime::{absolute, relative};
-use bitcoin_units::{
-    amount, fee_rate, Amount, BlockHeight, BlockInterval, FeeRate, SignedAmount, Weight,
-};
+use bitcoin_units::{amount, fee_rate, Amount, BlockHeight, BlockInterval, FeeRate, SignedAmount, Weight};
 use serde::{Deserialize, Serialize};
 
 /// A struct that includes all the types that implement or support `serde` traits.
@@ -48,11 +45,7 @@ struct Serde {
 
     a: BlockHeight,
     b: BlockInterval,
-    c: absolute::Height,
-    d: absolute::MedianTimePast,
-    e: relative::Height,
-    f: relative::Time,
-    g: Weight,
+    c: Weight,
 }
 
 impl Serde {
@@ -81,11 +74,7 @@ impl Serde {
 
             a: BlockHeight::MAX,
             b: BlockInterval::MAX,
-            c: absolute::Height::MAX,
-            d: absolute::MedianTimePast::MAX,
-            e: relative::Height::MAX,
-            f: relative::Time::MAX,
-            g: Weight::MAX,
+            c: Weight::MAX,
         }
     }
 }