From a8f9e8ad96a444e14ee0b10a14ff951b1f74f52c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 1 Sep 2022 16:08:12 +1000 Subject: [PATCH 1/3] Add serde roundtrip tests We have a TODO in the code to add serde roundtrip unit tests, seems reasonable so do it and remove the TODO. --- src/address.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/address.rs b/src/address.rs index 0f636b1e..60aaaa80 100644 --- a/src/address.rs +++ b/src/address.rs @@ -951,7 +951,13 @@ mod tests { "script round-trip failed for {}", addr, ); - //TODO: add serde roundtrip after no-strason PR + + #[cfg(feature = "serde")] + { + let ser = serde_json::to_string(addr).expect("failed to serialize address"); + let back: Address = serde_json::from_str(&ser).expect("failed to deserialize address"); + assert_eq!(back, *addr, "serde round-trip failed for {}", addr) + } } #[test] From 08e4b28dd0c7ead6df7144161518b2be71a93e26 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 1 Sep 2022 16:46:49 +1000 Subject: [PATCH 2/3] Add integer serialization tests Add some negative integer unit tests and remove the TODO. Note, this is not really that necessary because we are going to move to using the stdlib `to_le_bytes` methods so we don't really need any of these test except to test that we are calling the correct be/le method. They do no harm however. --- src/consensus/encode.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/consensus/encode.rs b/src/consensus/encode.rs index e1f2b41b..04b0fe01 100644 --- a/src/consensus/encode.rs +++ b/src/consensus/encode.rs @@ -1009,14 +1009,31 @@ mod tests { let failure16: Result = deserialize(&[1u8]); assert!(failure16.is_err()); + // i16 + assert_eq!(deserialize(&[0x32_u8, 0xF4]).ok(), Some(-0x0bce_i16)); + assert_eq!(deserialize(&[0xFF_u8, 0xFE]).ok(), Some(-0x0101_i16)); + assert_eq!(deserialize(&[0x00_u8, 0x00]).ok(), Some(-0_i16)); + assert_eq!(deserialize(&[0xFF_u8, 0xFA]).ok(), Some(-0x0501_i16)); + // u32 assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0]).ok(), Some(0xCDABu32)); assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD]).ok(), Some(0xCDAB0DA0u32)); + let failure32: Result = deserialize(&[1u8, 2, 3]); assert!(failure32.is_err()); - // TODO: test negative numbers + + // i32 assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0]).ok(), Some(0xCDABi32)); assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0x2D]).ok(), Some(0x2DAB0DA0i32)); + + assert_eq!(deserialize(&[0, 0, 0, 0]).ok(), Some(-0_i32)); + assert_eq!(deserialize(&[0, 0, 0, 0]).ok(), Some(0_i32)); + + assert_eq!(deserialize(&[0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-1_i32)); + assert_eq!(deserialize(&[0xFE, 0xFF, 0xFF, 0xFF]).ok(), Some(-2_i32)); + assert_eq!(deserialize(&[0x01, 0xFF, 0xFF, 0xFF]).ok(), Some(-255_i32)); + assert_eq!(deserialize(&[0x02, 0xFF, 0xFF, 0xFF]).ok(), Some(-254_i32)); + let failurei32: Result = deserialize(&[1u8, 2, 3]); assert!(failurei32.is_err()); @@ -1025,11 +1042,18 @@ mod tests { assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD, 0x99, 0, 0, 0x99]).ok(), Some(0x99000099CDAB0DA0u64)); let failure64: Result = deserialize(&[1u8, 2, 3, 4, 5, 6, 7]); assert!(failure64.is_err()); - // TODO: test negative numbers + + // i64 assert_eq!(deserialize(&[0xABu8, 0xCD, 0, 0, 0, 0, 0, 0]).ok(), Some(0xCDABi64)); assert_eq!(deserialize(&[0xA0u8, 0x0D, 0xAB, 0xCD, 0x99, 0, 0, 0x99]).ok(), Some(-0x66ffff663254f260i64)); + assert_eq!(deserialize(&[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-1_i64)); + assert_eq!(deserialize(&[0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-2_i64)); + assert_eq!(deserialize(&[0x01, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-255_i64)); + assert_eq!(deserialize(&[0x02, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF]).ok(), Some(-254_i64)); + let failurei64: Result = deserialize(&[1u8, 2, 3, 4, 5, 6, 7]); assert!(failurei64.is_err()); + } #[test] @@ -1145,6 +1169,5 @@ mod tests { ); } } - } From fea908c8afe21946bb587e6e610bff8621b53f4e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 1 Sep 2022 16:52:29 +1000 Subject: [PATCH 3/3] Add private::Sealed trait bound to SerdeAmount As suggested by the todo seal the `SerdeAmount` so users of the library explicitly cannot implement it. Its not obvious to me why this wasn't done at the time. Remove the associated TODO from the code. --- src/util/amount.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index 9813c977..88fab77b 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -1281,9 +1281,7 @@ pub mod serde { /// This trait is used only to avoid code duplication and naming collisions /// of the different serde serialization crates. - /// - /// TODO: Add the private::Sealed bound in next breaking release - pub trait SerdeAmount: Copy + Sized { + pub trait SerdeAmount: Copy + Sized + private::Sealed { fn ser_sat(self, s: S) -> Result; fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result; fn ser_btc(self, s: S) -> Result;