From 71013afe07adb2adc700266618145ea02c24627c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 10:59:17 +1100 Subject: [PATCH 1/7] hashes: Put attribute under doc As is typical round here, put the attribute under the associated rustdoc. --- hashes/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index c42f38cd2..c8ac1e2d3 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -74,8 +74,8 @@ extern crate core; #[cfg(feature = "bitcoin-io")] extern crate bitcoin_io as io; -#[cfg(feature = "serde")] /// A generic serialization/deserialization framework. +#[cfg(feature = "serde")] pub extern crate serde; #[cfg(all(test, feature = "serde"))] From bb7dd2c479a629c8ebcea1eec0e34ea03cc2f485 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 11:05:18 +1100 Subject: [PATCH 2/7] hashes: Move DISPLAY_BACKWARD to top of impl block There is no obvious reason why this const is further down the block, move it. Refactor only, no logic change. --- hashes/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index c8ac1e2d3..8759693cd 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -265,15 +265,15 @@ pub trait Hash: /// Length of the hash, in bytes. const LEN: usize = Self::Bytes::LEN; + /// Flag indicating whether user-visible serializations of this hash should be backward. + /// + /// For some reason Satoshi decided this should be true for `Sha256dHash`, so here we are. + const DISPLAY_BACKWARD: bool = false; + /// Copies a byte slice into a hash object. #[deprecated(since = "TBD", note = "use `from_byte_array` instead")] fn from_slice(sl: &[u8]) -> Result; - /// Flag indicating whether user-visible serializations of this hash - /// should be backward. For some reason Satoshi decided this should be - /// true for `Sha256dHash`, so here we are. - const DISPLAY_BACKWARD: bool = false; - /// Returns the underlying byte array. fn to_byte_array(self) -> Self::Bytes; From 62617cf9acf880379cfeecb756eb438756d78a23 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 12:00:41 +1100 Subject: [PATCH 3/7] hashes: Move from_engine function to other macro The `from_engine` function is associated with a general hash type but we are defining it in the `hash_type` macro which holds nothing but functions associated with the `Hash` trait. By "associated" I mean methods on the type as opposed to trait methods. In preparation for re-naming the `hash_type` macro move the `from_engine` function there. Requires duplicating the code in the `siphash` impl block, this is as expected because the `siphash` requires a custom implementation of the general hashing functionality. Internal change only. --- hashes/src/internal_macros.rs | 6 +++--- hashes/src/siphash24.rs | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 320b29d7d..4e5f61bd1 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -130,6 +130,9 @@ macro_rules! hash_type { $crate::internal_macros::hash_type_no_default!($bits, $reverse, $doc); impl Hash { + /// Produces a hash from the current state of a given engine. + pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } + /// Constructs a new engine. pub fn engine() -> HashEngine { Default::default() } @@ -180,9 +183,6 @@ macro_rules! hash_type_no_default { unsafe { &mut *(bytes as *mut _ as *mut Self) } } - /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } - /// Copies a byte slice into a hash object. pub fn from_slice( sl: &[u8], diff --git a/hashes/src/siphash24.rs b/hashes/src/siphash24.rs index 1d17885ff..a40f8185c 100644 --- a/hashes/src/siphash24.rs +++ b/hashes/src/siphash24.rs @@ -168,6 +168,9 @@ impl crate::HashEngine for HashEngine { } impl Hash { + /// Produces a hash from the current state of a given engine. + pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } + /// Hashes the given data with an engine with the provided keys. pub fn hash_with_keys(k0: u64, k1: u64, data: &[u8]) -> Hash { let mut engine = HashEngine::with_keys(k0, k1); From c11587d60d09d3dc3cb2e2fb82a5c3f560d644b2 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 12:05:30 +1100 Subject: [PATCH 4/7] hashes: Rename hash_type macro Conceptually (and using traits) we split the hashes into "general" hash types and more restricted hash types (`Hash`). Also we observe that the `hash_type` macro defines all the inherent functions name identically to the `GeneralHash` trait methods. Rename the trait to describe better what it does. Internal change only. --- hashes/src/hash160.rs | 2 +- hashes/src/internal_macros.rs | 6 +++--- hashes/src/ripemd160.rs | 2 +- hashes/src/sha1.rs | 2 +- hashes/src/sha256.rs | 2 +- hashes/src/sha256d.rs | 2 +- hashes/src/sha384.rs | 2 +- hashes/src/sha512.rs | 2 +- hashes/src/sha512_256.rs | 2 +- 9 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hashes/src/hash160.rs b/hashes/src/hash160.rs index c9b2bc51c..a0a8f715c 100644 --- a/hashes/src/hash160.rs +++ b/hashes/src/hash160.rs @@ -9,7 +9,7 @@ use crate::{ripemd160, sha256}; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 160, false, "Output of the Bitcoin HASH160 hash function. (RIPEMD160(SHA256))" diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 4e5f61bd1..f8272cd09 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -112,7 +112,7 @@ macro_rules! hash_trait_impls { } pub(crate) use hash_trait_impls; -/// Creates a type called `Hash` and implements standard interface for it. +/// Creates a type called `Hash` and implements the standard general hashing interface for it. /// /// The created type has a single field and will have all standard derives as well as an /// implementation of [`crate::Hash`]. @@ -125,7 +125,7 @@ pub(crate) use hash_trait_impls; /// /// The `from_engine` free-standing function is still required with this macro. See the doc of /// [`hash_trait_impls`]. -macro_rules! hash_type { +macro_rules! general_hash_type { ($bits:expr, $reverse:expr, $doc:literal) => { $crate::internal_macros::hash_type_no_default!($bits, $reverse, $doc); @@ -157,7 +157,7 @@ macro_rules! hash_type { } }; } -pub(crate) use hash_type; +pub(crate) use general_hash_type; macro_rules! hash_type_no_default { ($bits:expr, $reverse:expr, $doc:literal) => { diff --git a/hashes/src/ripemd160.rs b/hashes/src/ripemd160.rs index a423c2282..d77a2d928 100644 --- a/hashes/src/ripemd160.rs +++ b/hashes/src/ripemd160.rs @@ -6,7 +6,7 @@ use core::cmp; use crate::{incomplete_block_len, HashEngine as _}; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 160, false, "Output of the RIPEMD160 hash function." diff --git a/hashes/src/sha1.rs b/hashes/src/sha1.rs index 8d2f28aab..b028dd75a 100644 --- a/hashes/src/sha1.rs +++ b/hashes/src/sha1.rs @@ -6,7 +6,7 @@ use core::cmp; use crate::{incomplete_block_len, HashEngine as _}; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 160, false, "Output of the SHA1 hash function." diff --git a/hashes/src/sha256.rs b/hashes/src/sha256.rs index 6be32f92a..53bffcf2b 100644 --- a/hashes/src/sha256.rs +++ b/hashes/src/sha256.rs @@ -14,7 +14,7 @@ use crate::{incomplete_block_len, sha256d, HashEngine as _}; #[cfg(doc)] use crate::{sha256t, sha256t_tag}; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 256, false, "Output of the SHA256 hash function." diff --git a/hashes/src/sha256d.rs b/hashes/src/sha256d.rs index b9437184d..44e6580f3 100644 --- a/hashes/src/sha256d.rs +++ b/hashes/src/sha256d.rs @@ -4,7 +4,7 @@ use crate::sha256; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 256, true, "Output of the SHA256d hash function." diff --git a/hashes/src/sha384.rs b/hashes/src/sha384.rs index 4b197bac9..e6e1aec3a 100644 --- a/hashes/src/sha384.rs +++ b/hashes/src/sha384.rs @@ -4,7 +4,7 @@ use crate::sha512; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 384, false, "Output of the SHA384 hash function." diff --git a/hashes/src/sha512.rs b/hashes/src/sha512.rs index 42f1da922..0b3168133 100644 --- a/hashes/src/sha512.rs +++ b/hashes/src/sha512.rs @@ -6,7 +6,7 @@ use core::cmp; use crate::{incomplete_block_len, HashEngine as _}; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 512, false, "Output of the SHA512 hash function." diff --git a/hashes/src/sha512_256.rs b/hashes/src/sha512_256.rs index 6698d0319..53d08e3de 100644 --- a/hashes/src/sha512_256.rs +++ b/hashes/src/sha512_256.rs @@ -9,7 +9,7 @@ use crate::sha512; -crate::internal_macros::hash_type! { +crate::internal_macros::general_hash_type! { 256, false, "Output of the SHA512/256 hash function.\n\nSHA512/256 is a hash function that uses the sha512 algorithm but it truncates the output to 256 bits. It has different initial constants than sha512 so it produces an entirely different hash compared to sha512. More information at ." From 12f261c00958c918b1c991332ede64153ded0bb6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 12:13:27 +1100 Subject: [PATCH 5/7] hashes: Re-order from_byte_array The `hashes` crate has a bunch of similar types defined by a bunch of similar macros and impl blocks, all of which makes it difficult to tell exactly what is implemented where. In an effort to make the code easier to read order the `from_byte_array` constructor in the same place across the crate. Note also we typically put constructors up the top, also `from_byte_array` is the likely most used constructor so put it first. FWIW I discovered this while polishing the HTML docs. Internal change only. --- hashes/src/hmac.rs | 4 ++-- hashes/src/internal_macros.rs | 14 +++++++------- hashes/src/lib.rs | 6 +++--- hashes/src/sha256t.rs | 6 +++--- hashes/src/util.rs | 14 +++++++------- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index ecd5bc673..2e30277f7 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -127,14 +127,14 @@ impl GeneralHash for Hmac { impl Hash for Hmac { type Bytes = T::Bytes; + fn from_byte_array(bytes: T::Bytes) -> Self { Hmac(T::from_byte_array(bytes)) } + #[allow(deprecated_in_future)] fn from_slice(sl: &[u8]) -> Result, FromSliceError> { T::from_slice(sl).map(Hmac) } fn to_byte_array(self) -> Self::Bytes { self.0.to_byte_array() } fn as_byte_array(&self) -> &Self::Bytes { self.0.as_byte_array() } - - fn from_byte_array(bytes: T::Bytes) -> Self { Hmac(T::from_byte_array(bytes)) } } #[cfg(feature = "serde")] diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index f8272cd09..d8a73ad18 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -97,6 +97,8 @@ macro_rules! hash_trait_impls { const DISPLAY_BACKWARD: bool = $reverse; + fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } + #[allow(deprecated_in_future)] fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result, $crate::FromSliceError> { Self::from_slice(sl) @@ -105,8 +107,6 @@ macro_rules! hash_trait_impls { fn to_byte_array(self) -> Self::Bytes { self.to_byte_array() } fn as_byte_array(&self) -> &Self::Bytes { self.as_byte_array() } - - fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } } } } @@ -169,6 +169,11 @@ macro_rules! hash_type_no_default { impl Hash { const fn internal_new(arr: [u8; $bits / 8]) -> Self { Hash(arr) } + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: [u8; $bits / 8]) -> Self { + Self::internal_new(bytes) + } + /// Zero cost conversion between a fixed length byte array shared reference and /// a shared reference to this Hash type. pub fn from_bytes_ref(bytes: &[u8; $bits / 8]) -> &Self { @@ -201,11 +206,6 @@ macro_rules! hash_type_no_default { /// Returns a reference to the underlying byte array. pub const fn as_byte_array(&self) -> &[u8; $bits / 8] { &self.0 } - - /// Constructs a hash from the underlying byte array. - pub const fn from_byte_array(bytes: [u8; $bits / 8]) -> Self { - Self::internal_new(bytes) - } } $crate::internal_macros::hash_trait_impls!($bits, $reverse); diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 8759693cd..255239fe9 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -270,6 +270,9 @@ pub trait Hash: /// For some reason Satoshi decided this should be true for `Sha256dHash`, so here we are. const DISPLAY_BACKWARD: bool = false; + /// Constructs a hash from the underlying byte array. + fn from_byte_array(bytes: Self::Bytes) -> Self; + /// Copies a byte slice into a hash object. #[deprecated(since = "TBD", note = "use `from_byte_array` instead")] fn from_slice(sl: &[u8]) -> Result; @@ -279,9 +282,6 @@ pub trait Hash: /// Returns a reference to the underlying byte array. fn as_byte_array(&self) -> &Self::Bytes; - - /// Constructs a hash from the underlying byte array. - fn from_byte_array(bytes: Self::Bytes) -> Self; } /// Ensures that a type is an array. diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 4df7c4bd9..20faa9e06 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -25,6 +25,9 @@ where { const fn internal_new(arr: [u8; 32]) -> Self { Hash(arr, PhantomData) } + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(bytes) } + /// Zero cost conversion between a fixed length byte array shared reference and /// a shared reference to this Hash type. pub fn from_bytes_ref(bytes: &[u8; 32]) -> &Self { @@ -104,9 +107,6 @@ where /// Returns a reference to the underlying byte array. pub const fn as_byte_array(&self) -> &[u8; 32] { &self.0 } - - /// Constructs a hash from the underlying byte array. - pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(bytes) } } impl Copy for Hash {} diff --git a/hashes/src/util.rs b/hashes/src/util.rs index 80ca72af7..d0da798f3 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -195,6 +195,11 @@ macro_rules! hash_newtype { #[allow(unused)] // Private wrapper types may not need all functions. impl $newtype { + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: <$hash as $crate::Hash>::Bytes) -> Self { + $newtype(<$hash>::from_byte_array(bytes)) + } + /// Copies a byte slice into a hash object. #[deprecated(since = "TBD", note = "use `from_byte_array` instead")] #[allow(deprecated_in_future)] @@ -211,11 +216,6 @@ macro_rules! hash_newtype { pub const fn as_byte_array(&self) -> &<$hash as $crate::Hash>::Bytes { self.0.as_byte_array() } - - /// Constructs a hash from the underlying byte array. - pub const fn from_byte_array(bytes: <$hash as $crate::Hash>::Bytes) -> Self { - $newtype(<$hash>::from_byte_array(bytes)) - } } impl $crate::_export::_core::convert::From<$hash> for $newtype { @@ -236,6 +236,8 @@ macro_rules! hash_newtype { const DISPLAY_BACKWARD: bool = $crate::hash_newtype_get_direction!($hash, $(#[$($type_attrs)*])*); + fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } + #[inline] #[allow(deprecated_in_future)] fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<$newtype, $crate::FromSliceError> { @@ -245,8 +247,6 @@ macro_rules! hash_newtype { fn to_byte_array(self) -> Self::Bytes { self.to_byte_array() } fn as_byte_array(&self) -> &Self::Bytes { self.as_byte_array() } - - fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } } impl $crate::_export::_core::str::FromStr for $newtype { From 98691186dcb816c46b86c3f8d6b97f8d31a45a17 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 12:19:12 +1100 Subject: [PATCH 6/7] hashes: Move engine functions The `sha256t` module is unique in that it implements its methods manually (as call throughs) instead of using the macros. To make it more clear what is implemented re-order the engine constructor and getter to better mirror the layout in the macros. Internal change only. --- hashes/src/sha256t.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 20faa9e06..3619efe92 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -42,12 +42,6 @@ where unsafe { &mut *(bytes as *mut _ as *mut Self) } } - /// Constructs a new engine. - pub fn engine() -> HashEngine { T::engine() } - - /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } - /// Copies a byte slice into a hash object. #[deprecated(since = "TBD", note = "Use `from_byte_array` instead.")] pub fn from_slice(sl: &[u8]) -> Result, FromSliceError> { @@ -60,6 +54,12 @@ where } } + /// Produces a hash from the current state of a given engine. + pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } + + /// Constructs a new engine. + pub fn engine() -> HashEngine { T::engine() } + /// Hashes some bytes. #[allow(clippy::self_named_constructors)] // Hash is a noun and a verb. pub fn hash(data: &[u8]) -> Self { From 1cb24c1f150bc2d65d0be439f2005f41d95ad23c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 16 Oct 2024 12:25:06 +1100 Subject: [PATCH 7/7] hashes:: Polish crate level rustdocs `hashes v1.0.0` is close, make an effort to polish the crate level rustdocs. --- hashes/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 255239fe9..44aa1e0e0 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -2,10 +2,8 @@ //! Rust hashes library. //! -//! This is a simple, no-dependency library which implements the hash functions -//! needed by Bitcoin. These are SHA256, SHA256d, and RIPEMD160. As an ancillary -//! thing, it exposes hexadecimal serialization and deserialization, since these -//! are needed to display hashes anway. +//! This library implements the hash functions needed by Bitcoin. As an ancillary thing, it exposes +//! hexadecimal serialization and deserialization, since these are needed to display hashes. //! //! ## Commonly used operations //! @@ -23,9 +21,10 @@ //! Hashing content from a reader: //! //! ``` -//! #[cfg(feature = "std")] { +//! # #[cfg(feature = "std")] { //! use bitcoin_hashes::Sha256; -//! let mut reader: &[u8] = b"hello"; // in real code, this could be a `File` or `TcpStream` +//! +//! let mut reader: &[u8] = b"hello"; // In real code, this could be a `File` or `TcpStream`. //! let mut engine = Sha256::engine(); //! std::io::copy(&mut reader, &mut engine).unwrap(); //! let _hash = Sha256::from_engine(engine); @@ -33,12 +32,13 @@ //! ``` //! //! -//! Hashing content by [`std::io::Write`] on `HashEngine`: +//! Hashing content using [`std::io::Write`] on a `HashEngine`: //! //! ``` -//! #[cfg(feature = "std")] { -//! use std::io::Write as _; // Or `bitcoin-io::Write` if `bitcoin-io` feature is enabled. +//! # #[cfg(feature = "std")] { +//! use std::io::Write as _; //! use bitcoin_hashes::Sha256; +//! //! let part1: &[u8] = b"hello"; //! let part2: &[u8] = b" "; //! let part3: &[u8] = b"world";