From 4dedf7d555b6476f6fa6e83f3b7d3e97e054982b Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 13 Aug 2022 11:42:46 +0200 Subject: [PATCH 1/2] Remove duplicated `#[cfg(test)]` conditions Several macros used in tests had `#[cfg(test)]` condition on each of them. This deduplicates them by putting them into a separate module and reexporting (with one condition for module and one for re-export). --- src/internal_macros.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/internal_macros.rs b/src/internal_macros.rs index ece0f0f6..288de32a 100644 --- a/src/internal_macros.rs +++ b/src/internal_macros.rs @@ -119,19 +119,19 @@ macro_rules! debug_from_display { pub(crate) use debug_from_display; #[cfg(test)] -macro_rules! hex_script (($s:expr) => (<$crate::Script as core::str::FromStr>::from_str($s).unwrap())); -#[cfg(test)] -pub(crate) use hex_script; +pub(crate) use test_macros::*; #[cfg(test)] -macro_rules! hex_hash (($h:ident, $s:expr) => ($h::from_slice(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); -#[cfg(test)] -pub(crate) use hex_hash; +mod test_macros { + macro_rules! hex_script (($s:expr) => (<$crate::Script as core::str::FromStr>::from_str($s).unwrap())); + pub(crate) use hex_script; -#[cfg(test)] -macro_rules! hex_decode (($h:ident, $s:expr) => (deserialize::<$h>(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); -#[cfg(test)] -pub(crate) use hex_decode; + macro_rules! hex_hash (($h:ident, $s:expr) => ($h::from_slice(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); + pub(crate) use hex_hash; + + macro_rules! hex_decode (($h:ident, $s:expr) => (deserialize::<$h>(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); + pub(crate) use hex_decode; +} macro_rules! serde_string_impl { ($name:ident, $expecting:literal) => { From 43827a1e1c6125cc6c1c09e031b176a252af3c77 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 13 Aug 2022 13:29:18 +0200 Subject: [PATCH 2/2] Hex macro clean up There were some duplicated hex macros in the `address` module as well as several macros outputing specific types instead of working with any type deserializable from hex. This cleanup moves the macros and deduplicates them, introduces a new `TestFromHex` trait to generalize parsing hex and rewrites the code to use hex parsing generally. `hex_script` is still kept because its usage is very frequent. --- src/address.rs | 24 ++++++------------ src/internal_macros.rs | 55 ++++++++++++++++++++++++++++++++++++++--- src/util/key.rs | 1 + src/util/sighash.rs | 56 +++++++++++++++++++++--------------------- 4 files changed, 88 insertions(+), 48 deletions(-) diff --git a/src/address.rs b/src/address.rs index dc77cf4a..0f636b1e 100644 --- a/src/address.rs +++ b/src/address.rs @@ -933,17 +933,11 @@ mod tests { use secp256k1::XOnlyPublicKey; use super::*; - use crate::blockdata::script::Script; use crate::hashes::hex::{FromHex, ToHex}; + use crate::internal_macros::{hex, hex_into, hex_script}; use crate::network::constants::Network::{Bitcoin, Testnet}; use crate::util::key::PublicKey; - macro_rules! hex (($hex:literal) => (Vec::from_hex($hex).unwrap())); - macro_rules! hex_key (($hex:literal) => (PublicKey::from_slice(&hex!($hex)).unwrap())); - macro_rules! hex_script (($hex:literal) => (Script::from(hex!($hex)))); - macro_rules! hex_pubkeyhash (($hex:literal) => (PubkeyHash::from_hex(&$hex).unwrap())); - macro_rules! hex_scripthash (($hex:literal) => (ScriptHash::from_hex($hex).unwrap())); - fn roundtrips(addr: &Address) { assert_eq!( Address::from_str(&addr.to_string()).unwrap(), @@ -964,9 +958,7 @@ mod tests { fn test_p2pkh_address_58() { let addr = Address { network: Bitcoin, - payload: Payload::PubkeyHash(hex_pubkeyhash!( - "162c5ea71c0b23f5b9022ef047c4a86470a5b070" - )), + payload: Payload::PubkeyHash(hex_into!("162c5ea71c0b23f5b9022ef047c4a86470a5b070")), }; assert_eq!( @@ -980,11 +972,11 @@ mod tests { #[test] fn test_p2pkh_from_key() { - let key = hex_key!("048d5141948c1702e8c95f438815794b87f706a8d4cd2bffad1dc1570971032c9b6042a0431ded2478b5c9cf2d81c124a5e57347a3c63ef0e7716cf54d613ba183"); + let key = hex_into!("048d5141948c1702e8c95f438815794b87f706a8d4cd2bffad1dc1570971032c9b6042a0431ded2478b5c9cf2d81c124a5e57347a3c63ef0e7716cf54d613ba183"); let addr = Address::p2pkh(&key, Bitcoin); assert_eq!(&addr.to_string(), "1QJVDzdqb1VpbDK7uDeyVXy9mR27CJiyhY"); - let key = hex_key!("03df154ebfcf29d29cc10d5c2565018bce2d9edbab267c31d2caf44a63056cf99f"); + let key = hex_into!("03df154ebfcf29d29cc10d5c2565018bce2d9edbab267c31d2caf44a63056cf99f"); let addr = Address::p2pkh(&key, Testnet); assert_eq!(&addr.to_string(), "mqkhEMH6NCeYjFybv7pvFC22MFeaNT9AQC"); assert_eq!(addr.address_type(), Some(AddressType::P2pkh)); @@ -995,9 +987,7 @@ mod tests { fn test_p2sh_address_58() { let addr = Address { network: Bitcoin, - payload: Payload::ScriptHash(hex_scripthash!( - "162c5ea71c0b23f5b9022ef047c4a86470a5b070" - )), + payload: Payload::ScriptHash(hex_into!("162c5ea71c0b23f5b9022ef047c4a86470a5b070")), }; assert_eq!( @@ -1028,7 +1018,7 @@ mod tests { fn test_p2wpkh() { // stolen from Bitcoin transaction: b3c8c2b6cfc335abbcb2c7823a8453f55d64b2b5125a9a61e8737230cdb8ce20 let mut key = - hex_key!("033bc8c83c52df5712229a2f72206d90192366c36428cb0c12b6af98324d97bfbc"); + hex_into!("033bc8c83c52df5712229a2f72206d90192366c36428cb0c12b6af98324d97bfbc"); let addr = Address::p2wpkh(&key, Bitcoin).unwrap(); assert_eq!(&addr.to_string(), "bc1qvzvkjn4q3nszqxrv3nraga2r822xjty3ykvkuw"); assert_eq!(addr.address_type(), Some(AddressType::P2wpkh)); @@ -1056,7 +1046,7 @@ mod tests { fn test_p2shwpkh() { // stolen from Bitcoin transaction: ad3fd9c6b52e752ba21425435ff3dd361d6ac271531fc1d2144843a9f550ad01 let mut key = - hex_key!("026c468be64d22761c30cd2f12cbc7de255d592d7904b1bab07236897cc4c2e766"); + hex_into!("026c468be64d22761c30cd2f12cbc7de255d592d7904b1bab07236897cc4c2e766"); let addr = Address::p2shwpkh(&key, Bitcoin).unwrap(); assert_eq!(&addr.to_string(), "3QBRmWNqqBGme9er7fMkGqtZtp4gjMFxhE"); assert_eq!(addr.address_type(), Some(AddressType::P2sh)); diff --git a/src/internal_macros.rs b/src/internal_macros.rs index 288de32a..26ff764e 100644 --- a/src/internal_macros.rs +++ b/src/internal_macros.rs @@ -123,11 +123,60 @@ pub(crate) use test_macros::*; #[cfg(test)] mod test_macros { - macro_rules! hex_script (($s:expr) => (<$crate::Script as core::str::FromStr>::from_str($s).unwrap())); + use crate::hashes::hex::FromHex; + use crate::PublicKey; + + /// Trait used to create a value from hex string for testing purposes. + pub(crate) trait TestFromHex { + /// Produces the value from hex. + /// + /// ## Panics + /// + /// The function panics if the hex or the value is invalid. + fn test_from_hex(hex: &str) -> Self; + } + + impl TestFromHex for T { + fn test_from_hex(hex: &str) -> Self { Self::from_hex(hex).unwrap() } + } + + impl TestFromHex for PublicKey { + fn test_from_hex(hex: &str) -> Self { + PublicKey::from_slice(&Vec::from_hex(hex).unwrap()).unwrap() + } + } + + macro_rules! hex (($hex:literal) => (Vec::from_hex($hex).unwrap())); + pub(crate) use hex; + + macro_rules! hex_into { + ($hex:expr) => { + $crate::internal_macros::hex_into!(_, $hex) + }; + ($type:ty, $hex:expr) => { + <$type as $crate::internal_macros::TestFromHex>::test_from_hex($hex) + }; + } + pub(crate) use hex_into; + + // Script is commonly used in places where inference may fail + macro_rules! hex_script (($hex:expr) => ($crate::internal_macros::hex_into!($crate::Script, $hex))); pub(crate) use hex_script; - macro_rules! hex_hash (($h:ident, $s:expr) => ($h::from_slice(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); - pub(crate) use hex_hash; + // For types that can't use TestFromHex due to coherence rules or reversed hex + macro_rules! hex_from_slice { + ($hex:expr) => { + $crate::internal_macros::hex_from_slice!(_, $hex) + }; + ($type:ty, $hex:expr) => { + <$type>::from_slice( + &<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($hex) + .unwrap(), + ) + .unwrap() + }; + } + pub(crate) use hex_from_slice; macro_rules! hex_decode (($h:ident, $s:expr) => (deserialize::<$h>(&<$crate::prelude::Vec as $crate::hashes::hex::FromHex>::from_hex($s).unwrap()).unwrap())); pub(crate) use hex_decode; diff --git a/src/util/key.rs b/src/util/key.rs index c4dce143..52f72a10 100644 --- a/src/util/key.rs +++ b/src/util/key.rs @@ -263,6 +263,7 @@ impl PublicKey { pub fn from_private_key(secp: &Secp256k1, sk: &PrivateKey) -> PublicKey { sk.public_key(secp) } + } /// An opaque return type for PublicKey::to_sort_key diff --git a/src/util/sighash.rs b/src/util/sighash.rs index b3bdb592..42b4aa68 100644 --- a/src/util/sighash.rs +++ b/src/util/sighash.rs @@ -1104,7 +1104,7 @@ mod tests { use crate::hashes::hex::{FromHex, ToHex}; use crate::hashes::{Hash, HashEngine}; use crate::hash_types::Sighash; - use crate::internal_macros::{hex_hash, hex_script, hex_decode}; + use crate::internal_macros::{hex_into, hex_script, hex_decode, hex_from_slice}; use crate::network::constants::Network; use crate::util::key::PublicKey; use crate::util::sighash::{Annex, Error, Prevouts, ScriptPath, SighashCache}; @@ -1447,19 +1447,19 @@ mod tests { for inp in key_path["inputSpending"].as_array().unwrap() { let tx_ind = inp["given"]["txinIndex"].as_u64().unwrap() as usize; - let internal_priv_key = hex_hash!(SecretKey, inp["given"]["internalPrivkey"].as_str().unwrap()); + let internal_priv_key = hex_from_slice!(SecretKey, inp["given"]["internalPrivkey"].as_str().unwrap()); let merkle_root = if inp["given"]["merkleRoot"].is_null() { None } else { - Some(hex_hash!(TapBranchHash, inp["given"]["merkleRoot"].as_str().unwrap())) + Some(hex_into!(TapBranchHash, inp["given"]["merkleRoot"].as_str().unwrap())) }; let hash_ty = SchnorrSighashType::from_consensus_u8(inp["given"]["hashType"].as_u64().unwrap() as u8).unwrap(); - let expected_internal_pk = hex_hash!(XOnlyPublicKey, inp["intermediary"]["internalPubkey"].as_str().unwrap()); - let expected_tweak = hex_hash!(TapTweakHash, inp["intermediary"]["tweak"].as_str().unwrap()); - let expected_tweaked_priv_key = hex_hash!(SecretKey, inp["intermediary"]["tweakedPrivkey"].as_str().unwrap()); + let expected_internal_pk = hex_from_slice!(XOnlyPublicKey, inp["intermediary"]["internalPubkey"].as_str().unwrap()); + let expected_tweak = hex_into!(TapTweakHash, inp["intermediary"]["tweak"].as_str().unwrap()); + let expected_tweaked_priv_key = hex_from_slice!(SecretKey, inp["intermediary"]["tweakedPrivkey"].as_str().unwrap()); let expected_sig_msg = Vec::::from_hex(inp["intermediary"]["sigMsg"].as_str().unwrap()).unwrap(); - let expected_sighash = hex_hash!(TapSighashHash, inp["intermediary"]["sigHash"].as_str().unwrap()); + let expected_sighash = hex_into!(TapSighashHash, inp["intermediary"]["sigHash"].as_str().unwrap()); let sig_str = inp["expected"]["witness"][0].as_str().unwrap(); let (expected_key_spend_sig, expected_hash_ty) = if sig_str.len() == 128 { (secp256k1::schnorr::Signature::from_str(sig_str).unwrap(), SchnorrSighashType::Default) @@ -1566,18 +1566,18 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(1, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_hash!(Sighash, "c37af31116d1b27caf68aae9e3ac82f1477929014d5b917657d0eb49478cb670") + hex_from_slice!(Sighash, "c37af31116d1b27caf68aae9e3ac82f1477929014d5b917657d0eb49478cb670") ); let cache = cache.segwit_cache(); - assert_eq!(cache.prevouts, hex_hash!( - Hash, "96b827c8483d4e9b96712b6713a7b68d6e8003a781feba36c31143470b4efd37" + assert_eq!(cache.prevouts, hex_from_slice!( + "96b827c8483d4e9b96712b6713a7b68d6e8003a781feba36c31143470b4efd37" )); - assert_eq!(cache.sequences, hex_hash!( - Hash, "52b0a642eea2fb7ae638c36f6252b6750293dbe574a806984b8e4d8548339a3b" + assert_eq!(cache.sequences, hex_from_slice!( + "52b0a642eea2fb7ae638c36f6252b6750293dbe574a806984b8e4d8548339a3b" )); - assert_eq!(cache.outputs, hex_hash!( - Hash, "863ef3e1a92afbfdb97f31ad0fc7683ee943e9abcf2501590ff8f6551f47e5e5" + assert_eq!(cache.outputs, hex_from_slice!( + "863ef3e1a92afbfdb97f31ad0fc7683ee943e9abcf2501590ff8f6551f47e5e5" )); } @@ -1597,18 +1597,18 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(0, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_hash!(Sighash, "64f3b0f4dd2bb3aa1ce8566d220cc74dda9df97d8490cc81d89d735c92e59fb6") + hex_from_slice!(Sighash, "64f3b0f4dd2bb3aa1ce8566d220cc74dda9df97d8490cc81d89d735c92e59fb6") ); let cache = cache.segwit_cache(); - assert_eq!(cache.prevouts, hex_hash!( - Hash, "b0287b4a252ac05af83d2dcef00ba313af78a3e9c329afa216eb3aa2a7b4613a" + assert_eq!(cache.prevouts, hex_from_slice!( + "b0287b4a252ac05af83d2dcef00ba313af78a3e9c329afa216eb3aa2a7b4613a" )); - assert_eq!(cache.sequences, hex_hash!( - Hash, "18606b350cd8bf565266bc352f0caddcf01e8fa789dd8a15386327cf8cabe198" + assert_eq!(cache.sequences, hex_from_slice!( + "18606b350cd8bf565266bc352f0caddcf01e8fa789dd8a15386327cf8cabe198" )); - assert_eq!(cache.outputs, hex_hash!( - Hash, "de984f44532e2173ca0d64314fcefe6d30da6f8cf27bafa706da61df8a226c83" + assert_eq!(cache.outputs, hex_from_slice!( + "de984f44532e2173ca0d64314fcefe6d30da6f8cf27bafa706da61df8a226c83" )); } @@ -1634,18 +1634,18 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(0, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_hash!(Sighash, "185c0be5263dce5b4bb50a047973c1b6272bfbd0103a89444597dc40b248ee7c") + hex_from_slice!(Sighash, "185c0be5263dce5b4bb50a047973c1b6272bfbd0103a89444597dc40b248ee7c") ); let cache = cache.segwit_cache(); - assert_eq!(cache.prevouts, hex_hash!( - Hash, "74afdc312af5183c4198a40ca3c1a275b485496dd3929bca388c4b5e31f7aaa0" + assert_eq!(cache.prevouts, hex_from_slice!( + "74afdc312af5183c4198a40ca3c1a275b485496dd3929bca388c4b5e31f7aaa0" )); - assert_eq!(cache.sequences, hex_hash!( - Hash, "3bb13029ce7b1f559ef5e747fcac439f1455a2ec7c5f09b72290795e70665044" + assert_eq!(cache.sequences, hex_from_slice!( + "3bb13029ce7b1f559ef5e747fcac439f1455a2ec7c5f09b72290795e70665044" )); - assert_eq!(cache.outputs, hex_hash!( - Hash, "bc4d309071414bed932f98832b27b4d76dad7e6c1346f487a8fdbb8eb90307cc" + assert_eq!(cache.outputs, hex_from_slice!( + "bc4d309071414bed932f98832b27b4d76dad7e6c1346f487a8fdbb8eb90307cc" )); } }