Skip to content

Commit fa07198

Browse files
apoelstraapoelstra
committed
Merge rust-bitcoin#4511: Modify locktime serde implementations
4621d2b Modify locktime serde implemenations (Tobin C. Harding) 200c276 bitcoin: Make test code spacing uniform (Tobin C. Harding) Pull request description: Patch 1 is preparatory clean up. Patch 2 is the meat and potatoes. See commit log there for full explanation. Briefly: - Remove `serde` stuff from `units::locktime` - Manually implement `serde` traits on `relative::LockTime` - Fix the regression test to use the new format ACKs for top commit: apoelstra: ACK 4621d2b; successfully ran local tests Tree-SHA512: bc96d79dd4d9f5a124c22f0d3be4750cb7b6e86ba448b31e233982567578337d331b2582c6e1f953c00e8393c4a4552c4b574fe8a55323c911115b269b24090e
2 parents a13ba99 + 4621d2b commit fa07198

File tree

9 files changed

+42
-165
lines changed

9 files changed

+42
-165
lines changed

‎bitcoin/tests/data/serde/relative_lock_time_blocks_bincode‎

-2 Bytes
Binary file not shown.

‎bitcoin/tests/data/serde/relative_lock_time_seconds_bincode‎

-2 Bytes
Binary file not shown.

