Skip to content

Commit

Permalink
Merge rust-bitcoin#3354: Enable attribute deprecated_in_future
Browse files Browse the repository at this point in the history
e68da28 Warn on future deprecations (Tobin C. Harding)
2cc9445 Fully deprecate Hash::from_slice (Tobin C. Harding)
8bc3b2e Stop using deprecated to_vec (Tobin C. Harding)

Pull request description:

  This is a bit curly because of current `serde` logic, fixed in patch 4. All the other patches are trivial.

ACKs for top commit:
  apoelstra:
    ACK e68da28 successfully ran local tests

Tree-SHA512: f78b3f91923e3278bdec09c321c57f2c25fc92a6e2a3daa8acd60dc1f454dfc5f092424fee113e4e03ac01d80d27a959f346068ecad7bc7894ebb15d98228a40
  • Loading branch information
apoelstra committed Oct 15, 2024
2 parents 299754c + e68da28 commit da462d6
Show file tree
Hide file tree
Showing 24 changed files with 72 additions and 77 deletions.
1 change: 1 addition & 0 deletions addresses/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![doc(test(attr(warn(unused))))]
// Coding conventions.
#![warn(deprecated_in_future)]
#![warn(missing_docs)]
// Exclude lints we don't think are valuable.
#![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
Expand Down
1 change: 1 addition & 0 deletions base58/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#![cfg_attr(bench, feature(test))]
// Coding conventions.
#![warn(missing_docs)]
#![warn(deprecated_in_future)]
#![doc(test(attr(warn(unused))))]
// Instead of littering the codebase for non-fuzzing and bench code just globally allow.
#![cfg_attr(fuzzing, allow(dead_code, unused_imports))]
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/address/script_pubkey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ define_extension_trait! {
fn p2wpkh_script_code(&self) -> Option<ScriptBuf> {
if self.is_p2wpkh() {
// The `self` script is 0x00, 0x14, <pubkey_hash>
let bytes = &self.as_bytes()[2..];
let wpkh = WPubkeyHash::from_slice(bytes).expect("length checked in is_p2wpkh()");
let bytes = <[u8; 20]>::try_from(&self.as_bytes()[2..]).expect("length checked in is_p2wpkh()");
let wpkh = WPubkeyHash::from_byte_array(bytes);
Some(script::p2wpkh_script_code(wpkh))
} else {
None
Expand Down
6 changes: 2 additions & 4 deletions bitcoin/src/blockdata/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,8 @@ impl Block {
.iter()
.rposition(|o| o.script_pubkey.len() >= 38 && o.script_pubkey.as_bytes()[0..6] == MAGIC)
{
let commitment = WitnessCommitment::from_slice(
&coinbase.output[pos].script_pubkey.as_bytes()[6..38],
)
.unwrap();
let bytes = <[u8; 32]>::try_from(&coinbase.output[pos].script_pubkey.as_bytes()[6..38]).unwrap();
let commitment = WitnessCommitment::from_byte_array(bytes);
// Witness reserved value is in coinbase input witness.
let witness_vec: Vec<_> = coinbase.input[0].witness.iter().collect();
if witness_vec.len() == 1 && witness_vec[0].len() == 32 {
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/blockdata/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1786,7 +1786,7 @@ mod tests {
.is_err());

// test that we get a failure if we corrupt a signature
let mut witness: Vec<_> = spending.input[1].witness.to_vec();
let mut witness = spending.input[1].witness.to_bytes();
witness[0][10] = 42;
spending.input[1].witness = Witness::from_slice(&witness);

Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/blockdata/witness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ mod test {
let expected_witness = vec![hex!(
"304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")
];
assert_eq!(witness.to_vec(), expected_witness);
assert_eq!(witness.to_bytes(), expected_witness);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/crypto/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,6 @@ mod tests {
let mut buf = vec![];
sig.serialize_to_writer(&mut buf).expect("write failed");

assert_eq!(sig.to_vec(), buf)
assert_eq!(sig.to_bytes(), buf)
}
}
5 changes: 3 additions & 2 deletions bitcoin/src/crypto/sighash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1527,7 +1527,7 @@ mod tests {
let cache = SighashCache::new(&tx);

let got = cache.legacy_signature_hash(1, &script, SIGHASH_SINGLE).expect("sighash");
let want = LegacySighash::from_slice(&UINT256_ONE).unwrap();
let want = LegacySighash::from_byte_array(UINT256_ONE);

assert_eq!(got, want)
}
Expand All @@ -1550,7 +1550,8 @@ mod tests {
let script = ScriptBuf::from(Vec::from_hex(script).unwrap());
let mut raw_expected = Vec::from_hex(expected_result).unwrap();
raw_expected.reverse();
let want = LegacySighash::from_slice(&raw_expected[..]).unwrap();
let bytes = <[u8; 32]>::try_from(&raw_expected[..]).unwrap();
let want = LegacySighash::from_byte_array(bytes);

let cache = SighashCache::new(&tx);
let got = cache.legacy_signature_hash(input_index, &script, hash_type as u32).unwrap();
Expand Down
1 change: 1 addition & 0 deletions bitcoin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#![cfg_attr(bench, feature(test))]
// Coding conventions.
#![warn(missing_docs)]
#![warn(deprecated_in_future)]
#![doc(test(attr(warn(unused))))]
// Instead of littering the codebase for non-fuzzing and bench code just globally allow.
#![cfg_attr(fuzzing, allow(dead_code, unused_imports))]
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/merkle_tree/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ mod tests {
let hashes = &mut self.hashes;
let mut hash = hashes[n].to_byte_array();
hash[(bit >> 3) as usize] ^= 1 << (bit & 7);
hashes[n] = TxMerkleNode::from_slice(&hash).unwrap();
hashes[n] = TxMerkleNode::from_byte_array(hash);
}
}

Expand Down
6 changes: 3 additions & 3 deletions bitcoin/src/psbt/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum Error {
/// Unable to parse as a standard sighash type.
NonStandardSighashType(u32),
/// Invalid hash when parsing slice.
InvalidHash(hashes::FromSliceError),
InvalidHash(core::array::TryFromSliceError),
/// The pre-image must hash to the corresponding psbt hash
InvalidPreimageHashPair {
/// Hash-type
Expand Down Expand Up @@ -203,8 +203,8 @@ impl std::error::Error for Error {
}
}

impl From<hashes::FromSliceError> for Error {
fn from(e: hashes::FromSliceError) -> Error { Error::InvalidHash(e) }
impl From<core::array::TryFromSliceError> for Error {
fn from(e: core::array::TryFromSliceError) -> Error { Error::InvalidHash(e) }
}

impl From<encode::Error> for Error {
Expand Down
4 changes: 3 additions & 1 deletion bitcoin/src/psbt/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ macro_rules! impl_psbt_hash_deserialize {
($hash_type:ty) => {
impl $crate::psbt::serialize::Deserialize for $hash_type {
fn deserialize(bytes: &[u8]) -> core::result::Result<Self, $crate::psbt::Error> {
<$hash_type>::from_slice(&bytes[..]).map_err(|e| $crate::psbt::Error::from(e))
const LEN: usize = <$hash_type as hashes::Hash>::LEN;
let bytes = <[u8; LEN]>::try_from(bytes).map_err(|e| $crate::psbt::Error::from(e))?;
Ok(<$hash_type>::from_byte_array(bytes))
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/psbt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1886,7 +1886,7 @@ mod tests {
#[cfg(not(feature = "std"))]
assert_eq!(
err.to_string(),
"invalid hash when parsing slice: invalid slice length 33 (expected 32)"
"invalid hash when parsing slice: could not convert slice to array"
);
let err = hex_psbt("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b69241142cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2cd970e15f53fc0c82f950fd560ffa919b76172be017368a89913af074f400b094289756aa3739ccc689ec0fcf3a360be32cc0b59b16e93a1e8bb4605726b2ca7a3ff706c4176649632b2cc68e1f912b8a578e3719ce7710885c7a966f49bcd43cb01010000").unwrap_err();
#[cfg(feature = "std")]
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/psbt/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Deserialize for secp256k1::PublicKey {
}

impl Serialize for ecdsa::Signature {
fn serialize(&self) -> Vec<u8> { self.to_vec() }
fn serialize(&self) -> Vec<u8> { self.to_bytes() }
}

impl Deserialize for ecdsa::Signature {
Expand Down Expand Up @@ -265,7 +265,7 @@ impl Deserialize for XOnlyPublicKey {
}

impl Serialize for taproot::Signature {
fn serialize(&self) -> Vec<u8> { self.to_vec() }
fn serialize(&self) -> Vec<u8> { self.to_bytes() }
}

impl Deserialize for taproot::Signature {
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/taproot/merkle_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ impl TaprootMerkleBranch {
let inner = sl
.chunks_exact(TAPROOT_CONTROL_NODE_SIZE)
.map(|chunk| {
TapNodeHash::from_slice(chunk)
.expect("chunks_exact always returns the correct size")
let bytes = <[u8; 32]>::try_from(chunk).expect("chunks_exact always returns the correct size");
TapNodeHash::from_byte_array(bytes)
})
.collect();

Expand Down
7 changes: 2 additions & 5 deletions bitcoin/src/taproot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,6 @@ mod test {

#[cfg(feature = "serde")]
use {
hex::test_hex_unwrap as hex,
serde_test::Configure,
serde_test::{assert_tokens, Token},
};
Expand Down Expand Up @@ -1855,10 +1854,8 @@ mod test {
#[test]
#[cfg(feature = "serde")]
fn test_merkle_branch_serde() {
let dummy_hash = hex!("03ba2a4dcd914fed29a1c630c7e811271b081a0e2f2f52cf1c197583dfd46c1b");
let hash1 = TapNodeHash::from_slice(&dummy_hash).unwrap();
let dummy_hash = hex!("8d79dedc2fa0b55167b5d28c61dbad9ce1191a433f3a1a6c8ee291631b2c94c9");
let hash2 = TapNodeHash::from_slice(&dummy_hash).unwrap();
let hash1 = TapNodeHash("03ba2a4dcd914fed29a1c630c7e811271b081a0e2f2f52cf1c197583dfd46c1b".parse::<sha256t::Hash<TapBranchTag>>().unwrap());
let hash2 = TapNodeHash("8d79dedc2fa0b55167b5d28c61dbad9ce1191a433f3a1a6c8ee291631b2c94c9".parse::<sha256t::Hash<TapBranchTag>>().unwrap());
let merkle_branch = TaprootMerkleBranch::from([hash1, hash2]);
// use serde_test to test serialization and deserialization
serde_test::assert_tokens(
Expand Down
3 changes: 2 additions & 1 deletion hashes/src/hmac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ impl<T: GeneralHash> GeneralHash for Hmac<T> {
impl<T: GeneralHash> Hash for Hmac<T> {
type Bytes = T::Bytes;

#[allow(deprecated_in_future)]
fn from_slice(sl: &[u8]) -> Result<Hmac<T>, FromSliceError> { T::from_slice(sl).map(Hmac) }

fn to_byte_array(self) -> Self::Bytes { self.0.to_byte_array() }
Expand Down Expand Up @@ -307,7 +308,7 @@ mod tests {
0x0b, 0x2d, 0x8a, 0x60, 0x0b, 0xdf, 0x4c, 0x0c,
];

let hash = Hmac::<sha512::Hash>::from_slice(&HASH_BYTES).expect("right number of bytes");
let hash = Hmac::<sha512::Hash>::from_byte_array(HASH_BYTES);
assert_tokens(&hash.compact(), &[Token::BorrowedBytes(&HASH_BYTES[..])]);
assert_tokens(
&hash.readable(),
Expand Down
3 changes: 2 additions & 1 deletion hashes/src/internal_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ macro_rules! hash_trait_impls {
}

$crate::internal_macros::arr_newtype_fmt_impl!(Hash, $bits / 8 $(, $gen: $gent)*);
serde_impl!(Hash, $bits / 8 $(, $gen: $gent)*);
serde_impl!(Hash, { $bits / 8 } $(, $gen: $gent)*);
borrow_slice_impl!(Hash $(, $gen: $gent)*);

impl<$($gen: $gent),*> $crate::_export::_core::convert::AsRef<[u8; $bits / 8]> for Hash<$($gen),*> {
Expand All @@ -97,6 +97,7 @@ macro_rules! hash_trait_impls {

const DISPLAY_BACKWARD: bool = $reverse;

#[allow(deprecated_in_future)]
fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<Hash<$($gen),*>, $crate::FromSliceError> {
Self::from_slice(sl)
}
Expand Down
2 changes: 2 additions & 0 deletions hashes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#![cfg_attr(bench, feature(test))]
// Coding conventions.
#![warn(missing_docs)]
#![warn(deprecated_in_future)]
#![doc(test(attr(warn(unused))))]
// Instead of littering the codebase for non-fuzzing and bench code just globally allow.
#![cfg_attr(hashes_fuzz, allow(dead_code, unused_imports))]
Expand Down Expand Up @@ -265,6 +266,7 @@ pub trait Hash:
const LEN: usize = Self::Bytes::LEN;

/// Copies a byte slice into a hash object.
#[deprecated(since = "TBD", note = "use `from_byte_array` instead")]
fn from_slice(sl: &[u8]) -> Result<Self, FromSliceError>;

/// Flag indicating whether user-visible serializations of this hash
Expand Down
80 changes: 32 additions & 48 deletions hashes/src/serde_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ pub mod serde_details {
use core::str::FromStr;
use core::{fmt, str};

use crate::FromSliceError;
struct HexVisitor<ValueT>(PhantomData<ValueT>);
use serde::{de, Deserializer, Serializer};
use serde::de;

/// Type used to implement serde traits for hashes as hex strings.
pub struct HexVisitor<ValueT>(PhantomData<ValueT>);

impl<ValueT> Default for HexVisitor<ValueT> {
fn default() -> Self { Self(PhantomData) }
}

impl<'de, ValueT> de::Visitor<'de> for HexVisitor<ValueT>
where
Expand Down Expand Up @@ -43,12 +48,17 @@ pub mod serde_details {
}
}

struct BytesVisitor<ValueT>(PhantomData<ValueT>);
/// Type used to implement serde traits for hashes as bytes.
pub struct BytesVisitor<ValueT, const N: usize>(PhantomData<ValueT>);

impl<'de, ValueT> de::Visitor<'de> for BytesVisitor<ValueT>
impl<ValueT, const N: usize> Default for BytesVisitor<ValueT, N> {
fn default() -> Self { Self(PhantomData) }
}

impl<'de, ValueT, const N: usize> de::Visitor<'de> for BytesVisitor<ValueT, N>
where
ValueT: SerdeHash,
<ValueT as FromStr>::Err: fmt::Display,
ValueT: crate::Hash,
ValueT: crate::Hash<Bytes = [u8; N]>,
{
type Value = ValueT;

Expand All @@ -60,41 +70,12 @@ pub mod serde_details {
where
E: de::Error,
{
SerdeHash::from_slice_delegated(v).map_err(|_| {
let bytes = <[u8; N]>::try_from(v).map_err(|_| {
// from_slice only errors on incorrect length
E::invalid_length(v.len(), &stringify!(N))
})
}
}

/// Default serialization/deserialization methods.
pub trait SerdeHash
where
Self: Sized + FromStr + fmt::Display + crate::Hash,
<Self as FromStr>::Err: fmt::Display,
{
/// Size, in bits, of the hash.
const N: usize;
})?;

/// Helper function to turn a deserialized slice into the correct hash type.
fn from_slice_delegated(sl: &[u8]) -> core::result::Result<Self, FromSliceError>;

/// Do serde serialization.
fn serialize<S: Serializer>(&self, s: S) -> core::result::Result<S::Ok, S::Error> {
if s.is_human_readable() {
s.collect_str(self)
} else {
s.serialize_bytes(<Self as crate::Hash>::as_byte_array(self).as_ref())
}
}

/// Do serde deserialization.
fn deserialize<'de, D: Deserializer<'de>>(d: D) -> core::result::Result<Self, D::Error> {
if d.is_human_readable() {
d.deserialize_str(HexVisitor::<Self>(PhantomData))
} else {
d.deserialize_bytes(BytesVisitor::<Self>(PhantomData))
}
Ok(<Self::Value as crate::Hash>::from_byte_array(bytes))
}
}
}
Expand All @@ -105,22 +86,25 @@ pub mod serde_details {
#[cfg(feature = "serde")]
macro_rules! serde_impl(
($t:ident, $len:expr $(, $gen:ident: $gent:ident)*) => (
impl<$($gen: $gent),*> $crate::serde_macros::serde_details::SerdeHash for $t<$($gen),*> {
const N : usize = $len;
fn from_slice_delegated(sl: &[u8]) -> core::result::Result<Self, $crate::FromSliceError> {
<$t<$($gen),*> as $crate::Hash>::from_slice(sl)
}
}

impl<$($gen: $gent),*> $crate::serde::Serialize for $t<$($gen),*> {
fn serialize<S: $crate::serde::Serializer>(&self, s: S) -> core::result::Result<S::Ok, S::Error> {
$crate::serde_macros::serde_details::SerdeHash::serialize(self, s)
if s.is_human_readable() {
s.collect_str(self)
} else {
s.serialize_bytes(<Self as $crate::Hash>::as_byte_array(self))
}
}
}

impl<'de $(, $gen: $gent)*> $crate::serde::Deserialize<'de> for $t<$($gen),*> {
fn deserialize<D: $crate::serde::Deserializer<'de>>(d: D) -> core::result::Result<$t<$($gen),*>, D::Error> {
$crate::serde_macros::serde_details::SerdeHash::deserialize(d)
use $crate::serde_macros::serde_details::{BytesVisitor, HexVisitor};

if d.is_human_readable() {
d.deserialize_str(HexVisitor::<Self>::default())
} else {
d.deserialize_bytes(BytesVisitor::<Self, $len>::default())
}
}
}
));
Expand Down
5 changes: 4 additions & 1 deletion hashes/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,14 @@ macro_rules! hash_newtype {
}

$crate::hex_fmt_impl!(<$newtype as $crate::Hash>::DISPLAY_BACKWARD, <$newtype as $crate::Hash>::LEN, $newtype);
$crate::serde_impl!($newtype, <$newtype as $crate::Hash>::LEN);
$crate::serde_impl!($newtype, { <$newtype as $crate::Hash>::LEN });
$crate::borrow_slice_impl!($newtype);

#[allow(unused)] // Private wrapper types may not need all functions.
impl $newtype {
/// Copies a byte slice into a hash object.
#[deprecated(since = "TBD", note = "use `from_byte_array` instead")]
#[allow(deprecated_in_future)]
pub fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<$newtype, $crate::FromSliceError> {
Ok($newtype(<$hash as $crate::Hash>::from_slice(sl)?))
}
Expand Down Expand Up @@ -235,6 +237,7 @@ macro_rules! hash_newtype {
const DISPLAY_BACKWARD: bool = $crate::hash_newtype_get_direction!($hash, $(#[$($type_attrs)*])*);

#[inline]
#[allow(deprecated_in_future)]
fn from_slice(sl: &[u8]) -> $crate::_export::_core::result::Result<$newtype, $crate::FromSliceError> {
Self::from_slice(sl)
}
Expand Down
1 change: 1 addition & 0 deletions internals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#![cfg_attr(docsrs, feature(doc_auto_cfg))]
// Coding conventions.
#![warn(missing_docs)]
#![warn(deprecated_in_future)]
#![doc(test(attr(warn(unused))))]
// Exclude lints we don't think are valuable.
#![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
Expand Down
Loading

0 comments on commit da462d6

Please sign in to comment.