Merge rust-bitcoin/rust-bitcoin#2889: Move `serde_round_trip` macro to internals

7fa53440dc Move serde_round_trip macro to internals (Tobin C. Harding)

Pull request description:

  We currently duplicate the serde_round_trip macro in `units` and `bitcoin`, this is unnecessary since it is a private test macro we can just throw it in `internals`.

  While we are at it lets improve the macro by testing a binary encoding also, elect to use the `bincode` crate because we already have it in our dependency graph.

  Add `test-serde` feature to `internals` to feature gate the macro and its usage (preventing the transient dependency on `bincode` and `serde_json`).

ACKs for top commit:
  Kixunil:
    ACK 7fa53440dc
  apoelstra:
    ACK 7fa53440dc

Tree-SHA512: f40c78bf2539940b7836ed413d5fe96ce4e9ce59bad7b3f86d831971320d1c2effcd23d0da5c785d6c372a2c6962bf720080ec4351248fbbdc0f2cfb4ffd602c

Cargo-minimal.lock

@@ -86,7 +86,9 @@ dependencies = [
 name = "bitcoin-internals"
 version = "0.3.0"
 dependencies = [
+ "bincode",
  "serde",
+ "serde_json",
 ]
 
 [[package]]

Cargo-recent.lock

@@ -85,7 +85,9 @@ dependencies = [
 name = "bitcoin-internals"
 version = "0.3.0"
 dependencies = [
+ "bincode",
  "serde",
+ "serde_json",
 ]
 
 [[package]]

bitcoin/Cargo.toml

@@ -43,6 +43,7 @@ bitcoinconsensus = { version = "0.106.0+26", default-features = false, optional
 actual-serde = { package = "serde", version = "1.0.103", default-features = false, features = [ "derive", "alloc" ], optional = true }
 
 [dev-dependencies]
+internals = { package = "bitcoin-internals", version = "0.3.0", features = ["test-serde"] }
 serde_json = "1.0.0"
 serde_test = "1.0.19"
 bincode = "1.3.1"

bitcoin/src/bip32.rs

@@ -917,6 +917,8 @@ impl std::error::Error for InvalidBase58PayloadLengthError {}
 #[cfg(test)]
 mod tests {
     use hex::test_hex_unwrap as hex;
+    #[cfg(feature = "serde")]
+    use internals::serde_round_trip;
 
     use super::ChildNumber::{Hardened, Normal};
     use super::*;

bitcoin/src/blockdata/transaction.rs

@@ -1633,6 +1633,8 @@ mod tests {
     use core::str::FromStr;
 
     use hex::{test_hex_unwrap as hex, FromHex};
+    #[cfg(feature = "serde")]
+    use internals::serde_round_trip;
 
     use super::*;
     use crate::consensus::encode::{deserialize, serialize};

bitcoin/src/lib.rs

@@ -81,9 +81,6 @@ pub extern crate secp256k1;
 #[macro_use]
 extern crate actual_serde as serde;
 
-#[cfg(test)]
-#[macro_use]
-mod test_macros;
 mod internal_macros;
 #[cfg(feature = "serde")]
 mod serde_utils;

bitcoin/src/test_macros.rs

@@ -1,16 +0,0 @@
-// 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);
-    })
-);

internals/Cargo.toml

@@ -18,9 +18,15 @@ default = []
 std = ["alloc"]
 alloc = []
 
+test-serde = ["serde/derive", "serde_json", "bincode"]
+
 [dependencies]
 serde = { version = "1.0.103", default-features = false, optional = true }
 
+# Don't enable these directly, use `test-serde` feature instead.
+serde_json = { version = "1.0.68", optional = true }
+bincode = { version = "1.3.1", optional = true }
+
 [dev-dependencies]
 
 [package.metadata.docs.rs]

internals/src/lib.rs

@@ -21,10 +21,17 @@ extern crate alloc;
 #[cfg(feature = "std")]
 extern crate std;
 
+#[cfg(feature = "test-serde")]
+pub extern crate serde_json;
+
+#[cfg(feature = "test-serde")]
+pub extern crate bincode;
+
 pub mod array_vec;
 pub mod const_tools;
 pub mod error;
 pub mod macros;
 mod parse;
 #[cfg(feature = "serde")]
+#[macro_use]
 pub mod serde;

internals/src/serde.rs

@@ -293,3 +293,20 @@ macro_rules! serde_struct_human_string_impl {
         }
     )
 }
+
+/// Does round trip test to/from serde value.
+#[cfg(feature = "test-serde")]
+#[macro_export]
+macro_rules! serde_round_trip (
+    ($var:expr) => ({
+        use serde_json;
+
+        let encoded = $crate::serde_json::to_value(&$var).expect("serde_json failed to encode");
+        let decoded = $crate::serde_json::from_value(encoded).expect("serde_json failed to decode");
+        assert_eq!($var, decoded);
+
+        let encoded = $crate::bincode::serialize(&$var).expect("bincode failed to encode");
+        let decoded = $crate::bincode::deserialize(&encoded).expect("bincode failed to decode");
+        assert_eq!($var, decoded);
+    })
+);

units/Cargo.toml

@@ -23,6 +23,7 @@ internals = { package = "bitcoin-internals", version = "0.3.0" }
 serde = { version = "1.0.103", default-features = false, features = ["derive"], optional = true }
 
 [dev-dependencies]
+internals = { package = "bitcoin-internals", version = "0.3.0", features = ["test-serde"] }
 serde_test = "1.0"
 serde_json = "1.0"
 

units/src/lib.rs

@@ -31,10 +31,6 @@ 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 block;

units/src/locktime/absolute.rs

@@ -373,6 +373,8 @@ impl From<ConversionError> for ParseError {
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[cfg(feature = "serde")]
+    use internals::serde_round_trip;
 
     #[test]
     fn time_from_str_hex_happy_path() {

units/src/test_macros.rs

@@ -1,16 +0,0 @@
-// 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);
-    })
-);