Merge rust-bitcoin/rust-bitcoin#2582: fix: Manually implement serde traits

911f8cbd6a fix: Manually implement serde traits (Tobin C. Harding)

Pull request description:

  Currently we are deriving the serde traits for the `absolute::{Height, Time}` types, this is incorrect because we maintain an invariant on the inner `u32` of both types that it is above or below the threshold.

  Manually implement the serde traits and pass the deserialized `u32` to `from_consensus` to maintain the invariant.

  Close: #2559

ACKs for top commit:
  apoelstra:
    ACK 911f8cbd6a
  sanket1729:
    ACK 911f8cbd6a.

Tree-SHA512: 28f9f3e23b2016e00216e5e94f35e22a5bfa6b29e1dd30fa3351575eafc2e47dbd98aa8e51631de2f09eec8d881b0e5e79231185c7a293c1ba0170a90fbbd19d

units/src/lib.rs

@@ -33,6 +33,10 @@ extern crate std;
 #[cfg(feature = "serde")]
 pub extern crate serde;
 
+#[cfg(test)]
+#[macro_use]
+mod test_macros;
+
 pub mod amount;
 #[cfg(feature = "alloc")]
 pub mod locktime;

units/src/locktime/absolute.rs

@@ -4,9 +4,6 @@
 
 use core::fmt;
 
-#[cfg(feature = "serde")]
-use serde::{Deserialize, Serialize};
-
 use internals::write_err;
 
 #[cfg(feature = "alloc")]
@@ -28,7 +25,6 @@ pub const LOCK_TIME_THRESHOLD: u32 = 500_000_000;
 
 /// An absolute block height, guaranteed to always contain a valid height value.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct Height(u32);
 
 impl Height {
@@ -100,13 +96,33 @@ 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)?;
+        Ok(Height::from_consensus(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_consensus_u32().serialize(serializer)
+    }
+}
+
 /// A UNIX timestamp, seconds since epoch, guaranteed to always contain a valid time value.
 ///
 /// Note that there is no manipulation of the inner value during construction or when using
 /// `to_consensus_u32()`. Said another way, `Time(x)` means 'x seconds since epoch' _not_ '(x -
 /// threshold) seconds since epoch'.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub struct Time(u32);
 
 impl Time {
@@ -155,6 +171,27 @@ impl fmt::Display for Time {
 
 crate::impl_parse_str!(Time, ParseTimeError, parser(Time::from_consensus));
 
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for Time {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let u = u32::deserialize(deserializer)?;
+        Ok(Time::from_consensus(u).map_err(serde::de::Error::custom)?)
+    }
+}
+
+#[cfg(feature = "serde")]
+impl serde::Serialize for Time {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.to_consensus_u32().serialize(serializer)
+    }
+}
+
 /// Error returned when parsing block time fails.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct ParseTimeError(ParseError);
@@ -364,4 +401,18 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[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!(Time::MIN);
+        serde_round_trip!(Time::MAX);
+    }
 }

units/src/test_macros.rs

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Bitcoin serde macros.
+//!
+//! This module provides internal macros used for unit tests.
+
+#[cfg(feature = "serde")]
+macro_rules! serde_round_trip (
+    ($var:expr) => ({
+        use serde_json;
+
+        let encoded = serde_json::to_value(&$var).unwrap();
+        let decoded = serde_json::from_value(encoded).unwrap();
+        assert_eq!($var, decoded);
+    })
+);