From 32ca6cc3200beaaf5eab48e08a715d4e53a1313b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 20 Jan 2023 14:44:13 +1100 Subject: [PATCH] Remove hex_from_slice and display Sighash forwards `Sighash` should be displayed forwards according to BIP143. Currently we are displaying it backwards (as we do for double SHA256). This is working because parse using `Vec::from_hex`. We have the means to parse hex strings directly for hashes, we no longer need `hex_from_slice`. BIP143 test vectors display double SHA256 forwards but we display backwards, this is acceptable because there is no fixed display in the ecosystem for double SHA256 hashes. In order to overcome this we parse test vector hex strings with into `Vec` when needed. --- bitcoin/src/hash_types.rs | 2 +- bitcoin/src/internal_macros.rs | 30 ------------------ bitcoin/src/psbt/mod.rs | 2 +- bitcoin/src/sighash.rs | 56 +++++++++++++++------------------- 4 files changed, 27 insertions(+), 63 deletions(-) diff --git a/bitcoin/src/hash_types.rs b/bitcoin/src/hash_types.rs index edcbd98a..c7a32e41 100644 --- a/bitcoin/src/hash_types.rs +++ b/bitcoin/src/hash_types.rs @@ -45,7 +45,7 @@ See [`hashes::Hash::DISPLAY_BACKWARD`] for more details. "); hash_newtype!(Wtxid, sha256d::Hash, 32, doc="A bitcoin witness transaction ID."); hash_newtype!(BlockHash, sha256d::Hash, 32, doc="A bitcoin block hash."); - hash_newtype!(Sighash, sha256d::Hash, 32, doc="Hash of the transaction according to the signature algorithm"); + hash_newtype!(Sighash, sha256d::Hash, 32, doc="Hash of the transaction according to the signature algorithm", false); impl secp256k1::ThirtyTwoByteHash for Sighash { fn into_32(self) -> [u8; 32] { use hashes::Hash; diff --git a/bitcoin/src/internal_macros.rs b/bitcoin/src/internal_macros.rs index 4a838472..1c657938 100644 --- a/bitcoin/src/internal_macros.rs +++ b/bitcoin/src/internal_macros.rs @@ -51,39 +51,9 @@ pub(crate) use test_macros::*; #[cfg(test)] mod test_macros { - use crate::hashes::hex::FromHex; - - /// 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() } - } macro_rules! hex (($hex:expr) => ( as hashes::hex::FromHex>::from_hex($hex).unwrap())); pub(crate) use hex; - - // 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; } /// Implements several traits for byte-based newtypes. diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index f493f0b0..9d83ca90 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -1397,7 +1397,7 @@ mod tests { #[cfg(feature = "std")] assert_eq!(err.to_string(), "hash parse error"); #[cfg(not(feature = "std"))] - assert_eq!(err.to_string(), "hash parse error: bad slice length 33 (expected 32)"); + assert_eq!(err.to_string(), "hash parse error: invalid slice length 33 (expected 32)"); let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b69241142cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2cd970e15f53fc0c82f950fd560ffa919b76172be017368a89913af074f400b094289756aa3739ccc689ec0fcf3a360be32cc0b59b16e93a1e8bb4605726b2ca7a3ff706c4176649632b2cc68e1f912b8a578e3719ce7710885c7a966f49bcd43cb01010000").unwrap_err(); #[cfg(feature = "std")] assert_eq!(err.to_string(), "invalid Schnorr signature"); diff --git a/bitcoin/src/sighash.rs b/bitcoin/src/sighash.rs index 226df4e7..39e4a1a3 100644 --- a/bitcoin/src/sighash.rs +++ b/bitcoin/src/sighash.rs @@ -1057,7 +1057,7 @@ mod tests { use crate::hash_types::Sighash; use crate::hashes::hex::FromHex; use crate::hashes::{Hash, HashEngine}; - use crate::internal_macros::{hex, hex_from_slice}; + use crate::internal_macros::hex; use crate::network::constants::Network; use crate::taproot::{TapLeafHash, TapSighashHash}; @@ -1621,24 +1621,22 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(1, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_from_slice!( - Sighash, - "c37af31116d1b27caf68aae9e3ac82f1477929014d5b917657d0eb49478cb670" - ) + "c37af31116d1b27caf68aae9e3ac82f1477929014d5b917657d0eb49478cb670".parse::().unwrap(), ); let cache = cache.segwit_cache(); + // Parse hex into Vec because BIP143 test vector displays forwards but our sha256d::Hash displays backwards. assert_eq!( - cache.prevouts, - hex_from_slice!("96b827c8483d4e9b96712b6713a7b68d6e8003a781feba36c31143470b4efd37") + cache.prevouts.into_inner().as_ref(), + &Vec::from_hex("96b827c8483d4e9b96712b6713a7b68d6e8003a781feba36c31143470b4efd37").unwrap()[..], ); assert_eq!( - cache.sequences, - hex_from_slice!("52b0a642eea2fb7ae638c36f6252b6750293dbe574a806984b8e4d8548339a3b") + cache.sequences.into_inner().as_ref(), + &Vec::from_hex("52b0a642eea2fb7ae638c36f6252b6750293dbe574a806984b8e4d8548339a3b").unwrap()[..], ); assert_eq!( - cache.outputs, - hex_from_slice!("863ef3e1a92afbfdb97f31ad0fc7683ee943e9abcf2501590ff8f6551f47e5e5") + cache.outputs.into_inner().as_ref(), + &Vec::from_hex("863ef3e1a92afbfdb97f31ad0fc7683ee943e9abcf2501590ff8f6551f47e5e5").unwrap()[..], ); } @@ -1659,24 +1657,22 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(0, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_from_slice!( - Sighash, - "64f3b0f4dd2bb3aa1ce8566d220cc74dda9df97d8490cc81d89d735c92e59fb6" - ) + "64f3b0f4dd2bb3aa1ce8566d220cc74dda9df97d8490cc81d89d735c92e59fb6".parse::().unwrap(), ); let cache = cache.segwit_cache(); + // Parse hex into Vec because BIP143 test vector displays forwards but our sha256d::Hash displays backwards. assert_eq!( - cache.prevouts, - hex_from_slice!("b0287b4a252ac05af83d2dcef00ba313af78a3e9c329afa216eb3aa2a7b4613a") + cache.prevouts.into_inner().as_ref(), + &Vec::from_hex("b0287b4a252ac05af83d2dcef00ba313af78a3e9c329afa216eb3aa2a7b4613a").unwrap()[..], ); assert_eq!( - cache.sequences, - hex_from_slice!("18606b350cd8bf565266bc352f0caddcf01e8fa789dd8a15386327cf8cabe198") + cache.sequences.into_inner().as_ref(), + &Vec::from_hex("18606b350cd8bf565266bc352f0caddcf01e8fa789dd8a15386327cf8cabe198").unwrap()[..], ); assert_eq!( - cache.outputs, - hex_from_slice!("de984f44532e2173ca0d64314fcefe6d30da6f8cf27bafa706da61df8a226c83") + cache.outputs.into_inner().as_ref(), + &Vec::from_hex("de984f44532e2173ca0d64314fcefe6d30da6f8cf27bafa706da61df8a226c83").unwrap()[..], ); } @@ -1702,24 +1698,22 @@ mod tests { let mut cache = SighashCache::new(&tx); assert_eq!( cache.segwit_signature_hash(0, &witness_script, value, EcdsaSighashType::All).unwrap(), - hex_from_slice!( - Sighash, - "185c0be5263dce5b4bb50a047973c1b6272bfbd0103a89444597dc40b248ee7c" - ) + "185c0be5263dce5b4bb50a047973c1b6272bfbd0103a89444597dc40b248ee7c".parse::().unwrap(), ); let cache = cache.segwit_cache(); + // Parse hex into Vec because BIP143 test vector displays forwards but our sha256d::Hash displays backwards. assert_eq!( - cache.prevouts, - hex_from_slice!("74afdc312af5183c4198a40ca3c1a275b485496dd3929bca388c4b5e31f7aaa0") + cache.prevouts.into_inner().as_ref(), + &Vec::from_hex("74afdc312af5183c4198a40ca3c1a275b485496dd3929bca388c4b5e31f7aaa0").unwrap()[..], ); assert_eq!( - cache.sequences, - hex_from_slice!("3bb13029ce7b1f559ef5e747fcac439f1455a2ec7c5f09b72290795e70665044") + cache.sequences.into_inner().as_ref(), + &Vec::from_hex("3bb13029ce7b1f559ef5e747fcac439f1455a2ec7c5f09b72290795e70665044").unwrap()[..], ); assert_eq!( - cache.outputs, - hex_from_slice!("bc4d309071414bed932f98832b27b4d76dad7e6c1346f487a8fdbb8eb90307cc") + cache.outputs.into_inner().as_ref(), + &Vec::from_hex("bc4d309071414bed932f98832b27b4d76dad7e6c1346f487a8fdbb8eb90307cc").unwrap()[..], ); } }