From a01370052715b6733f07011f28944105493bda63 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Tue, 4 Mar 2025 11:02:18 +0100 Subject: [PATCH] Replace uses of `chunks_exact` with `as_chunks` In the past we've been using `chunks_exact` because const generics were unstable but then, when they were stabilized we didn't use `as_chunks` (or `array_chunks`) since they were unstable. But the instability was only because Rust devs don't know how to handle `0` being passed in. The function is perfectly implementable on stable. (With a tiny, easy-to-understand `unsafe` block.) `core` doesn't want to make a decision for all other crates yet but we can make it for our own crates because we know that we simply never pass zero. (And even if we did, we could just change the decision.) It also turns out there's a hack to simulate `const {}` block in our MSRV, so we can make compilation fail early. This commit adds an extension trait to internals to provide the methods, so we no longer have to use `chunks_exact`. It also cleans up the code quite nicely. --- Cargo-minimal.lock | 1 + Cargo-recent.lock | 1 + bitcoin/src/taproot/merkle_branch.rs | 17 ++-- hashes/Cargo.toml | 2 +- hashes/src/ripemd160/crypto.rs | 5 +- hashes/src/ripemd160/mod.rs | 6 +- hashes/src/sha1/crypto.rs | 5 +- hashes/src/sha1/mod.rs | 6 +- hashes/src/sha256/crypto.rs | 5 +- hashes/src/sha256/mod.rs | 9 ++- hashes/src/sha512/crypto.rs | 5 +- hashes/src/sha512/mod.rs | 6 +- internals/src/lib.rs | 1 + internals/src/slice.rs | 117 +++++++++++++++++++++++++++ 14 files changed, 158 insertions(+), 28 deletions(-) create mode 100644 internals/src/slice.rs diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 83c663c9c..f1b5097fa 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -146,6 +146,7 @@ dependencies = [ name = "bitcoin_hashes" version = "0.16.0" dependencies = [ + "bitcoin-internals", "hex-conservative 0.3.0", "serde", "serde_json", diff --git a/Cargo-recent.lock b/Cargo-recent.lock index bb8ec5132..558ade0a8 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -145,6 +145,7 @@ dependencies = [ name = "bitcoin_hashes" version = "0.16.0" dependencies = [ + "bitcoin-internals", "hex-conservative 0.3.0", "serde", "serde_json", diff --git a/bitcoin/src/taproot/merkle_branch.rs b/bitcoin/src/taproot/merkle_branch.rs index 10b2d0718..33f0330fc 100644 --- a/bitcoin/src/taproot/merkle_branch.rs +++ b/bitcoin/src/taproot/merkle_branch.rs @@ -45,18 +45,17 @@ impl TaprootMerkleBranch { /// The function returns an error if the number of bytes is not an integer multiple of 32 or /// if the number of hashes exceeds 128. pub fn decode(sl: &[u8]) -> Result { - if sl.len() % TAPROOT_CONTROL_NODE_SIZE != 0 { + use internals::slice::SliceExt; + let (node_hashes, remainder) = sl.bitcoin_as_chunks::(); + if !remainder.is_empty() { Err(InvalidMerkleBranchSizeError(sl.len()).into()) - } else if sl.len() > TAPROOT_CONTROL_NODE_SIZE * TAPROOT_CONTROL_MAX_NODE_COUNT { + } else if node_hashes.len() > TAPROOT_CONTROL_MAX_NODE_COUNT { Err(InvalidMerkleTreeDepthError(sl.len() / TAPROOT_CONTROL_NODE_SIZE).into()) } else { - let inner = sl - .chunks_exact(TAPROOT_CONTROL_NODE_SIZE) - .map(|chunk| { - let bytes = <[u8; 32]>::try_from(chunk) - .expect("chunks_exact always returns the correct size"); - TapNodeHash::from_byte_array(bytes) - }) + let inner = node_hashes + .iter() + .copied() + .map(TapNodeHash::from_byte_array) .collect(); Ok(TaprootMerkleBranch(inner)) diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 25e213790..3e020d4b9 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -22,7 +22,7 @@ serde = ["dep:serde", "hex"] small-hash = [] [dependencies] - +internals = { package = "bitcoin-internals", version = "0.4.0" } hex = { package = "hex-conservative", version = "0.3.0", default-features = false, optional = true } serde = { version = "1.0", default-features = false, optional = true } diff --git a/hashes/src/ripemd160/crypto.rs b/hashes/src/ripemd160/crypto.rs index 4c6226892..178a2f441 100644 --- a/hashes/src/ripemd160/crypto.rs +++ b/hashes/src/ripemd160/crypto.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: CC0-1.0 +use internals::slice::SliceExt; use super::{HashEngine, BLOCK_SIZE}; #[cfg(feature = "small-hash")] @@ -132,8 +133,8 @@ impl HashEngine { debug_assert_eq!(self.buffer.len(), BLOCK_SIZE); let mut w = [0u32; 16]; - for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.chunks_exact(4)) { - *w_val = u32::from_le_bytes(buff_bytes.try_into().expect("4 byte slice")) + for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.bitcoin_as_chunks().0) { + *w_val = u32::from_le_bytes(*buff_bytes) } process_block!(self.h, w, diff --git a/hashes/src/ripemd160/mod.rs b/hashes/src/ripemd160/mod.rs index e6be17537..c69b8ec39 100644 --- a/hashes/src/ripemd160/mod.rs +++ b/hashes/src/ripemd160/mod.rs @@ -2,6 +2,8 @@ //! RIPEMD160 implementation. +use internals::slice::SliceExt; + #[cfg(bench)] mod benches; mod crypto; @@ -68,8 +70,8 @@ impl HashEngine { #[cfg(not(hashes_fuzz))] fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; - for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { - ret_bytes.copy_from_slice(&(*val).to_le_bytes()); + for (val, ret_bytes) in self.h.iter().zip(ret.bitcoin_as_chunks_mut().0) { + *ret_bytes = val.to_le_bytes(); } ret } diff --git a/hashes/src/sha1/crypto.rs b/hashes/src/sha1/crypto.rs index 6f8a3fd8a..9076c9109 100644 --- a/hashes/src/sha1/crypto.rs +++ b/hashes/src/sha1/crypto.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: CC0-1.0 +use internals::slice::SliceExt; use super::{HashEngine, BLOCK_SIZE}; impl HashEngine { @@ -8,8 +9,8 @@ impl HashEngine { debug_assert_eq!(self.buffer.len(), BLOCK_SIZE); let mut w = [0u32; 80]; - for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.chunks_exact(4)) { - *w_val = u32::from_be_bytes(buff_bytes.try_into().expect("4 bytes slice")) + for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.bitcoin_as_chunks().0) { + *w_val = u32::from_be_bytes(*buff_bytes) } for i in 16..80 { w[i] = (w[i - 3] ^ w[i - 8] ^ w[i - 14] ^ w[i - 16]).rotate_left(1); diff --git a/hashes/src/sha1/mod.rs b/hashes/src/sha1/mod.rs index dc32d31dd..420cc4602 100644 --- a/hashes/src/sha1/mod.rs +++ b/hashes/src/sha1/mod.rs @@ -2,6 +2,8 @@ //! SHA1 implementation. +use internals::slice::SliceExt; + #[cfg(bench)] mod benches; mod crypto; @@ -60,8 +62,8 @@ impl HashEngine { #[cfg(not(hashes_fuzz))] pub(crate) fn midstate(&self) -> [u8; 20] { let mut ret = [0; 20]; - for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { - ret_bytes.copy_from_slice(&val.to_be_bytes()) + for (val, ret_bytes) in self.h.iter().zip(ret.bitcoin_as_chunks_mut().0) { + *ret_bytes = val.to_be_bytes(); } ret } diff --git a/hashes/src/sha256/crypto.rs b/hashes/src/sha256/crypto.rs index 210eb6e6e..34e780d7b 100644 --- a/hashes/src/sha256/crypto.rs +++ b/hashes/src/sha256/crypto.rs @@ -5,6 +5,7 @@ use core::arch::x86::*; #[cfg(all(feature = "std", target_arch = "x86_64"))] use core::arch::x86_64::*; +use internals::slice::SliceExt; use super::{HashEngine, Midstate, BLOCK_SIZE}; #[allow(non_snake_case)] @@ -526,8 +527,8 @@ impl HashEngine { debug_assert_eq!(self.buffer.len(), BLOCK_SIZE); let mut w = [0u32; 16]; - for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.chunks_exact(4)) { - *w_val = u32::from_be_bytes(buff_bytes.try_into().expect("4 byte slice")); + for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.bitcoin_as_chunks().0) { + *w_val = u32::from_be_bytes(*buff_bytes); } let mut a = self.h[0]; diff --git a/hashes/src/sha256/mod.rs b/hashes/src/sha256/mod.rs index 6ae83b879..39624ae23 100644 --- a/hashes/src/sha256/mod.rs +++ b/hashes/src/sha256/mod.rs @@ -9,6 +9,7 @@ mod crypto; mod tests; use core::{cmp, convert, fmt}; +use internals::slice::SliceExt; use crate::{incomplete_block_len, sha256d, HashEngine as _}; #[cfg(doc)] @@ -79,8 +80,8 @@ impl HashEngine { /// Please see docs on [`Midstate`] before using this function. pub fn from_midstate(midstate: Midstate) -> HashEngine { let mut ret = [0; 8]; - for (ret_val, midstate_bytes) in ret.iter_mut().zip(midstate.as_ref().chunks_exact(4)) { - *ret_val = u32::from_be_bytes(midstate_bytes.try_into().expect("4 byte slice")); + for (ret_val, midstate_bytes) in ret.iter_mut().zip(midstate.as_ref().bitcoin_as_chunks().0) { + *ret_val = u32::from_be_bytes(*midstate_bytes); } HashEngine { buffer: [0; BLOCK_SIZE], h: ret, bytes_hashed: midstate.bytes_hashed } @@ -108,8 +109,8 @@ impl HashEngine { #[cfg(not(hashes_fuzz))] fn midstate_unchecked(&self) -> Midstate { let mut ret = [0; 32]; - for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(4)) { - ret_bytes.copy_from_slice(&val.to_be_bytes()); + for (val, ret_bytes) in self.h.iter().zip(ret.bitcoin_as_chunks_mut::<4>().0) { + *ret_bytes = val.to_be_bytes(); } Midstate { bytes: ret, bytes_hashed: self.bytes_hashed } } diff --git a/hashes/src/sha512/crypto.rs b/hashes/src/sha512/crypto.rs index cbb1c82ab..0a9d5b0da 100644 --- a/hashes/src/sha512/crypto.rs +++ b/hashes/src/sha512/crypto.rs @@ -1,5 +1,6 @@ // SPDX-License-Identifier: CC0-1.0 +use internals::slice::SliceExt; use super::{HashEngine, BLOCK_SIZE}; #[allow(non_snake_case)] @@ -75,8 +76,8 @@ impl HashEngine { debug_assert_eq!(self.buffer.len(), BLOCK_SIZE); let mut w = [0u64; 16]; - for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.chunks_exact(8)) { - *w_val = u64::from_be_bytes(buff_bytes.try_into().expect("8 byte slice")); + for (w_val, buff_bytes) in w.iter_mut().zip(self.buffer.bitcoin_as_chunks().0) { + *w_val = u64::from_be_bytes(*buff_bytes); } let mut a = self.h[0]; diff --git a/hashes/src/sha512/mod.rs b/hashes/src/sha512/mod.rs index 65bf75c18..2ee451f46 100644 --- a/hashes/src/sha512/mod.rs +++ b/hashes/src/sha512/mod.rs @@ -2,6 +2,8 @@ //! SHA512 implementation. +use internals::slice::SliceExt; + #[cfg(bench)] mod benches; mod crypto; @@ -79,8 +81,8 @@ impl HashEngine { #[cfg(not(hashes_fuzz))] pub(crate) fn midstate(&self) -> [u8; 64] { let mut ret = [0; 64]; - for (val, ret_bytes) in self.h.iter().zip(ret.chunks_exact_mut(8)) { - ret_bytes.copy_from_slice(&val.to_be_bytes()); + for (val, ret_bytes) in self.h.iter().zip(ret.bitcoin_as_chunks_mut().0) { + *ret_bytes = val.to_be_bytes(); } ret } diff --git a/internals/src/lib.rs b/internals/src/lib.rs index 34fa682ae..bed67bd2f 100644 --- a/internals/src/lib.rs +++ b/internals/src/lib.rs @@ -42,6 +42,7 @@ pub mod error; pub mod macros; mod parse; pub mod script; +pub mod slice; pub mod wrap_debug; #[cfg(feature = "serde")] #[macro_use] diff --git a/internals/src/slice.rs b/internals/src/slice.rs new file mode 100644 index 000000000..38ac95878 --- /dev/null +++ b/internals/src/slice.rs @@ -0,0 +1,117 @@ +//! Contains extensions related to slices. + +/// Extension trait for slice. +pub trait SliceExt { + /// The item type the slice is storing. + type Item; + + /// Splits up the slice into a slice of arrays and a remainder. + /// + /// Note that `N` must not be zero: + /// + /// ```compile_fail + /// let slice = [1, 2, 3]; + /// let fail = slice.as_chunks::<0>(); + /// ``` + fn bitcoin_as_chunks(&self) -> (&[[Self::Item; N]], &[Self::Item]); + + /// Splits up the slice into a slice of arrays and a remainder. + /// + /// Note that `N` must not be zero: + /// + /// ```compile_fail + /// let mut slice = [1, 2, 3]; + /// let fail = slice.as_chunks_mut::<0>(); + /// ``` + fn bitcoin_as_chunks_mut(&mut self) -> (&mut [[Self::Item; N]], &mut [Self::Item]); +} + +impl SliceExt for [T] { + type Item = T; + + fn bitcoin_as_chunks(&self) -> (&[[Self::Item; N]], &[Self::Item]) { + #[allow(clippy::let_unit_value)] + let _ = Hack::::IS_NONZERO; + + let chunks_count = self.len() / N; + let total_left_len = chunks_count * N; + let (left, right) = self.split_at(total_left_len); + // SAFETY: we've obtained the pointer from a slice that's still live + // we're merely casting, so no aliasing issues here + // arrays of T have same alignment as T + // the resulting slice points within the obtained slice as was computed above + let left = unsafe { core::slice::from_raw_parts(left.as_ptr().cast::<[Self::Item; N]>(), chunks_count) }; + (left, right) + } + + fn bitcoin_as_chunks_mut(&mut self) -> (&mut [[Self::Item; N]], &mut [Self::Item]) { + #[allow(clippy::let_unit_value)] + let _ = Hack::::IS_NONZERO; + + let chunks_count = self.len() / N; + let total_left_len = chunks_count * N; + let (left, right) = self.split_at_mut(total_left_len); + // SAFETY: we've obtained the pointer from a slice that's still live + // we're merely casting, so no aliasing issues here + // arrays of T have same alignment as T + // the resulting slice points within the obtained slice as was computed above + let left = unsafe { core::slice::from_raw_parts_mut(left.as_mut_ptr().cast::<[Self::Item; N]>(), chunks_count) }; + (left, right) + } +} + +struct Hack; + +impl Hack { + const IS_NONZERO: () = { assert!(N != 0); }; +} + +#[cfg(test)] +mod tests { + use super::SliceExt; + + // some comparisons require type annotations + const EMPTY: &[i32] = &[]; + + #[test] + fn one_to_one() { + let slice = [1]; + let (left, right) = slice.bitcoin_as_chunks::<1>(); + assert_eq!(left, &[[1]]); + assert_eq!(right, EMPTY); + } + + #[test] + fn one_to_two() { + const EMPTY_LEFT: &[[i32; 2]] = &[]; + + let slice = [1i32]; + let (left, right) = slice.bitcoin_as_chunks::<2>(); + assert_eq!(left, EMPTY_LEFT); + assert_eq!(right, &[1]); + } + + #[test] + fn two_to_one() { + let slice = [1, 2]; + let (left, right) = slice.bitcoin_as_chunks::<1>(); + assert_eq!(left, &[[1], [2]]); + assert_eq!(right, EMPTY); + } + + #[test] + fn two_to_two() { + let slice = [1, 2]; + let (left, right) = slice.bitcoin_as_chunks::<2>(); + assert_eq!(left, &[[1, 2]]); + assert_eq!(right, EMPTY); + } + + #[test] + fn three_to_two() { + let slice = [1, 2, 3]; + let (left, right) = slice.bitcoin_as_chunks::<2>(); + assert_eq!(left, &[[1, 2]]); + assert_eq!(right, &[3]); + } +}