From 766f498b33873dc6d66dd5d013cd400050161501 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 13 Nov 2024 12:21:28 +1100 Subject: [PATCH 1/3] Pull serde stuff out of impl_bytelike_traits macro The `impl_bytelike_traits` macro is public and it is used in the `hash_newtype` macro, also public. Currently if a user calls the `hash_newtype` macro in a crate that depends on `hashes` without the `serde` feature enabled and with no `serde` dependency everything works. However if the user then adds a dependency that happens to enable the `serde` feature in `hashes` their build will blow up because `serde` code will start getting called from the original crate's call to `hash_newtype`. Pull the serde stuff out of `hash_newtype` and provide a macro to implement it `impl_serde_for_newtype`. --- bitcoin/src/bip158.rs | 3 +++ bitcoin/src/bip32.rs | 3 +++ bitcoin/src/blockdata/script/mod.rs | 4 ++++ bitcoin/src/crypto/key.rs | 4 ++++ bitcoin/src/crypto/sighash.rs | 6 ++++++ hashes/src/internal_macros.rs | 3 +++ hashes/src/macros.rs | 18 +++++++++++++----- primitives/src/block.rs | 3 +++ primitives/src/merkle_tree.rs | 3 +++ primitives/src/taproot.rs | 9 +++++++++ primitives/src/transaction.rs | 3 +++ 11 files changed, 54 insertions(+), 5 deletions(-) diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs index 8c5499ad5..d0a81aa83 100644 --- a/bitcoin/src/bip158.rs +++ b/bitcoin/src/bip158.rs @@ -62,6 +62,9 @@ hashes::hash_newtype! { pub struct FilterHeader(sha256d::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(FilterHash, FilterHeader); + impl_hashencode!(FilterHash); impl_hashencode!(FilterHeader); diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs index 6f609b59a..a3477c765 100644 --- a/bitcoin/src/bip32.rs +++ b/bitcoin/src/bip32.rs @@ -58,6 +58,9 @@ hash_newtype! { pub struct XKeyIdentifier(hash160::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(XKeyIdentifier); + /// Extended private key #[derive(Copy, Clone, PartialEq, Eq)] #[cfg_attr(feature = "std", derive(Debug))] diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index 774e95c3b..be3bf676a 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -92,6 +92,10 @@ hashes::hash_newtype! { /// SegWit version of a Bitcoin Script bytecode hash. pub struct WScriptHash(sha256::Hash); } + +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(ScriptHash, WScriptHash); + impl_asref_push_bytes!(ScriptHash, WScriptHash); impl ScriptHash { diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index a0627ba0d..753d81ac5 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -264,6 +264,10 @@ hashes::hash_newtype! { /// SegWit version of a public key hash. pub struct WPubkeyHash(hash160::Hash); } + +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(PubkeyHash, WPubkeyHash); + impl_asref_push_bytes!(PubkeyHash, WPubkeyHash); impl From for PubkeyHash { diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index e6530921a..f5a59e897 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -56,6 +56,9 @@ hash_newtype! { pub struct SegwitV0Sighash(sha256d::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(LegacySighash, SegwitV0Sighash); + impl_message_from_hash!(LegacySighash); impl_message_from_hash!(SegwitV0Sighash); @@ -82,6 +85,9 @@ hash_newtype! { pub struct TapSighash(sha256t::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(TapSighash); + impl_message_from_hash!(TapSighash); /// Efficiently calculates signature hash message for legacy, segwit and Taproot inputs. diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index a707d2222..9702444a3 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -28,6 +28,9 @@ macro_rules! hash_trait_impls { fn from_engine(e: HashEngine) -> Hash<$($gen),*> { Self::from_engine(e) } } + #[cfg(feature = "serde")] + $crate::serde_impl!(Hash, { $bits / 8} $(, $gen: $gent)*); + impl<$($gen: $gent),*> $crate::Hash for Hash<$($gen),*> { type Bytes = [u8; $bits / 8]; diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs index 5520ffe90..3501a564c 100644 --- a/hashes/src/macros.rs +++ b/hashes/src/macros.rs @@ -4,6 +4,7 @@ //! //! - [`sha256t_tag`](crate::sha256t_tag) //! - [`hash_newtype`](crate::hash_newtype) +//! - [`impl_serde_for_newtype`](crate::impl_serde_for_newtype) /// Macro used to define a tag. /// @@ -71,8 +72,7 @@ macro_rules! sha256t_tag { /// /// You can add arbitrary doc comments or other attributes to the struct or it's field. Note that /// the macro already derives [`Copy`], [`Clone`], [`Eq`], [`PartialEq`], -/// [`Hash`](core::hash::Hash), [`Ord`], [`PartialOrd`]. With the `serde` feature on, this also adds -/// `Serialize` and `Deserialize` implementations. +/// [`Hash`](core::hash::Hash), [`Ord`], [`PartialOrd`]. /// /// You can also define multiple newtypes within one macro call: /// @@ -201,7 +201,6 @@ macro_rules! hash_newtype { /// * `str::FromStr` /// * `fmt::{LowerHex, UpperHex}` using `hex-conservative`. /// * `fmt::{Display, Debug}` by calling `LowerHex` -/// * `serde::{Deserialize, Serialize}` /// * `AsRef[u8; $len]` /// * `AsRef[u8]` /// * `Borrow<[u8; $len]>` @@ -247,8 +246,6 @@ macro_rules! impl_bytelike_traits { } } - $crate::serde_impl!($ty, $len $(, $gen: $gent)*); - impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8; { $len }]> for $ty<$($gen),*> { #[inline] fn as_ref(&self) -> &[u8; { $len }] { self.as_byte_array() } @@ -433,6 +430,17 @@ pub mod serde_details { } } +/// Implements `Serialize` and `Deserialize` for a new type created with [`crate::hash_newtype`] macro. +#[macro_export] +#[cfg(feature = "serde")] +macro_rules! impl_serde_for_newtype { + ($($newtype:ident),*) => { + $( + $crate::serde_impl!($newtype, { <$newtype as $crate::Hash>::LEN }); + )* + } +} + /// Implements `Serialize` and `Deserialize` for a type `$t` which /// represents a newtype over a byte-slice over length `$len`. #[doc(hidden)] diff --git a/primitives/src/block.rs b/primitives/src/block.rs index 20475c942..4fdd9d605 100644 --- a/primitives/src/block.rs +++ b/primitives/src/block.rs @@ -162,6 +162,9 @@ hashes::hash_newtype! { pub struct WitnessCommitment(sha256d::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(BlockHash, WitnessCommitment); + impl BlockHash { /// Dummy hash used as the previous blockhash of the genesis block. pub const GENESIS_PREVIOUS_BLOCK_HASH: Self = Self::from_byte_array([0; 32]); diff --git a/primitives/src/merkle_tree.rs b/primitives/src/merkle_tree.rs index 763ace1fa..5f3c5de71 100644 --- a/primitives/src/merkle_tree.rs +++ b/primitives/src/merkle_tree.rs @@ -10,3 +10,6 @@ hashes::hash_newtype! { /// A hash corresponding to the Merkle tree root for witness data. pub struct WitnessMerkleNode(sha256d::Hash); } + +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(TxMerkleNode, WitnessMerkleNode); diff --git a/primitives/src/taproot.rs b/primitives/src/taproot.rs index 452b10af5..b85ac5617 100644 --- a/primitives/src/taproot.rs +++ b/primitives/src/taproot.rs @@ -18,6 +18,9 @@ hash_newtype! { pub struct TapLeafHash(sha256t::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(TapLeafHash); + sha256t_tag! { pub struct TapBranchTag = hash_str("TapBranch"); } @@ -29,6 +32,9 @@ hash_newtype! { pub struct TapNodeHash(sha256t::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(TapNodeHash); + sha256t_tag! { pub struct TapTweakTag = hash_str("TapTweak"); } @@ -40,6 +46,9 @@ hash_newtype! { pub struct TapTweakHash(sha256t::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(TapTweakHash); + impl From for TapNodeHash { fn from(leaf: TapLeafHash) -> TapNodeHash { TapNodeHash::from_byte_array(leaf.to_byte_array()) } } diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs index 5b6fe1086..dbfc3c896 100644 --- a/primitives/src/transaction.rs +++ b/primitives/src/transaction.rs @@ -476,6 +476,9 @@ hashes::hash_newtype! { pub struct Wtxid(sha256d::Hash); } +#[cfg(feature = "serde")] +hashes::impl_serde_for_newtype!(Txid, Wtxid); + impl Txid { /// The `Txid` used in a coinbase prevout. /// From 9dce0b4b8c6c905fa09585c0814e09b1bdf599b0 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 13 Nov 2024 12:55:40 +1100 Subject: [PATCH 2/3] Remove hex string trait bounds from GeneralHash For the `hashes` crate we would like to make `hex` an optional dependency. In preparation for doing so do the following: - Remove the trait bounds from `GeneralHash` - Split the hex/string stuff out of `impl_bytelike_traits` into a separate macro. --- bitcoin/src/bip158.rs | 1 + bitcoin/src/bip32.rs | 1 + bitcoin/src/blockdata/script/mod.rs | 1 + bitcoin/src/crypto/key.rs | 1 + bitcoin/src/crypto/sighash.rs | 2 + hashes/src/hmac.rs | 6 +-- hashes/src/internal_macros.rs | 3 +- hashes/src/lib.rs | 18 ++----- hashes/src/macros.rs | 82 ++++++++++++++++++++--------- hashes/src/sha256t.rs | 2 + primitives/src/block.rs | 1 + primitives/src/merkle_tree.rs | 1 + primitives/src/taproot.rs | 3 ++ primitives/src/transaction.rs | 1 + 14 files changed, 82 insertions(+), 41 deletions(-) diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs index d0a81aa83..55a290730 100644 --- a/bitcoin/src/bip158.rs +++ b/bitcoin/src/bip158.rs @@ -62,6 +62,7 @@ hashes::hash_newtype! { pub struct FilterHeader(sha256d::Hash); } +hashes::impl_hex_for_newtype!(FilterHash, FilterHeader); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(FilterHash, FilterHeader); diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs index a3477c765..8f11e67b8 100644 --- a/bitcoin/src/bip32.rs +++ b/bitcoin/src/bip32.rs @@ -58,6 +58,7 @@ hash_newtype! { pub struct XKeyIdentifier(hash160::Hash); } +hashes::impl_hex_for_newtype!(XKeyIdentifier); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(XKeyIdentifier); diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index be3bf676a..55947e43a 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -93,6 +93,7 @@ hashes::hash_newtype! { pub struct WScriptHash(sha256::Hash); } +hashes::impl_hex_for_newtype!(ScriptHash, WScriptHash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(ScriptHash, WScriptHash); diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 753d81ac5..9772f39d4 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -265,6 +265,7 @@ hashes::hash_newtype! { pub struct WPubkeyHash(hash160::Hash); } +hashes::impl_hex_for_newtype!(PubkeyHash, WPubkeyHash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(PubkeyHash, WPubkeyHash); diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index f5a59e897..d5651de35 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -56,6 +56,7 @@ hash_newtype! { pub struct SegwitV0Sighash(sha256d::Hash); } +hashes::impl_hex_for_newtype!(LegacySighash, SegwitV0Sighash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(LegacySighash, SegwitV0Sighash); @@ -85,6 +86,7 @@ hash_newtype! { pub struct TapSighash(sha256t::Hash); } +hashes::impl_hex_for_newtype!(TapSighash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(TapSighash); diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index a14a54455..283bdc7d0 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -96,15 +96,15 @@ impl HashEngine for HmacEngine { fn input(&mut self, buf: &[u8]) { self.iengine.input(buf) } } -impl fmt::Debug for Hmac { +impl fmt::Debug for Hmac { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Debug::fmt(&self.0, f) } } -impl fmt::Display for Hmac { +impl fmt::Display for Hmac { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } } -impl fmt::LowerHex for Hmac { +impl fmt::LowerHex for Hmac { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0, f) } } diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 9702444a3..f74f49d49 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -20,7 +20,8 @@ /// `from_engine` obviously implements the finalization algorithm. macro_rules! hash_trait_impls { ($bits:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { - $crate::impl_bytelike_traits!(Hash, { $bits / 8 }, $reverse $(, $gen: $gent)*); + $crate::impl_bytelike_traits!(Hash, { $bits / 8 } $(, $gen: $gent)*); + $crate::impl_hex_string_traits!(Hash, { $bits / 8 }, $reverse $(, $gen: $gent)*); impl<$($gen: $gent),*> $crate::GeneralHash for Hash<$($gen),*> { type Engine = HashEngine; diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 11acde41d..972c7bf57 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -126,7 +126,7 @@ pub mod serde_macros { } } -use core::{convert, fmt, hash}; +use core::{convert, hash}; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] @@ -261,20 +261,10 @@ pub trait GeneralHash: Hash { /// Trait which applies to hashes of all types. pub trait Hash: - Copy - + Clone - + PartialEq - + Eq - + PartialOrd - + Ord - + hash::Hash - + fmt::Debug - + fmt::Display - + fmt::LowerHex - + convert::AsRef<[u8]> + Copy + Clone + PartialEq + Eq + PartialOrd + Ord + hash::Hash + convert::AsRef<[u8]> { /// The byte array that represents the hash internally. - type Bytes: hex::FromHex + Copy + IsByteArray; + type Bytes: Copy + IsByteArray; /// Length of the hash, in bytes. const LEN: usize = Self::Bytes::LEN; @@ -335,6 +325,8 @@ mod tests { struct TestNewtype2(sha256d::Hash); } + crate::impl_hex_for_newtype!(TestNewtype, TestNewtype2); + #[test] #[cfg(feature = "alloc")] fn newtype_fmt_roundtrip() { diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs index 3501a564c..b8207f6f2 100644 --- a/hashes/src/macros.rs +++ b/hashes/src/macros.rs @@ -4,6 +4,7 @@ //! //! - [`sha256t_tag`](crate::sha256t_tag) //! - [`hash_newtype`](crate::hash_newtype) +//! - [`impl_hex_for_newtype`](crate::impl_hex_for_newtype) //! - [`impl_serde_for_newtype`](crate::impl_serde_for_newtype) /// Macro used to define a tag. @@ -133,7 +134,7 @@ macro_rules! hash_newtype { $({ $($type_attrs)* })* } - $crate::impl_bytelike_traits!($newtype, { <$newtype as $crate::Hash>::LEN }, <$newtype as $crate::Hash>::DISPLAY_BACKWARD); + $crate::impl_bytelike_traits!($newtype, { <$newtype as $crate::Hash>::LEN }); #[allow(unused)] // Private wrapper types may not need all functions. impl $newtype { @@ -194,17 +195,67 @@ macro_rules! hash_newtype { }; } +/// Implements string functions using hex for a new type crated with [`crate::hash_newtype`] macro. +/// +/// Implements: +/// +/// * `str::FromStr` +/// * `fmt::{LowerHex, UpperHex}` using `hex-conservative` +/// * `fmt::{Display, Debug}` by calling `LowerHex` +#[macro_export] +macro_rules! impl_hex_for_newtype { + ($($newtype:ident),*) => { + $( + $crate::impl_hex_string_traits!($newtype, { <$newtype as $crate::Hash>::LEN }, { <$newtype as $crate::Hash>::DISPLAY_BACKWARD }); + )* + } +} + /// Adds trait impls to a bytelike type. /// /// Implements: /// +/// * `AsRef[u8; $len]` +/// * `AsRef[u8]` +/// * `Borrow<[u8; $len]>` +/// * `Borrow<[u8]>` +/// +/// ## Parameters +/// +/// * `ty` - The bytelike type to implement the traits on. +/// * `$len` - The number of bytes this type has. +/// * `$gen: $gent` - generic type(s) and trait bound(s). +#[doc(hidden)] +#[macro_export] +macro_rules! impl_bytelike_traits { + ($ty:ident, $len:expr $(, $gen:ident: $gent:ident)*) => { + impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8; { $len }]> for $ty<$($gen),*> { + #[inline] + fn as_ref(&self) -> &[u8; { $len }] { self.as_byte_array() } + } + + impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8]> for $ty<$($gen),*> { + #[inline] + fn as_ref(&self) -> &[u8] { self.as_byte_array() } + } + + impl<$($gen: $gent),*> $crate::_export::_core::borrow::Borrow<[u8; { $len }]> for $ty<$($gen),*> { + fn borrow(&self) -> &[u8; { $len }] { self.as_byte_array() } + } + + impl<$($gen: $gent),*> $crate::_export::_core::borrow::Borrow<[u8]> for $ty<$($gen),*> { + fn borrow(&self) -> &[u8] { self.as_byte_array() } + } + } +} + +/// Adds hex string trait impls to a bytelike type using hex. +/// +/// Implements: +/// /// * `str::FromStr` /// * `fmt::{LowerHex, UpperHex}` using `hex-conservative`. /// * `fmt::{Display, Debug}` by calling `LowerHex` -/// * `AsRef[u8; $len]` -/// * `AsRef[u8]` -/// * `Borrow<[u8; $len]>` -/// * `Borrow<[u8]>` /// /// Requires: /// @@ -223,7 +274,7 @@ macro_rules! hash_newtype { /// [`hex-conservative`]: #[doc(hidden)] #[macro_export] -macro_rules! impl_bytelike_traits { +macro_rules! impl_hex_string_traits { ($ty:ident, $len:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { impl<$($gen: $gent),*> $crate::_export::_core::str::FromStr for $ty<$($gen),*> { type Err = $crate::hex::HexToArrayError; @@ -245,24 +296,6 @@ macro_rules! impl_bytelike_traits { const LENGTH: usize = ($len); // parens required due to rustc parser weirdness } } - - impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8; { $len }]> for $ty<$($gen),*> { - #[inline] - fn as_ref(&self) -> &[u8; { $len }] { self.as_byte_array() } - } - - impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8]> for $ty<$($gen),*> { - #[inline] - fn as_ref(&self) -> &[u8] { self.as_byte_array() } - } - - impl<$($gen: $gent),*> $crate::_export::_core::borrow::Borrow<[u8; { $len }]> for $ty<$($gen),*> { - fn borrow(&self) -> &[u8; { $len }] { self.as_byte_array() } - } - - impl<$($gen: $gent),*> $crate::_export::_core::borrow::Borrow<[u8]> for $ty<$($gen),*> { - fn borrow(&self) -> &[u8] { self.as_byte_array() } - } } } @@ -509,6 +542,7 @@ mod test { /// Test hash. struct TestHash(crate::sha256d::Hash); } + crate::impl_hex_for_newtype!(TestHash); impl TestHash { fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 1b4188d2c..262a4ec3f 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -321,6 +321,7 @@ mod tests { #[hash_newtype(backward)] struct NewTypeHashBackward(sha256t::Hash); } + crate::impl_hex_for_newtype!(NewTypeHashBackward); #[test] #[cfg(feature = "alloc")] @@ -344,6 +345,7 @@ mod tests { #[hash_newtype(forward)] struct NewTypeHashForward(sha256t::Hash); } + crate::impl_hex_for_newtype!(NewTypeHashForward); #[test] #[cfg(feature = "alloc")] diff --git a/primitives/src/block.rs b/primitives/src/block.rs index 4fdd9d605..9edea1470 100644 --- a/primitives/src/block.rs +++ b/primitives/src/block.rs @@ -162,6 +162,7 @@ hashes::hash_newtype! { pub struct WitnessCommitment(sha256d::Hash); } +hashes::impl_hex_for_newtype!(BlockHash, WitnessCommitment); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(BlockHash, WitnessCommitment); diff --git a/primitives/src/merkle_tree.rs b/primitives/src/merkle_tree.rs index 5f3c5de71..18a16a5a9 100644 --- a/primitives/src/merkle_tree.rs +++ b/primitives/src/merkle_tree.rs @@ -11,5 +11,6 @@ hashes::hash_newtype! { pub struct WitnessMerkleNode(sha256d::Hash); } +hashes::impl_hex_for_newtype!(TxMerkleNode, WitnessMerkleNode); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(TxMerkleNode, WitnessMerkleNode); diff --git a/primitives/src/taproot.rs b/primitives/src/taproot.rs index b85ac5617..743980dcd 100644 --- a/primitives/src/taproot.rs +++ b/primitives/src/taproot.rs @@ -18,6 +18,7 @@ hash_newtype! { pub struct TapLeafHash(sha256t::Hash); } +hashes::impl_hex_for_newtype!(TapLeafHash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(TapLeafHash); @@ -32,6 +33,7 @@ hash_newtype! { pub struct TapNodeHash(sha256t::Hash); } +hashes::impl_hex_for_newtype!(TapNodeHash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(TapNodeHash); @@ -46,6 +48,7 @@ hash_newtype! { pub struct TapTweakHash(sha256t::Hash); } +hashes::impl_hex_for_newtype!(TapTweakHash); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(TapTweakHash); diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs index dbfc3c896..68b7d2393 100644 --- a/primitives/src/transaction.rs +++ b/primitives/src/transaction.rs @@ -476,6 +476,7 @@ hashes::hash_newtype! { pub struct Wtxid(sha256d::Hash); } +hashes::impl_hex_for_newtype!(Txid, Wtxid); #[cfg(feature = "serde")] hashes::impl_serde_for_newtype!(Txid, Wtxid); From ec06028f63ba591a14c3a15cdfd410bb5ff1c09b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 13 Nov 2024 13:29:06 +1100 Subject: [PATCH 3/3] hashes: Make hex dependency optional The only reason we need `hex-conservative` is to parse strings and format them as hex. For users that do not require this functionality we can make the `hex-conservative` crate an optional dependency. The `serde` feature requires `Display` so we enable `hex` from the `serde` feature. If `hex` feature is not enabled we still need to be able to debug so provide `fmt::Debug` functionality by way of macros. Close: #2654 --- .github/workflows/rust.yml | 4 ++- bitcoin/Cargo.toml | 2 +- hashes/Cargo.toml | 3 ++- hashes/embedded/Cargo.toml | 1 + hashes/embedded/src/main.rs | 12 ++++++++- hashes/src/internal_macros.rs | 3 +++ hashes/src/lib.rs | 21 ++++++++++++++++ hashes/src/macros.rs | 46 +++++++++++++++++++++++++++++++++++ hashes/src/sha256.rs | 11 ++++++--- hashes/src/sha256t.rs | 8 ++++++ hashes/tests/io.rs | 1 + hashes/tests/regression.rs | 2 ++ primitives/Cargo.toml | 2 +- 13 files changed, 108 insertions(+), 8 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 50b8a0d82..cf06c57ae 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -253,8 +253,10 @@ jobs: run: cd bitcoin/embedded && cargo run --target thumbv7m-none-eabi - name: "Run hashes/embedded no alloc" run: cd hashes/embedded && cargo run --target thumbv7m-none-eabi - - name: "Run hashes/embedded with alloc" + - name: "Run hashes/embedded with alloc and no hex" run: cd hashes/embedded && cargo run --target thumbv7m-none-eabi --features=alloc + - name: "Run hashes/embedded with alloc and hex" + run: cd hashes/embedded && cargo run --target thumbv7m-none-eabi --features=alloc,hex ASAN: # hashes crate only. name: ASAN - nightly toolchain diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index ff078247a..d96e8af50 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -27,7 +27,7 @@ arbitrary = ["dep:arbitrary", "units/arbitrary", "primitives/arbitrary"] [dependencies] base58 = { package = "base58ck", version = "0.1.0", default-features = false, features = ["alloc"] } bech32 = { version = "0.11.0", default-features = false, features = ["alloc"] } -hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false, features = ["alloc", "bitcoin-io"] } +hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false, features = ["alloc", "bitcoin-io", "hex"] } hex = { package = "hex-conservative", version = "0.3.0", default-features = false, features = ["alloc"] } internals = { package = "bitcoin-internals", version = "0.4.0", features = ["alloc"] } io = { package = "bitcoin-io", version = "0.2.0", default-features = false, features = ["alloc"] } diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 1553f7882..c05e31880 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -17,12 +17,13 @@ exclude = ["tests", "contrib"] default = ["std"] std = ["alloc", "bitcoin-io?/std", "hex/std"] alloc = ["bitcoin-io?/alloc", "hex/alloc"] +serde = ["dep:serde", "hex"] # Smaller (but slower) implementation of sha256, sha512 and ripemd160 small-hash = [] [dependencies] -hex = { package = "hex-conservative", version = "0.3.0", default-features = false } +hex = { package = "hex-conservative", version = "0.3.0", default-features = false, optional = true } bitcoin-io = { version = "0.2.0", default-features = false, optional = true } serde = { version = "1.0", default-features = false, optional = true } diff --git a/hashes/embedded/Cargo.toml b/hashes/embedded/Cargo.toml index 3cebf9643..be73fe563 100644 --- a/hashes/embedded/Cargo.toml +++ b/hashes/embedded/Cargo.toml @@ -11,6 +11,7 @@ members = ["."] [features] alloc = ["alloc-cortex-m", "bitcoin_hashes/alloc"] +hex = ["bitcoin_hashes/hex"] [dependencies] cortex-m = "0.6.0" diff --git a/hashes/embedded/src/main.rs b/hashes/embedded/src/main.rs index 027563d00..3a19faaff 100644 --- a/hashes/embedded/src/main.rs +++ b/hashes/embedded/src/main.rs @@ -12,13 +12,20 @@ extern crate bitcoin_hashes; use bitcoin_hashes::{sha256, HashEngine}; use bitcoin_io::Write; use cortex_m_rt::entry; -use cortex_m_semihosting::{debug, hprintln}; +use cortex_m_semihosting::debug; +#[cfg(feature = "hex")] +use cortex_m_semihosting::hprintln; use panic_halt as _; hash_newtype! { struct TestType(sha256::Hash); } +#[cfg(feature = "hex")] +bitcoin_hashes::impl_hex_for_newtype!(TestType); +#[cfg(not(feature = "hex"))] +bitcoin_hashes::impl_debug_only_for_newtype!(TestType); + // this is the allocator the application will use #[cfg(feature = "alloc")] #[global_allocator] @@ -34,16 +41,19 @@ fn main() -> ! { let mut engine = sha256::Hash::engine(); engine.write_all(b"abc").unwrap(); + #[cfg(feature = "hex")] check_result(engine); let mut engine = sha256::Hash::engine(); engine.input(b"abc"); + #[cfg(feature = "hex")] check_result(engine); debug::exit(debug::EXIT_SUCCESS); loop {} } +#[cfg(feature = "hex")] fn check_result(engine: sha256::HashEngine) { let hash = TestType(sha256::Hash::from_engine(engine)); diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index f74f49d49..39999b3dd 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -21,7 +21,10 @@ macro_rules! hash_trait_impls { ($bits:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { $crate::impl_bytelike_traits!(Hash, { $bits / 8 } $(, $gen: $gent)*); + #[cfg(feature = "hex")] $crate::impl_hex_string_traits!(Hash, { $bits / 8 }, $reverse $(, $gen: $gent)*); + #[cfg(not(feature = "hex"))] + $crate::impl_debug_only!(Hash, { $bits / 8 }, $reverse $(, $gen: $gent)*); impl<$($gen: $gent),*> $crate::GeneralHash for Hash<$($gen),*> { type Engine = HashEngine; diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 972c7bf57..bc6206d30 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -86,6 +86,7 @@ extern crate serde_test; extern crate test; /// Re-export the `hex-conservative` crate. +#[cfg(feature = "hex")] pub extern crate hex; #[doc(hidden)] @@ -126,6 +127,7 @@ pub mod serde_macros { } } +use core::fmt::{self, Write as _}; use core::{convert, hash}; #[rustfmt::skip] // Keep public re-exports separate. @@ -312,6 +314,22 @@ fn incomplete_block_len(eng: &H) -> usize { (eng.n_bytes_hashed() % block_size) as usize } +/// Writes `bytes` as a `hex` string to the formatter. +/// +/// For when we cannot rely on having the `hex` feature enabled. Ignores formatter options and just +/// writes with plain old `f.write_char()`. +pub fn debug_hex(bytes: &[u8], f: &mut fmt::Formatter) -> fmt::Result { + const HEX_TABLE: [u8; 16] = *b"0123456789abcdef"; + + for &b in bytes { + let lower = HEX_TABLE[usize::from(b >> 4)]; + let upper = HEX_TABLE[usize::from(b & 0b00001111)]; + f.write_char(char::from(lower))?; + f.write_char(char::from(upper))?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -325,7 +343,10 @@ mod tests { struct TestNewtype2(sha256d::Hash); } + #[cfg(feature = "hex")] crate::impl_hex_for_newtype!(TestNewtype, TestNewtype2); + #[cfg(not(feature = "hex"))] + crate::impl_debug_only_for_newtype!(TestNewtype, TestNewtype2); #[test] #[cfg(feature = "alloc")] diff --git a/hashes/src/macros.rs b/hashes/src/macros.rs index b8207f6f2..3d5382aec 100644 --- a/hashes/src/macros.rs +++ b/hashes/src/macros.rs @@ -203,6 +203,7 @@ macro_rules! hash_newtype { /// * `fmt::{LowerHex, UpperHex}` using `hex-conservative` /// * `fmt::{Display, Debug}` by calling `LowerHex` #[macro_export] +#[cfg(feature = "hex")] macro_rules! impl_hex_for_newtype { ($($newtype:ident),*) => { $( @@ -211,6 +212,18 @@ macro_rules! impl_hex_for_newtype { } } +/// Implements `fmt::Debug` using hex for a new type crated with [`crate::hash_newtype`] macro. +/// +/// This is provided in case you do not want to use the `hex` feature. +#[macro_export] +macro_rules! impl_debug_only_for_newtype { + ($($newtype:ident),*) => { + $( + $crate::impl_debug_only!($newtype, { <$newtype as $crate::Hash>::LEN }, { <$newtype as $crate::Hash>::DISPLAY_BACKWARD }); + )* + } +} + /// Adds trait impls to a bytelike type. /// /// Implements: @@ -274,6 +287,7 @@ macro_rules! impl_bytelike_traits { /// [`hex-conservative`]: #[doc(hidden)] #[macro_export] +#[cfg(feature = "hex")] macro_rules! impl_hex_string_traits { ($ty:ident, $len:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { impl<$($gen: $gent),*> $crate::_export::_core::str::FromStr for $ty<$($gen),*> { @@ -299,6 +313,20 @@ macro_rules! impl_hex_string_traits { } } +/// Implements `fmt::Debug` using hex. +#[doc(hidden)] +#[macro_export] +macro_rules! impl_debug_only { + ($ty:ident, $len:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { + impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + $crate::debug_hex(self.as_byte_array(), f) + } + } + } +} + // Generates the struct only (no impls) // // This is a separate macro to make it more readable and have a separate interface that allows for @@ -542,14 +570,29 @@ mod test { /// Test hash. struct TestHash(crate::sha256d::Hash); } + #[cfg(feature = "hex")] crate::impl_hex_for_newtype!(TestHash); + #[cfg(not(feature = "hex"))] + crate::impl_debug_only_for_newtype!(TestHash); impl TestHash { fn all_zeros() -> Self { Self::from_byte_array([0; 32]) } } + // NB: This runs with and without `hex` feature enabled, testing different code paths for each. #[test] #[cfg(feature = "alloc")] + fn debug() { + use alloc::format; + + let want = "0000000000000000000000000000000000000000000000000000000000000000"; + let got = format!("{:?}", TestHash::all_zeros()); + assert_eq!(got, want) + } + + #[test] + #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn display() { use alloc::format; @@ -560,6 +603,7 @@ mod test { #[test] #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn display_alternate() { use alloc::format; @@ -570,6 +614,7 @@ mod test { #[test] #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn lower_hex() { use alloc::format; @@ -580,6 +625,7 @@ mod test { #[test] #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn lower_hex_alternate() { use alloc::format; diff --git a/hashes/src/sha256.rs b/hashes/src/sha256.rs index a040338f4..98e8ec132 100644 --- a/hashes/src/sha256.rs +++ b/hashes/src/sha256.rs @@ -8,8 +8,6 @@ use core::arch::x86::*; use core::arch::x86_64::*; use core::{cmp, convert, fmt}; -use hex::DisplayHex; - use crate::{incomplete_block_len, sha256d, HashEngine as _}; #[cfg(doc)] use crate::{sha256t, sha256t_tag}; @@ -218,8 +216,15 @@ impl Midstate { impl fmt::Debug for Midstate { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + struct Encoder<'a> { + bytes: &'a [u8; 32], + } + impl fmt::Debug for Encoder<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { crate::debug_hex(self.bytes, f) } + } + f.debug_struct("Midstate") - .field("bytes", &self.bytes.as_hex()) + .field("bytes", &Encoder { bytes: &self.bytes }) .field("length", &self.bytes_hashed) .finish() } diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 262a4ec3f..cb3f4aaaa 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -321,10 +321,14 @@ mod tests { #[hash_newtype(backward)] struct NewTypeHashBackward(sha256t::Hash); } + #[cfg(feature = "hex")] crate::impl_hex_for_newtype!(NewTypeHashBackward); + #[cfg(not(feature = "hex"))] + crate::impl_debug_only_for_newtype!(NewTypeHashBackward); #[test] #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn macro_created_sha256t_hash_type_backward() { use alloc::string::ToString; @@ -345,10 +349,14 @@ mod tests { #[hash_newtype(forward)] struct NewTypeHashForward(sha256t::Hash); } + #[cfg(feature = "hex")] crate::impl_hex_for_newtype!(NewTypeHashForward); + #[cfg(not(feature = "hex"))] + crate::impl_debug_only_for_newtype!(NewTypeHashForward); #[test] #[cfg(feature = "alloc")] + #[cfg(feature = "hex")] fn macro_created_sha256t_hash_type_prints_forward() { use alloc::string::ToString; diff --git a/hashes/tests/io.rs b/hashes/tests/io.rs index ffaa80678..9151bba30 100644 --- a/hashes/tests/io.rs +++ b/hashes/tests/io.rs @@ -3,6 +3,7 @@ //! Test the `bitcoin-io` implementations. #![cfg(feature = "bitcoin-io")] +#![cfg(feature = "hex")] use bitcoin_hashes::{ hash160, hmac, ripemd160, sha1, sha256, sha256d, sha384, sha512, sha512_256, siphash24, diff --git a/hashes/tests/regression.rs b/hashes/tests/regression.rs index 360bfd7f3..d40998e36 100644 --- a/hashes/tests/regression.rs +++ b/hashes/tests/regression.rs @@ -2,6 +2,8 @@ //! //! Note that if `bitcoin-io` is enabled then we get more regression-like testing from `./io.rs`. +#![cfg(feature = "hex")] + use bitcoin_hashes::{ hash160, ripemd160, sha1, sha256, sha256d, sha256t, sha384, sha512, sha512_256, siphash24, GeneralHash as _, HashEngine as _, Hmac, HmacEngine, diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index 1387fc1d9..fb60db403 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -22,7 +22,7 @@ serde = ["dep:serde", "hashes/serde", "internals/serde", "units/serde", "alloc"] arbitrary = ["dep:arbitrary", "units/arbitrary"] [dependencies] -hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false, features = ["bitcoin-io"] } +hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false, features = ["bitcoin-io", "hex"] } hex = { package = "hex-conservative", version = "0.3.0", default-features = false } internals = { package = "bitcoin-internals", version = "0.4.0" } io = { package = "bitcoin-io", version = "0.2.0", default-features = false }