‎bitcoin/tests/serde.rs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ fn serde_regression_block() {
4747
"data/testnet_block_000000000000045e0b1660b6445b5e5c5ab63c9a4f956be7e1e69be04fa4497b.raw"
4848
);
4949
let block: Block = deserialize(segwit).unwrap();
50+
5051
let got = serialize(&block).unwrap();
5152
let want = include_bytes!("data/serde/block_bincode");
5253
assert_eq!(got, want)
@@ -55,6 +56,7 @@ fn serde_regression_block() {
5556
#[test]
5657
fn serde_regression_absolute_lock_time_height() {
5758
let t = absolute::LockTime::from_height(741521).expect("valid height");
59+
5860
let got = serialize(&t).unwrap();
5961
let want = include_bytes!("data/serde/absolute_lock_time_blocks_bincode") as &[_];
6062
assert_eq!(got, want);
@@ -64,33 +66,34 @@ fn serde_regression_absolute_lock_time_height() {
6466
fn serde_regression_absolute_lock_time_time() {
6567
let seconds: u32 = 1653195600; // May 22nd, 5am UTC.
6668
let t = absolute::LockTime::from_mtp(seconds).expect("valid time");
67-
let got = serialize(&t).unwrap();
6869

70+
let got = serialize(&t).unwrap();
6971
let want = include_bytes!("data/serde/absolute_lock_time_seconds_bincode") as &[_];
7072
assert_eq!(got, want);
7173
}
7274

7375
#[test]
7476
fn serde_regression_relative_lock_time_height() {
7577
let t = relative::LockTime::from(relative::Height::from(0xCAFE_u16));
76-
let got = serialize(&t).unwrap();
7778

79+
let got = serialize(&t).unwrap();
7880
let want = include_bytes!("data/serde/relative_lock_time_blocks_bincode") as &[_];
7981
assert_eq!(got, want);
8082
}
8183

8284
#[test]
8385
fn serde_regression_relative_lock_time_time() {
8486
let t = relative::LockTime::from(relative::Time::from_512_second_intervals(0xFACE_u16));
85-
let got = serialize(&t).unwrap();
8687

88+
let got = serialize(&t).unwrap();
8789
let want = include_bytes!("data/serde/relative_lock_time_seconds_bincode") as &[_];
8890
assert_eq!(got, want);
8991
}
9092

9193
#[test]
9294
fn serde_regression_script() {
9395
let script = ScriptBuf::from(vec![0u8, 1u8, 2u8]);
96+
9497
let got = serialize(&script).unwrap();
9598
let want = include_bytes!("data/serde/script_bincode") as &[_];
9699
assert_eq!(got, want)
@@ -109,6 +112,7 @@ fn serde_regression_txin() {
109112
#[test]
110113
fn serde_regression_txout() {
111114
let txout = TxOut { value: Amount::MAX, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) };
115+
112116
let got = serialize(&txout).unwrap();
113117
let want = include_bytes!("data/serde/txout_bincode") as &[_];
114118
assert_eq!(got, want)
@@ -118,6 +122,7 @@ fn serde_regression_txout() {
118122
fn serde_regression_transaction() {
119123
let ser = include_bytes!("data/serde/transaction_ser");
120124
let tx: Transaction = deserialize(ser).unwrap();
125+
121126
let got = serialize(&tx).unwrap();
122127
let want = include_bytes!("data/serde/transaction_bincode") as &[_];
123128
assert_eq!(got, want)
@@ -151,6 +156,7 @@ fn serde_regression_address() {
151156
fn serde_regression_extended_priv_key() {
152157
let s = include_str!("data/serde/extended_priv_key");
153158
let key = s.trim().parse::<Xpriv>().unwrap();
159+
154160
let got = serialize(&key).unwrap();
155161
let want = include_bytes!("data/serde/extended_priv_key_bincode") as &[_];
156162
assert_eq!(got, want)
@@ -160,6 +166,7 @@ fn serde_regression_extended_priv_key() {
160166
fn serde_regression_extended_pub_key() {
161167
let s = include_str!("data/serde/extended_pub_key");
162168
let key = s.trim().parse::<Xpub>().unwrap();
169+
163170
let got = serialize(&key).unwrap();
164171
let want = include_bytes!("data/serde/extended_pub_key_bincode") as &[_];
165172
assert_eq!(got, want)
@@ -182,15 +189,16 @@ fn serde_regression_ecdsa_sig() {
182189
fn serde_regression_control_block() {
183190
let s = include_str!("data/serde/control_block_hex");
184191
let block = ControlBlock::decode(&Vec::<u8>::from_hex(s.trim()).unwrap()).unwrap();
185-
let got = serialize(&block).unwrap();
186192

193+
let got = serialize(&block).unwrap();
187194
let want = include_bytes!("data/serde/control_block_bincode") as &[_];
188195
assert_eq!(got, want)
189196
}
190197

191198
#[test]
192199
fn serde_regression_child_number() {
193200
let num = ChildNumber::Normal { index: 0xDEADBEEF };
201+
194202
let got = serialize(&num).unwrap();
195203
let want = include_bytes!("data/serde/child_number_bincode") as &[_];
196204
assert_eq!(got, want)
@@ -199,6 +207,7 @@ fn serde_regression_child_number() {
199207
#[test]
200208
fn serde_regression_private_key() {
201209
let sk = PrivateKey::from_wif("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy").unwrap();
210+
202211
let got = serialize(&sk).unwrap();
203212
let want = include_bytes!("data/serde/private_key_bincode") as &[_];
204213
assert_eq!(got, want)
@@ -208,6 +217,7 @@ fn serde_regression_private_key() {
208217
fn serde_regression_public_key() {
209218
let s = include_str!("data/serde/public_key_hex");
210219
let pk = s.trim().parse::<PublicKey>().unwrap();
220+
211221
let got = serialize(&pk).unwrap();
212222
let want = include_bytes!("data/serde/public_key_bincode") as &[_];
213223
assert_eq!(got, want)
@@ -368,6 +378,7 @@ fn le_bytes() -> [u8; 32] {
368378
#[test]
369379
fn serde_regression_work() {
370380
let work = Work::from_le_bytes(le_bytes());
381+
371382
let got = serialize(&work).unwrap();
372383
let want = include_bytes!("data/serde/u256_bincode") as &[_];
373384
assert_eq!(got, want)
@@ -376,6 +387,7 @@ fn serde_regression_work() {
376387
#[test]
377388
fn serde_regression_target() {
378389
let target = Target::from_le_bytes(le_bytes());
390+
379391
let got = serialize(&target).unwrap();
380392
let want = include_bytes!("data/serde/u256_bincode") as &[_];
381393
assert_eq!(got, want)

‎primitives/src/locktime/absolute.rs‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ impl serde::Serialize for LockTime {
413413
where
414414
S: serde::Serializer,
415415
{
416-
serializer.serialize_u32(self.to_consensus_u32())
416+
self.to_consensus_u32().serialize(serializer)
417417
}
418418
}
419419

@@ -423,26 +423,7 @@ impl<'de> serde::Deserialize<'de> for LockTime {
423423
where
424424
D: serde::Deserializer<'de>,
425425
{
426-
struct Visitor;
427-
impl serde::de::Visitor<'_> for Visitor {
428-
type Value = u32;
429-
fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("a u32") }
430-
// We cannot just implement visit_u32 because JSON (among other things) always
431-
// calls visit_u64, even when called from Deserializer::deserialize_u32. The
432-
// other visit_u*s have default implementations that forward to visit_u64.
433-
fn visit_u64<E: serde::de::Error>(self, v: u64) -> Result<u32, E> {
434-
v.try_into().map_err(|_| {
435-
E::invalid_value(serde::de::Unexpected::Unsigned(v), &"a 32-bit number")
436-
})
437-
}
438-
// Also do the signed version, just for good measure.
439-
fn visit_i64<E: serde::de::Error>(self, v: i64) -> Result<u32, E> {
440-
v.try_into().map_err(|_| {
441-
E::invalid_value(serde::de::Unexpected::Signed(v), &"a 32-bit number")
442-
})
443-
}
444-
}
445-
deserializer.deserialize_u32(Visitor).map(LockTime::from_consensus)
426+
u32::deserialize(deserializer).map(Self::from_consensus)
446427
}
447428
}
448429

‎primitives/src/locktime/relative.rs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ pub type Time = NumberOf512Seconds;
4242
/// * [BIP 68 Relative lock-time using consensus-enforced sequence numbers](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki)
4343
/// * [BIP 112 CHECKSEQUENCEVERIFY](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki)
4444
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
45-
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
4645
pub enum LockTime {
4746
/// A block height lock time value.
4847
Blocks(NumberOfBlocks),
@@ -372,6 +371,27 @@ impl From<LockTime> for Sequence {
372371
fn from(lt: LockTime) -> Sequence { lt.to_sequence() }
373372
}
374373

374+
#[cfg(feature = "serde")]
375+
impl serde::Serialize for LockTime {
376+
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
377+
where
378+
S: serde::Serializer,
379+
{
380+
self.to_consensus_u32().serialize(serializer)
381+
}
382+
}
383+
384+
#[cfg(feature = "serde")]
385+
impl<'de> serde::Deserialize<'de> for LockTime {
386+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
387+
where
388+
D: serde::Deserializer<'de>,
389+
{
390+
u32::deserialize(deserializer)
391+
.and_then(|n| Self::from_consensus(n).map_err(serde::de::Error::custom))
392+
}
393+
}
394+
375395
/// Error returned when a sequence number is parsed as a lock time, but its
376396
/// "disable" flag is set.
377397
#[derive(Debug, Clone, Eq, PartialEq)]

‎units/src/locktime/absolute.rs‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -122,27 +122,6 @@ impl From<ParseError> for ParseHeightError {
122122
fn from(value: ParseError) -> Self { Self(value) }
123123
}
124124

125-
#[cfg(feature = "serde")]
126-
impl<'de> serde::Deserialize<'de> for Height {
127-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
128-
where
129-
D: serde::Deserializer<'de>,
130-
{
131-
let u = u32::deserialize(deserializer)?;
132-
Height::from_u32(u).map_err(serde::de::Error::custom)
133-
}
134-
}
135-
136-
#[cfg(feature = "serde")]
137-
impl serde::Serialize for Height {
138-
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
139-
where
140-
S: serde::Serializer,
141-
{
142-
self.to_u32().serialize(serializer)
143-
}
144-
}
145-
146125
#[deprecated(since = "TBD", note = "use `MedianTimePast` instead")]
147126
#[doc(hidden)]
148127
pub type Time = MedianTimePast;
@@ -248,27 +227,6 @@ impl fmt::Display for MedianTimePast {
248227

249228
crate::impl_parse_str!(MedianTimePast, ParseTimeError, parser(MedianTimePast::from_u32));
250229

251-
#[cfg(feature = "serde")]
252-
impl<'de> serde::Deserialize<'de> for MedianTimePast {
253-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
254-
where
255-
D: serde::Deserializer<'de>,
256-
{
257-
let u = u32::deserialize(deserializer)?;
258-
MedianTimePast::from_u32(u).map_err(serde::de::Error::custom)
259-
}
260-
}
261-
262-
#[cfg(feature = "serde")]
263-
impl serde::Serialize for MedianTimePast {
264-
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
265-
where
266-
S: serde::Serializer,
267-
{
268-
self.to_u32().serialize(serializer)
269-
}
270-
}
271-
272230
/// Error returned when parsing block time fails.
273231
#[derive(Debug, Clone, Eq, PartialEq)]
274232
pub struct ParseTimeError(ParseError);
@@ -500,9 +458,6 @@ impl<'a> Arbitrary<'a> for MedianTimePast {
500458

501459
#[cfg(test)]
502460
mod tests {
503-
#[cfg(feature = "serde")]
504-
use internals::serde_round_trip;
505-
506461
use super::*;
507462

508463
#[test]
@@ -554,21 +509,6 @@ mod tests {
554509
assert!(is_block_time(500_000_000));
555510
}
556511

557-
#[test]
558-
#[cfg(feature = "serde")]
559-
pub fn encode_decode_height() {
560-
serde_round_trip!(Height::ZERO);
561-
serde_round_trip!(Height::MIN);
562-
serde_round_trip!(Height::MAX);
563-
}
564-
565-
#[test]
566-
#[cfg(feature = "serde")]
567-
pub fn encode_decode_time() {
568-
serde_round_trip!(MedianTimePast::MIN);
569-
serde_round_trip!(MedianTimePast::MAX);
570-
}
571-
572512
#[test]
573513
#[cfg(feature = "alloc")]
574514
fn locktime_unit_display() {

‎units/src/locktime/relative.rs‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ use core::fmt;
77

88
#[cfg(feature = "arbitrary")]
99
use arbitrary::{Arbitrary, Unstructured};
10-
#[cfg(feature = "serde")]
11-
use serde::{Deserialize, Deserializer, Serialize, Serializer};
1210

1311
#[deprecated(since = "TBD", note = "use `NumberOfBlocks` instead")]
1412
#[doc(hidden)]
@@ -89,28 +87,6 @@ impl fmt::Display for NumberOfBlocks {
8987
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
9088
}
9189

92-
#[cfg(feature = "serde")]
93-
impl Serialize for NumberOfBlocks {
94-
#[inline]
95-
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
96-
where
97-
S: Serializer,
98-
{
99-
u16::serialize(&self.to_height(), s)
100-
}
101-
}
102-
103-
#[cfg(feature = "serde")]
104-
impl<'de> Deserialize<'de> for NumberOfBlocks {
105-
#[inline]
106-
fn deserialize<D>(d: D) -> Result<Self, D::Error>
107-
where
108-
D: Deserializer<'de>,
109-
{
110-
Ok(Self::from_height(u16::deserialize(d)?))
111-
}
112-
}
113-
11490
#[deprecated(since = "TBD", note = "use `NumberOf512Seconds` instead")]
11591
#[doc(hidden)]
11692
pub type Time = NumberOf512Seconds;
@@ -227,28 +203,6 @@ impl fmt::Display for NumberOf512Seconds {
227203
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
228204
}
229205

230-
#[cfg(feature = "serde")]
231-
impl Serialize for NumberOf512Seconds {
232-
#[inline]
233-
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
234-
where
235-
S: Serializer,
236-
{
237-
u16::serialize(&self.to_512_second_intervals(), s)
238-
}
239-
}
240-
241-
#[cfg(feature = "serde")]
242-
impl<'de> Deserialize<'de> for NumberOf512Seconds {
243-
#[inline]
244-
fn deserialize<D>(d: D) -> Result<Self, D::Error>
245-
where
246-
D: Deserializer<'de>,
247-
{
248-
Ok(Self::from_512_second_intervals(u16::deserialize(d)?))
249-
}
250-
}
251-
252206
/// Error returned when the input time in seconds was too large to be encoded to a 16 bit 512 second interval.
253207
#[derive(Debug, Clone, PartialEq, Eq)]
254208
pub struct TimeOverflowError {
@@ -348,9 +302,6 @@ impl<'a> Arbitrary<'a> for NumberOf512Seconds {
348302

349303
#[cfg(test)]
350304
mod tests {
351-
#[cfg(feature = "serde")]
352-
use internals::serde_round_trip;
353-
354305
use super::*;
355306
use crate::BlockTime;
356307

@@ -416,22 +367,6 @@ mod tests {
416367
assert!(result.is_err());
417368
}
418369

419-
#[test]
420-
#[cfg(feature = "serde")]
421-
pub fn encode_decode_height() {
422-
serde_round_trip!(NumberOfBlocks::ZERO);
423-
serde_round_trip!(NumberOfBlocks::MIN);
424-
serde_round_trip!(NumberOfBlocks::MAX);
425-
}
426-
427-
#[test]
428-
#[cfg(feature = "serde")]
429-
pub fn encode_decode_time() {
430-
serde_round_trip!(NumberOf512Seconds::ZERO);
431-
serde_round_trip!(NumberOf512Seconds::MIN);
432-
serde_round_trip!(NumberOf512Seconds::MAX);
433-
}
434-
435370
fn generate_timestamps(start: u32, step: u16) -> [BlockTime; 11] {
436371
let mut timestamps = [BlockTime::from_u32(0); 11];
437372
for (i, ts) in timestamps.iter_mut().enumerate() {

‎units/tests/data/serde_bincode‎

-12 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)