From 216422dffcc7736548d37e3449ef76da1431aa6d Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 23 Apr 2024 12:40:26 +1000 Subject: [PATCH 1/5] Remove schemars impl for test type We do not test the schemars stuff in `hashes`, instead we do it in a separate crate `extended_tests/schemars`. There is therefore no reason to implement `schemars::JsonSchema` for the `TestHashTag`. --- hashes/src/sha256t.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 1a613cd39..abb8162ab 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -185,21 +185,6 @@ mod tests { } } - #[cfg(feature = "schemars")] - impl schemars::JsonSchema for TestHashTag { - fn schema_name() -> String { "Hash".to_owned() } - - fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { - let mut schema: schemars::schema::SchemaObject = ::json_schema(gen).into(); - schema.string = Some(Box::new(schemars::schema::StringValidation { - max_length: Some(64 * 2), - min_length: Some(64 * 2), - pattern: Some("[0-9a-fA-F]+".to_owned()), - })); - schema.into() - } - } - /// A hash tagged with `$name`. #[cfg(feature = "alloc")] pub type TestHash = sha256t::Hash; From 9aee65d1baf624978c2e767b059a800cc19681b5 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 23 Apr 2024 12:46:37 +1000 Subject: [PATCH 2/5] Refactor tagged hash tests In the tagged hash unit tests we are testing two separate things in a single test. To improve maintainability separate the test into two. Refactor only, no test coverage change. --- hashes/src/sha256t.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index abb8162ab..51809f52e 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -174,6 +174,10 @@ mod tests { 108, 71, 99, 110, 96, 125, 179, 62, 234, 221, 198, 240, 201, ]; + // The digest created by sha256 hashing `&[0]` starting with `TEST_MIDSTATE`. + #[cfg(feature = "alloc")] + const HASH_ZERO: &str = "29589d5122ec666ab5b4695070b6debc63881a4f85d88d93ddc90078038213ed"; + #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default, Hash)] pub struct TestHashTag; @@ -185,10 +189,17 @@ mod tests { } } - /// A hash tagged with `$name`. + // We support manually implementing `Tag` and creating a tagged hash from it. #[cfg(feature = "alloc")] pub type TestHash = sha256t::Hash; + #[test] + #[cfg(feature = "alloc")] + fn manually_created_sha256t_hash_type() { + assert_eq!(TestHash::hash(&[0]).to_string(), HASH_ZERO); + } + + // We also provide a macro to create the tag and the hash type. sha256t_hash_newtype! { /// Test detailed explanation. struct NewTypeTag = raw(TEST_MIDSTATE, 64); @@ -200,14 +211,7 @@ mod tests { #[test] #[cfg(feature = "alloc")] - fn test_sha256t() { - assert_eq!( - TestHash::hash(&[0]).to_string(), - "29589d5122ec666ab5b4695070b6debc63881a4f85d88d93ddc90078038213ed" - ); - assert_eq!( - NewTypeHash::hash(&[0]).to_string(), - "29589d5122ec666ab5b4695070b6debc63881a4f85d88d93ddc90078038213ed" - ); + fn macro_created_sha256t_hash_type() { + assert_eq!(NewTypeHash::hash(&[0]).to_string(), HASH_ZERO); } } From 5ecc69cd28b7cd3fc34ce0d52622f3e177a3bb02 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 23 Apr 2024 12:54:38 +1000 Subject: [PATCH 3/5] Add forward/backward unit test Add a unit test to verify that the forward/backward functionality of the `sha256t_hash_newtype` works as advertised. --- hashes/src/sha256t.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 51809f52e..6a32831c1 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -176,7 +176,10 @@ mod tests { // The digest created by sha256 hashing `&[0]` starting with `TEST_MIDSTATE`. #[cfg(feature = "alloc")] - const HASH_ZERO: &str = "29589d5122ec666ab5b4695070b6debc63881a4f85d88d93ddc90078038213ed"; + const HASH_ZERO_BACKWARD: &str = "29589d5122ec666ab5b4695070b6debc63881a4f85d88d93ddc90078038213ed"; + // And the same thing, forward. + #[cfg(feature = "alloc")] + const HASH_ZERO_FORWARD: &str = "ed1382037800c9dd938dd8854f1a8863bcdeb6705069b4b56a66ec22519d5829"; #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default, Hash)] pub struct TestHashTag; @@ -196,22 +199,38 @@ mod tests { #[test] #[cfg(feature = "alloc")] fn manually_created_sha256t_hash_type() { - assert_eq!(TestHash::hash(&[0]).to_string(), HASH_ZERO); + assert_eq!(TestHash::hash(&[0]).to_string(), HASH_ZERO_BACKWARD); } // We also provide a macro to create the tag and the hash type. sha256t_hash_newtype! { /// Test detailed explanation. - struct NewTypeTag = raw(TEST_MIDSTATE, 64); + struct NewTypeTagBackward = raw(TEST_MIDSTATE, 64); /// A test hash. #[hash_newtype(backward)] - struct NewTypeHash(_); + struct NewTypeHashBackward(_); } #[test] #[cfg(feature = "alloc")] - fn macro_created_sha256t_hash_type() { - assert_eq!(NewTypeHash::hash(&[0]).to_string(), HASH_ZERO); + fn macro_created_sha256t_hash_type_backward() { + assert_eq!(NewTypeHashBackward::hash(&[0]).to_string(), HASH_ZERO_BACKWARD); + } + + // We also provide a macro to create the tag and the hash type. + sha256t_hash_newtype! { + /// Test detailed explanation. + struct NewTypeTagForward = raw(TEST_MIDSTATE, 64); + + /// A test hash. + #[hash_newtype(forward)] + struct NewTypeHashForward(_); + } + + #[test] + #[cfg(feature = "alloc")] + fn macro_created_sha256t_hash_type_prints_forward() { + assert_eq!(NewTypeHashForward::hash(&[0]).to_string(), HASH_ZERO_FORWARD); } } From 30e91cc766b6b1523bb8e4c93dbe74a45c097053 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 23 Apr 2024 12:58:14 +1000 Subject: [PATCH 4/5] Default to forward for tagged hashes Displaying backward is an anomaly of Bitcoin Core's early days and the double SHA256 hash type. We should not let this unfortunate beast leak out into other places. Default to displaying forward when creating a new tagged hash and remove all the explicit attributes from `bitcoin` that just clutter the code. --- bitcoin/src/crypto/sighash.rs | 1 - bitcoin/src/taproot/mod.rs | 3 --- hashes/src/sha256t.rs | 4 ++-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index b786c9836..56d07e69b 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -61,7 +61,6 @@ sha256t_hash_newtype! { /// Taproot-tagged hash with tag \"TapSighash\". /// /// This hash type is used for computing taproot signature hash." - #[hash_newtype(forward)] pub struct TapSighash(_); } diff --git a/bitcoin/src/taproot/mod.rs b/bitcoin/src/taproot/mod.rs index 60ce28ee0..87318ada4 100644 --- a/bitcoin/src/taproot/mod.rs +++ b/bitcoin/src/taproot/mod.rs @@ -36,7 +36,6 @@ sha256t_hash_newtype! { /// Taproot-tagged hash with tag \"TapLeaf\". /// /// This is used for computing tapscript script spend hash. - #[hash_newtype(forward)] pub struct TapLeafHash(_); pub struct TapBranchTag = hash_str("TapBranch"); @@ -44,7 +43,6 @@ sha256t_hash_newtype! { /// Tagged hash used in taproot trees. /// /// See BIP-340 for tagging rules. - #[hash_newtype(forward)] pub struct TapNodeHash(_); pub struct TapTweakTag = hash_str("TapTweak"); @@ -52,7 +50,6 @@ sha256t_hash_newtype! { /// Taproot-tagged hash with tag \"TapTweak\". /// /// This hash type is used while computing the tweaked public key. - #[hash_newtype(forward)] pub struct TapTweakHash(_); } diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 6a32831c1..6627048cc 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -66,7 +66,7 @@ impl core::hash::Hash for Hash { fn hash(&self, h: &mut H) { self.0.hash(h) } } -crate::internal_macros::hash_trait_impls!(256, true, T: Tag); +crate::internal_macros::hash_trait_impls!(256, false, T: Tag); fn from_engine(e: sha256::HashEngine) -> Hash { use crate::Hash as _; @@ -199,7 +199,7 @@ mod tests { #[test] #[cfg(feature = "alloc")] fn manually_created_sha256t_hash_type() { - assert_eq!(TestHash::hash(&[0]).to_string(), HASH_ZERO_BACKWARD); + assert_eq!(TestHash::hash(&[0]).to_string(), HASH_ZERO_FORWARD); } // We also provide a macro to create the tag and the hash type. From 7685461e62beab0bcec8a96b732e58345b2640d9 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 24 Apr 2024 09:14:02 +1000 Subject: [PATCH 5/5] Document the sha256t_hash_newtype direction Since the default display direction is now forward, use `#[hash_newtype(backward)]` in the rustdocs on the macro. Also add an example usage to the changelog in case someone downstream is relying on the old default behaviour of displaying backwards (unlikely). --- hashes/CHANGELOG.md | 17 +++++++++++++++++ hashes/src/sha256t.rs | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/hashes/CHANGELOG.md b/hashes/CHANGELOG.md index ca96b020a..887776a59 100644 --- a/hashes/CHANGELOG.md +++ b/hashes/CHANGELOG.md @@ -1,3 +1,20 @@ +# unreleased + +* Change the default display direction of for tagged hashes to forwards. [#2707](https://github.com/rust-bitcoin/rust-bitcoin/pull/2707) + + Note please this usage if you need to display backward: + +```rust + sha256t_hash_newtype! { + /// Test detailed explanation. + struct NewTypeTag = hash_str("tag"); + + /// A test hash. + #[hash_newtype(backward)] + struct NewTypeHash(_); + } +``` + # 0.14.0 - 2024-03-21 * Bump MSRV to Rust version 1.56.1 [#2188](https://github.com/rust-bitcoin/rust-bitcoin/pull/2188) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 6627048cc..13b19c645 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -91,8 +91,8 @@ fn from_engine(e: sha256::HashEngine) -> Hash { /// pub struct FooTag = hash_str("foo"); /// /// /// A foo hash. -/// // Direction works just like in case of hash_newtype! macro. -/// #[hash_newtype(forward)] +/// // Direction works just like the hash_newtype! macro. +/// #[hash_newtype(backward)] /// pub struct FooHash(_); /// } /// ```