From 200c27631521ad139e2070a04c1cf223791a1277 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 15 May 2025 09:34:52 +1000 Subject: [PATCH 1/2] bitcoin: Make test code spacing uniform Make test code use uniform spacing - twitch averted. Whitespace only, no logic change. --- bitcoin/tests/serde.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs index a0842e2b8..3d2b4a28a 100644 --- a/bitcoin/tests/serde.rs +++ b/bitcoin/tests/serde.rs @@ -47,6 +47,7 @@ fn serde_regression_block() { "data/testnet_block_000000000000045e0b1660b6445b5e5c5ab63c9a4f956be7e1e69be04fa4497b.raw" ); let block: Block = deserialize(segwit).unwrap(); + let got = serialize(&block).unwrap(); let want = include_bytes!("data/serde/block_bincode"); assert_eq!(got, want) @@ -55,6 +56,7 @@ fn serde_regression_block() { #[test] fn serde_regression_absolute_lock_time_height() { let t = absolute::LockTime::from_height(741521).expect("valid height"); + let got = serialize(&t).unwrap(); let want = include_bytes!("data/serde/absolute_lock_time_blocks_bincode") as &[_]; assert_eq!(got, want); @@ -64,8 +66,8 @@ fn serde_regression_absolute_lock_time_height() { fn serde_regression_absolute_lock_time_time() { let seconds: u32 = 1653195600; // May 22nd, 5am UTC. let t = absolute::LockTime::from_mtp(seconds).expect("valid time"); - let got = serialize(&t).unwrap(); + let got = serialize(&t).unwrap(); let want = include_bytes!("data/serde/absolute_lock_time_seconds_bincode") as &[_]; assert_eq!(got, want); } @@ -73,8 +75,8 @@ fn serde_regression_absolute_lock_time_time() { #[test] fn serde_regression_relative_lock_time_height() { let t = relative::LockTime::from(relative::Height::from(0xCAFE_u16)); - let got = serialize(&t).unwrap(); + let got = serialize(&t).unwrap(); let want = include_bytes!("data/serde/relative_lock_time_blocks_bincode") as &[_]; assert_eq!(got, want); } @@ -82,8 +84,8 @@ fn serde_regression_relative_lock_time_height() { #[test] fn serde_regression_relative_lock_time_time() { let t = relative::LockTime::from(relative::Time::from_512_second_intervals(0xFACE_u16)); - let got = serialize(&t).unwrap(); + let got = serialize(&t).unwrap(); let want = include_bytes!("data/serde/relative_lock_time_seconds_bincode") as &[_]; assert_eq!(got, want); } @@ -91,6 +93,7 @@ fn serde_regression_relative_lock_time_time() { #[test] fn serde_regression_script() { let script = ScriptBuf::from(vec![0u8, 1u8, 2u8]); + let got = serialize(&script).unwrap(); let want = include_bytes!("data/serde/script_bincode") as &[_]; assert_eq!(got, want) @@ -109,6 +112,7 @@ fn serde_regression_txin() { #[test] fn serde_regression_txout() { let txout = TxOut { value: Amount::MAX, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) }; + let got = serialize(&txout).unwrap(); let want = include_bytes!("data/serde/txout_bincode") as &[_]; assert_eq!(got, want) @@ -118,6 +122,7 @@ fn serde_regression_txout() { fn serde_regression_transaction() { let ser = include_bytes!("data/serde/transaction_ser"); let tx: Transaction = deserialize(ser).unwrap(); + let got = serialize(&tx).unwrap(); let want = include_bytes!("data/serde/transaction_bincode") as &[_]; assert_eq!(got, want) @@ -151,6 +156,7 @@ fn serde_regression_address() { fn serde_regression_extended_priv_key() { let s = include_str!("data/serde/extended_priv_key"); let key = s.trim().parse::().unwrap(); + let got = serialize(&key).unwrap(); let want = include_bytes!("data/serde/extended_priv_key_bincode") as &[_]; assert_eq!(got, want) @@ -160,6 +166,7 @@ fn serde_regression_extended_priv_key() { fn serde_regression_extended_pub_key() { let s = include_str!("data/serde/extended_pub_key"); let key = s.trim().parse::().unwrap(); + let got = serialize(&key).unwrap(); let want = include_bytes!("data/serde/extended_pub_key_bincode") as &[_]; assert_eq!(got, want) @@ -182,8 +189,8 @@ fn serde_regression_ecdsa_sig() { fn serde_regression_control_block() { let s = include_str!("data/serde/control_block_hex"); let block = ControlBlock::decode(&Vec::::from_hex(s.trim()).unwrap()).unwrap(); - let got = serialize(&block).unwrap(); + let got = serialize(&block).unwrap(); let want = include_bytes!("data/serde/control_block_bincode") as &[_]; assert_eq!(got, want) } @@ -191,6 +198,7 @@ fn serde_regression_control_block() { #[test] fn serde_regression_child_number() { let num = ChildNumber::Normal { index: 0xDEADBEEF }; + let got = serialize(&num).unwrap(); let want = include_bytes!("data/serde/child_number_bincode") as &[_]; assert_eq!(got, want) @@ -199,6 +207,7 @@ fn serde_regression_child_number() { #[test] fn serde_regression_private_key() { let sk = PrivateKey::from_wif("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap(); + let got = serialize(&sk).unwrap(); let want = include_bytes!("data/serde/private_key_bincode") as &[_]; assert_eq!(got, want) @@ -208,6 +217,7 @@ fn serde_regression_private_key() { fn serde_regression_public_key() { let s = include_str!("data/serde/public_key_hex"); let pk = s.trim().parse::().unwrap(); + let got = serialize(&pk).unwrap(); let want = include_bytes!("data/serde/public_key_bincode") as &[_]; assert_eq!(got, want) @@ -368,6 +378,7 @@ fn le_bytes() -> [u8; 32] { #[test] fn serde_regression_work() { let work = Work::from_le_bytes(le_bytes()); + let got = serialize(&work).unwrap(); let want = include_bytes!("data/serde/u256_bincode") as &[_]; assert_eq!(got, want) @@ -376,6 +387,7 @@ fn serde_regression_work() { #[test] fn serde_regression_target() { let target = Target::from_le_bytes(le_bytes()); + let got = serialize(&target).unwrap(); let want = include_bytes!("data/serde/u256_bincode") as &[_]; assert_eq!(got, want) From 4621d2bde14d71b3d6ef0b14258a7479c049ba3b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 15 May 2025 09:40:15 +1000 Subject: [PATCH 2/2] 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. --- .../serde/relative_lock_time_blocks_bincode | Bin 6 -> 4 bytes .../serde/relative_lock_time_seconds_bincode | Bin 6 -> 4 bytes primitives/src/locktime/absolute.rs | 23 +------ primitives/src/locktime/relative.rs | 22 +++++- units/src/locktime/absolute.rs | 60 ---------------- units/src/locktime/relative.rs | 65 ------------------ units/tests/data/serde_bincode | Bin 147 -> 135 bytes units/tests/serde.rs | 17 +---- 8 files changed, 26 insertions(+), 161 deletions(-) diff --git a/bitcoin/tests/data/serde/relative_lock_time_blocks_bincode b/bitcoin/tests/data/serde/relative_lock_time_blocks_bincode index 4eb84bb45911c09de9bdc6e7ff57f8fa89090a80..167e25d50c5f161edba7ac04d8bdedd9a8f61a27 100644 GIT binary patch literal 4 Lcmex&ih%(D23i5h literal 6 NcmZQzU|{%n3IGDk0m%RW diff --git a/bitcoin/tests/data/serde/relative_lock_time_seconds_bincode b/bitcoin/tests/data/serde/relative_lock_time_seconds_bincode index b6ed9a62a79422b1e6555e84e8d0a8830abcccae..53442e8bb233bb74ea6fd91e7e5e509de6864ef3 100644 GIT binary patch literal 4 LcmX^2%Ygv^2C4!H literal 6 NcmZQ%U|=}+3jhM50m=XX diff --git a/primitives/src/locktime/absolute.rs b/primitives/src/locktime/absolute.rs index 808ff6c63..53225df3e 100644 --- a/primitives/src/locktime/absolute.rs +++ b/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(self, v: u64) -> Result { - 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(self, v: i64) -> Result { - 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) } } diff --git a/primitives/src/locktime/relative.rs b/primitives/src/locktime/relative.rs index fd58ff1db..5bb2dc1ab 100644 --- a/primitives/src/locktime/relative.rs +++ b/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 for Sequence { fn from(lt: LockTime) -> Sequence { lt.to_sequence() } } +#[cfg(feature = "serde")] +impl serde::Serialize for LockTime { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.to_consensus_u32().serialize(serializer) + } +} + +#[cfg(feature = "serde")] +impl<'de> serde::Deserialize<'de> for LockTime { + fn deserialize(deserializer: D) -> Result + 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)] diff --git a/units/src/locktime/absolute.rs b/units/src/locktime/absolute.rs index c0e2b8417..1787ea089 100644 --- a/units/src/locktime/absolute.rs +++ b/units/src/locktime/absolute.rs @@ -122,27 +122,6 @@ impl From for ParseHeightError { fn from(value: ParseError) -> Self { Self(value) } } -#[cfg(feature = "serde")] -impl<'de> serde::Deserialize<'de> for Height { - fn deserialize(deserializer: D) -> Result - 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(&self, serializer: S) -> Result - 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(deserializer: D) -> Result - 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(&self, serializer: S) -> Result - 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() { diff --git a/units/src/locktime/relative.rs b/units/src/locktime/relative.rs index 8aa218db9..f3dfba28a 100644 --- a/units/src/locktime/relative.rs +++ b/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(&self, s: S) -> Result - where - S: Serializer, - { - u16::serialize(&self.to_height(), s) - } -} - -#[cfg(feature = "serde")] -impl<'de> Deserialize<'de> for NumberOfBlocks { - #[inline] - fn deserialize(d: D) -> Result - 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(&self, s: S) -> Result - 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) -> Result - 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() { diff --git a/units/tests/data/serde_bincode b/units/tests/data/serde_bincode index 51d7dc1d09ea388238a66e571ba6b609e2685997..556b15cd794bd56e1ef82aa1bd0f4bf3642fd607 100644 GIT binary patch delta 14 RcmbQt*v>eif&D)O001<=2`vBs delta 26 UcmZo?oXj|(K{(~C?0+->0MlO+egFUf diff --git a/units/tests/serde.rs b/units/tests/serde.rs index 4c93da1f0..dd894aeaa 100644 --- a/units/tests/serde.rs +++ b/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, } } }