Merge rust-bitcoin/rust-bitcoin#4047: Properly deprecate `Hash::from_slice`

f61e93ccf1 Properly deprecate Hash::from_slice (Tobin C. Harding)
50c0af7138 Stop using Hash::from_slice (Tobin C. Harding)

Pull request description:

  The `hashes::error::FromSliceError` error is only returned from `from_slice`. We attempted to deprecate this function but it seems we only did half a job at it.

  - deprecate _all_ instances of the method/function
  - deprecate the error type
  - stop using the deprecated functions in `bitcoin`

  Close: #4053

ACKs for top commit:
  apoelstra:
    ACK f61e93ccf1db7e7e3c9604fdb09b4e25195d88b2; successfully ran local tests

Tree-SHA512: 61a0e5127019859776ffac66bd4d320c86b8462bb1e908127d0bf42896aaa8df85fd2b06850342b694ca1cd68ed50355c81cad6ae3e9a5fd6e3933efe85498ad

bitcoin/src/blockdata/constants.rs

@@ -373,7 +373,7 @@ mod test {
         // The genesis block hash is a double-sha256 and it is displayed backwards.
         let genesis_hash = genesis_block(network).block_hash();
         // We abuse the sha256 hash here so we get a LowerHex impl that does not print the hex backwards.
-        let hash = sha256::Hash::from_slice(genesis_hash.as_byte_array()).unwrap();
+        let hash = sha256::Hash::from_byte_array(genesis_hash.to_byte_array());
         let want = format!("{:02x}", hash);
 
         let chain_hash = ChainHash::using_genesis_block_const(network);

bitcoin/src/p2p/message.rs

@@ -716,7 +716,7 @@ mod test {
     use crate::script::ScriptBuf;
     use crate::transaction::Transaction;
 
-    fn hash(slice: [u8; 32]) -> sha256d::Hash { sha256d::Hash::from_slice(&slice).unwrap() }
+    fn hash(array: [u8; 32]) -> sha256d::Hash { sha256d::Hash::from_byte_array(array) }
 
     #[test]
     fn full_round_ser_der_raw_network_message() {

hashes/src/hash160/mod.rs

@@ -118,7 +118,7 @@ mod tests {
             0xf1, 0x4a, 0xca, 0xd7,
         ];
 
-        let hash = hash160::Hash::from_slice(&HASH_BYTES).expect("right number of bytes");
+        let hash = hash160::Hash::from_byte_array(HASH_BYTES);
         assert_tokens(&hash.compact(), &[Token::BorrowedBytes(&HASH_BYTES[..])]);
         assert_tokens(&hash.readable(), &[Token::Str("132072df690933835eb8b6ad0b77e7b6f14acad7")]);
     }

hashes/src/hmac/mod.rs

@@ -12,7 +12,7 @@ use core::{convert, fmt, str};
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
-use crate::{FromSliceError, GeneralHash, Hash, HashEngine};
+use crate::{GeneralHash, Hash, HashEngine};
 
 /// A hash computed from a RFC 2104 HMAC. Parameterized by the underlying hash function.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -132,8 +132,10 @@ impl<T: GeneralHash> Hash for Hmac<T> {
 
     fn from_byte_array(bytes: T::Bytes) -> Self { Hmac(T::from_byte_array(bytes)) }
 
-    #[allow(deprecated)]
+    #[allow(deprecated_in_future)] // Because of `FromSliceError`.
-    fn from_slice(sl: &[u8]) -> Result<Hmac<T>, FromSliceError> { T::from_slice(sl).map(Hmac) }
+    fn from_slice(sl: &[u8]) -> Result<Hmac<T>, crate::FromSliceError> {
+        T::from_slice(sl).map(Hmac)
+    }
 
     fn to_byte_array(self) -> Self::Bytes { self.0.to_byte_array() }
 

hashes/src/internal_macros.rs

@@ -42,7 +42,8 @@ macro_rules! hash_trait_impls {
 
             fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) }
 
-            #[allow(deprecated)]
+            #[allow(deprecated_in_future)] // Because of `FromSliceError`.
+            #[allow(deprecated)]           // Because of `from_slice`.
             fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<Hash<$($gen),*>, $crate::FromSliceError> {
                 Self::from_slice(sl)
             }
@@ -126,11 +127,13 @@ macro_rules! hash_type_no_default {
             }
 
             /// Copies a byte slice into a hash object.
+            #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")]
+            #[allow(deprecated_in_future)] // Because of `FromSliceError`.
             pub fn from_slice(
                 sl: &[u8],
             ) -> $crate::_export::_core::result::Result<Hash, $crate::FromSliceError> {
                 if sl.len() != $bits / 8 {
-                    Err($crate::FromSliceError($crate::error::FromSliceErrorInner {
+                    Err($crate::error::FromSliceError($crate::error::FromSliceErrorInner {
                         expected: $bits / 8,
                         got: sl.len(),
                     }))

hashes/src/lib.rs

@@ -96,10 +96,11 @@ pub mod _export {
     }
 }
 
+#[deprecated(since = "TBD", note = "unused now that `Hash::from_slice` is deprecated")]
+mod error;
 mod internal_macros;
 
 pub mod cmp;
-pub mod error;
 pub mod hash160;
 pub mod hkdf;
 pub mod hmac;
@@ -132,11 +133,9 @@ use core::{convert, hash};
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
 pub use self::{
-    error::FromSliceError,
     hkdf::Hkdf,
     hmac::{Hmac, HmacEngine},
 };
-
 /// HASH-160: Alias for the [`hash160::Hash`] hash type.
 #[doc(inline)]
 pub use hash160::Hash as Hash160;
@@ -165,6 +164,11 @@ pub use sha512_256::Hash as Sha512_256;
 #[doc(inline)]
 pub use siphash24::Hash as Siphash24;
 
+/// Attempted to create a hash from an invalid length slice.
+#[deprecated(since = "TBD", note = "unused now that `Hash::from_slice` is deprecated")]
+#[allow(deprecated_in_future)]
+pub type FromSliceError = crate::error::FromSliceError; // Alias instead of re-export so we can deprecate it.
+
 /// Tagged SHA-256: Type alias for the [`sha256t::Hash`] hash type.
 pub type Sha256t<T> = sha256t::Hash<T>;
 
@@ -257,7 +261,8 @@ pub trait Hash:
     fn from_byte_array(bytes: Self::Bytes) -> Self;
 
     /// Copies a byte slice into a hash object.
-    #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")]
+    #[allow(deprecated_in_future)] // Because of `FromSliceError`.
+    #[deprecated(since = "TBD", note = "use `from_byte_array` instead")]
     fn from_slice(sl: &[u8]) -> Result<Self, FromSliceError>;
 
     /// Returns the underlying byte array.

hashes/src/macros.rs

@@ -141,7 +141,8 @@ macro_rules! hash_newtype {
 
             /// Copies a byte slice into a hash object.
             #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")]
-            #[allow(deprecated)]
+            #[allow(deprecated_in_future)] // Because of `FromSliceError`.
+            #[allow(deprecated)]           // Because of `from_slice`.
             pub fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<$newtype, $crate::FromSliceError> {
                 Ok($newtype(<$hash as $crate::Hash>::from_slice(sl)?))
             }
@@ -178,7 +179,8 @@ macro_rules! hash_newtype {
             fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) }
 
             #[inline]
-            #[allow(deprecated)]
+            #[allow(deprecated_in_future)] // Because of `FromSliceError`.
+            #[allow(deprecated)]           // Because of `from_slice`.
             fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<$newtype, $crate::FromSliceError> {
                 Self::from_slice(sl)
             }

hashes/src/sha256d/mod.rs

@@ -122,7 +122,7 @@ mod tests {
             0xb7, 0x65, 0x44, 0x8c, 0x86, 0x35, 0xfb, 0x6c,
         ];
 
-        let hash = sha256d::Hash::from_slice(&HASH_BYTES).expect("right number of bytes");
+        let hash = sha256d::Hash::from_byte_array(HASH_BYTES);
         assert_tokens(&hash.compact(), &[Token::BorrowedBytes(&HASH_BYTES[..])]);
         assert_tokens(
             &hash.readable(),

hashes/src/sha256t/mod.rs

@@ -7,7 +7,7 @@ use core::marker::PhantomData;
 
 #[cfg(doc)]
 use crate::sha256::Midstate;
-use crate::{sha256, FromSliceError, HashEngine as _};
+use crate::{sha256, HashEngine as _};
 
 /// Trait representing a tag that can be used as a context for SHA256t hashes.
 pub trait Tag: Clone {
@@ -44,11 +44,12 @@ where
 
     /// Copies a byte slice into a hash object.
     #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")]
-    pub fn from_slice(sl: &[u8]) -> Result<Hash<T>, FromSliceError> {
+    #[allow(deprecated_in_future)] // Because of `FromSliceError`.
+    pub fn from_slice(sl: &[u8]) -> Result<Hash<T>, crate::FromSliceError> {
         use crate::error::FromSliceErrorInner;
 
         if sl.len() != 32 {
-            Err(FromSliceError(FromSliceErrorInner { expected: 32, got: sl.len() }))
+            Err(crate::error::FromSliceError(FromSliceErrorInner { expected: 32, got: sl.len() }))
         } else {
             let mut ret = [0; 32];
             ret.copy_from_slice(sl);

hashes/src/siphash24/mod.rs

@@ -321,7 +321,7 @@ mod tests {
 
         for i in 0..64 {
             vin[i] = i as u8;
-            let vec = Hash::from_slice(&vecs[i]).unwrap();
+            let vec = Hash::from_byte_array(vecs[i]);
             let out = Hash::hash_with_keys(k0, k1, &vin[0..i]);
             assert_eq!(vec, out, "vec #{}", i);