Merge rust-bitcoin/rust-bitcoin#4182: Replace uses of `chunks_exact` with `as_chunks`

a013700527 Replace uses of `chunks_exact` with `as_chunks` (Martin Habovstiak)

Pull request description:

  This is now ready for review.

  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.

  Previous unresolved question, leaving for reference:

  > One issue with this change is that the names collide which could lead to hard error in future Rust versions. How do we solve it?
  > * ignore and just backport the fix once that actually happens
  > * rename the methods to something reasonable (e.g. `as_array_chunks`) - this risks that they'll rename the methods to the same thing by accident and it'll break anyway
  > * rename the methods to something silly (`bitcoin_as_chunks`) - yeah, the risk above is not there but then we have silly-looking code.

  We've decide to just rename the methods to something that won't possibly collide.

ACKs for top commit:
  tcharding:
    ACK a013700527
  apoelstra:
    ACK a01370052715b6733f07011f28944105493bda63; successfully ran local tests; nice!

Tree-SHA512: cc3359518f97e510da5ee9a33495e26c338bfc3e4162aaffcc72ed9c7daad0daf5e9ca3d23bce50877b0d3881792e98e28d21174a4426bb01281f12285ce08d1

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",

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",

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<Self, TaprootError> {
-        if sl.len() % TAPROOT_CONTROL_NODE_SIZE != 0 {
+        use internals::slice::SliceExt;
+        let (node_hashes, remainder) = sl.bitcoin_as_chunks::<TAPROOT_CONTROL_NODE_SIZE>();
+        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))

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 }
 

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,

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
     }

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);

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
     }

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];

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 }
     }

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];

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
     }

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]

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<const N: usize>(&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<const N: usize>(&mut self) -> (&mut [[Self::Item; N]], &mut [Self::Item]);
+}
+
+impl<T> SliceExt for [T] {
+    type Item = T;
+
+    fn bitcoin_as_chunks<const N: usize>(&self) -> (&[[Self::Item; N]], &[Self::Item]) {
+        #[allow(clippy::let_unit_value)]
+        let _ = Hack::<N>::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<const N: usize>(&mut self) -> (&mut [[Self::Item; N]], &mut [Self::Item]) {
+        #[allow(clippy::let_unit_value)]
+        let _ = Hack::<N>::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<const N: usize>;
+
+impl<const N: usize> Hack<N> {
+    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]);
+    }
+}