Skip to content

Commit

Permalink
Merge #686: Deprecate ThirtyTwoByteHash
Browse files Browse the repository at this point in the history 
9f28cf6 Deprecate ThirtyTwoByteHash (Tobin C. Harding)
88c8c58 Fix import warnings (Tobin C. Harding)

Pull request description:

  The implementations of `ThirtyTwoByteHash` for types from the `hashes` crate are problematic during upgrades because both `bitcoin` and `secp256k1` depend on `hashes` and when the versions of `hashes` get out of sync usage of the trait breaks.

  Deprecate the `ThirtyTwoByteHash` trait and remove the impls for types from `bitcoin_hashes`.

  Add an explanation in the changelog because its too long to go in the deprecation message.

  Close: #673

ACKs for top commit:
  apoelstra:
    ACK 9f28cf6

Tree-SHA512: a7598b09c6a2f49913a9effad3e1ed9b0663970ac20fbfe5fc2f1329daaa2b4cab78b00128a03d0f3f6559ed7964b09e0bd939e60cf130b2cc6e609c90df6868
  • Loading branch information
@apoelstra
apoelstra committed Apr 2, 2024
2 parents d279c13 + 9f28cf6 commit a05078f
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 106 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
@@ -1,5 +1,14 @@
# Unreleased

* Deprecate `ThirtyTwoByteHash`

This trait turned out to be problematic during upgrade because we support a ranged dependency for
`bitcoin_hashes`. Consider implementing `From<T> for Message` for your type iff your type is a 32
byte hash (ie, output from a hash algorithm that produces a 32 byte digest like sha256). When
using the impl, consider using `Message::from` instead of `hash.into()` because we will be
introducing generics in a future version and the compiler will not be able to work out the target
type.

* Bump MSRV to Rust `v1.56.1`
* Upgrade `hashes` using range dependency `version = ">= 0.12, <= 0.14"`.

Expand Down
8 changes: 0 additions & 8 deletions api/all-features.txt
View file
Expand Up @@ -519,8 +519,6 @@ impl secp256k1_sys::CPtr for secp256k1::PublicKey
impl secp256k1_sys::CPtr for secp256k1::schnorr::Signature
impl secp256k1_sys::CPtr for secp256k1::SecretKey
impl secp256k1_sys::CPtr for secp256k1::XOnlyPublicKey
impl secp256k1::ThirtyTwoByteHash for bitcoin_hashes::sha256d::Hash
impl secp256k1::ThirtyTwoByteHash for bitcoin_hashes::sha256::Hash
impl secp256k1::Verification for secp256k1::All
impl secp256k1::Verification for secp256k1::VerifyOnly
impl secp256k1::XOnlyPublicKey
Expand All @@ -532,7 +530,6 @@ impl serde::ser::Serialize for secp256k1::PublicKey
impl serde::ser::Serialize for secp256k1::schnorr::Signature
impl serde::ser::Serialize for secp256k1::SecretKey
impl serde::ser::Serialize for secp256k1::XOnlyPublicKey
impl<T: bitcoin_hashes::sha256t::Tag> secp256k1::ThirtyTwoByteHash for bitcoin_hashes::sha256t::Hash<T>
impl<T: secp256k1::ThirtyTwoByteHash> core::convert::From<T> for secp256k1::Message
impl<T: secp256k1::ThirtyTwoByteHash> core::convert::From<T> for secp256k1::SecretKey
#[non_exhaustive] pub struct secp256k1::scalar::OutOfRangeError
Expand Down Expand Up @@ -580,9 +577,6 @@ pub enum secp256k1::SignOnly
pub enum secp256k1::VerifyOnly
pub extern crate secp256k1::hashes
pub fn &'a secp256k1::ecdsa::serialized_signature::SerializedSignature::into_iter(self) -> Self::IntoIter
pub fn bitcoin_hashes::sha256d::Hash::into_32(self) -> [u8; 32]
pub fn bitcoin_hashes::sha256::Hash::into_32(self) -> [u8; 32]
pub fn bitcoin_hashes::sha256t::Hash<T>::into_32(self) -> [u8; 32]
pub fn i32::from(parity: secp256k1::Parity) -> i32
pub fn secp256k1::All::clone(&self) -> secp256k1::All
pub fn secp256k1::All::cmp(&self, other: &secp256k1::All) -> core::cmp::Ordering
Expand Down Expand Up @@ -772,7 +766,6 @@ pub fn secp256k1::Message::eq(&self, other: &secp256k1::Message) -> bool
pub fn secp256k1::Message::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
pub fn secp256k1::Message::from_digest(digest: [u8; 32]) -> secp256k1::Message
pub fn secp256k1::Message::from_digest_slice(digest: &[u8]) -> core::result::Result<secp256k1::Message, secp256k1::Error>
pub fn secp256k1::Message::from_hashed_data<H: secp256k1::ThirtyTwoByteHash + bitcoin_hashes::Hash>(data: &[u8]) -> Self
pub fn secp256k1::Message::from_slice(digest: &[u8]) -> core::result::Result<secp256k1::Message, secp256k1::Error>
pub fn secp256k1::Message::from(t: T) -> secp256k1::Message
pub fn secp256k1::Message::hash<__H: core::hash::Hasher>(&self, state: &mut __H)
Expand Down Expand Up @@ -904,7 +897,6 @@ pub fn secp256k1::SecretKey::deserialize<D: serde::de::Deserializer<'de>>(d: D)
pub fn secp256k1::SecretKey::display_secret(&self) -> DisplaySecret
pub fn secp256k1::SecretKey::eq(&self, other: &Self) -> bool
pub fn secp256k1::SecretKey::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
pub fn secp256k1::SecretKey::from_hashed_data<H: secp256k1::ThirtyTwoByteHash + bitcoin_hashes::Hash>(data: &[u8]) -> Self
pub fn secp256k1::SecretKey::from_keypair(keypair: &secp256k1::Keypair) -> Self
pub fn secp256k1::SecretKey::from(pair: &'a secp256k1::Keypair) -> Self
pub fn secp256k1::SecretKey::from(pair: secp256k1::Keypair) -> Self
Expand Down
1 change: 0 additions & 1 deletion src/ecdsa/serialized_signature.rs
View file
Expand Up @@ -8,7 +8,6 @@
//! serialized signatures and since it's a bit more complicated it has its own module.

use core::borrow::Borrow;
use core::convert::TryFrom;
use core::{fmt, ops};

pub use into_iter::IntoIter;
Expand Down
31 changes: 4 additions & 27 deletions src/key.rs
View file
Expand Up @@ -3,7 +3,6 @@
//! Public and secret keys.
//!

use core::convert::TryFrom;
use core::ops::{self, BitXor};
use core::{fmt, ptr, str};

Expand All @@ -14,13 +13,14 @@ use crate::ellswift::ElligatorSwift;
use crate::ffi::types::c_uint;
use crate::ffi::{self, CPtr};
use crate::Error::{self, InvalidPublicKey, InvalidPublicKeySum, InvalidSecretKey};
#[cfg(feature = "hashes")]
#[allow(deprecated)]
use crate::ThirtyTwoByteHash;
#[cfg(feature = "global-context")]
use crate::SECP256K1;
use crate::{
constants, ecdsa, from_hex, schnorr, Message, Scalar, Secp256k1, Signing, Verification,
};
#[cfg(feature = "hashes")]
use crate::{hashes, ThirtyTwoByteHash};

/// Secret key - a 256-bit key used to create ECDSA and Taproot signatures.
///
Expand Down Expand Up @@ -257,30 +257,6 @@ impl SecretKey {
SecretKey(sk)
}

/// Constructs a [`SecretKey`] by hashing `data` with hash algorithm `H`.
///
/// Requires the feature `hashes` to be enabled.
///
/// # Examples
///
/// ```
/// # #[cfg(feature="hashes")] {
/// use secp256k1::hashes::{sha256, Hash};
/// use secp256k1::SecretKey;
///
/// let sk1 = SecretKey::from_hashed_data::<sha256::Hash>("Hello world!".as_bytes());
/// // is equivalent to
/// let sk2 = SecretKey::from(sha256::Hash::hash("Hello world!".as_bytes()));
///
/// assert_eq!(sk1, sk2);
/// # }
/// ```
#[cfg(feature = "hashes")]
#[inline]
pub fn from_hashed_data<H: ThirtyTwoByteHash + hashes::Hash>(data: &[u8]) -> Self {
<H as hashes::Hash>::hash(data).into()
}

/// Returns the secret key as a byte value.
#[inline]
pub fn secret_bytes(&self) -> [u8; constants::SECRET_KEY_SIZE] { self.0 }
Expand Down Expand Up @@ -373,6 +349,7 @@ impl SecretKey {
}

#[cfg(feature = "hashes")]
#[allow(deprecated)]
impl<T: ThirtyTwoByteHash> From<T> for SecretKey {
/// Converts a 32-byte hash directly to a secret key without error paths.
fn from(t: T) -> SecretKey {
Expand Down
79 changes: 12 additions & 67 deletions src/lib.rs
View file
Expand Up @@ -31,11 +31,12 @@
//! # #[cfg(all(feature = "rand-std", feature = "hashes-std"))] {
//! use secp256k1::rand::rngs::OsRng;
//! use secp256k1::{Secp256k1, Message};
//! use secp256k1::hashes::sha256;
//! use secp256k1::hashes::{sha256, Hash};
//!
//! let secp = Secp256k1::new();
//! let (secret_key, public_key) = secp.generate_keypair(&mut OsRng);
//! let message = Message::from_hashed_data::<sha256::Hash>("Hello World!".as_bytes());
//! let digest = sha256::Hash::hash("Hello World!".as_bytes());
//! let message = Message::from_digest(digest.to_byte_array());
//!
//! let sig = secp.sign_ecdsa(&message, &secret_key);
//! assert!(secp.verify_ecdsa(&message, &sig, &public_key).is_ok());
Expand All @@ -47,10 +48,11 @@
//! ```rust
//! # #[cfg(all(feature = "global-context", feature = "hashes-std", feature = "rand-std"))] {
//! use secp256k1::{generate_keypair, Message};
//! use secp256k1::hashes::sha256;
//! use secp256k1::hashes::{sha256, Hash};
//!
//! let (secret_key, public_key) = generate_keypair(&mut rand::thread_rng());
//! let message = Message::from_hashed_data::<sha256::Hash>("Hello World!".as_bytes());
//! let digest = sha256::Hash::hash("Hello World!".as_bytes());
//! let message = Message::from_digest(digest.to_byte_array());
//!
//! let sig = secret_key.sign_ecdsa(message);
//! assert!(sig.verify(&message, &public_key).is_ok());
Expand Down Expand Up @@ -176,8 +178,6 @@ use core::{fmt, mem, str};

#[cfg(all(feature = "global-context", feature = "std"))]
pub use context::global::{self, SECP256K1};
#[cfg(feature = "hashes")]
use hashes::Hash;
#[cfg(feature = "rand")]
pub use rand;
pub use secp256k1_sys as ffi;
Expand All @@ -198,34 +198,23 @@ pub use crate::scalar::Scalar;
/// Trait describing something that promises to be a 32-byte random number; in particular,
/// it has negligible probability of being zero or overflowing the group order. Such objects
/// may be converted to `Message`s without any error paths.
#[deprecated(
since = "0.29.0",
note = "Please see v0.29.0 rust-secp256k1/CHANGELOG.md for suggestion"
)]
pub trait ThirtyTwoByteHash {
/// Converts the object into a 32-byte array
fn into_32(self) -> [u8; 32];
}

#[cfg(feature = "hashes")]
impl ThirtyTwoByteHash for hashes::sha256::Hash {
fn into_32(self) -> [u8; 32] { self.to_byte_array() }
}

#[cfg(feature = "hashes")]
impl ThirtyTwoByteHash for hashes::sha256d::Hash {
fn into_32(self) -> [u8; 32] { self.to_byte_array() }
}

#[cfg(feature = "hashes")]
impl<T: hashes::sha256t::Tag> ThirtyTwoByteHash for hashes::sha256t::Hash<T> {
fn into_32(self) -> [u8; 32] { self.to_byte_array() }
}

/// A (hashed) message input to an ECDSA signature.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Message([u8; constants::MESSAGE_SIZE]);
impl_array_newtype!(Message, u8, constants::MESSAGE_SIZE);
impl_pretty_debug!(Message);

impl Message {
/// **If you just want to sign an arbitrary message use `Message::from_hashed_data` instead.**
/// Creates a [`Message`] from a 32 byte slice `digest`.
///
/// Converts a `MESSAGE_SIZE`-byte slice to a message object. **WARNING:** the slice has to be a
/// cryptographically secure hash of the actual message that's going to be signed. Otherwise
Expand All @@ -239,8 +228,6 @@ impl Message {

/// Creates a [`Message`] from a `digest`.
///
/// **If you just want to sign an arbitrary message use `Message::from_hashed_data` instead.**
///
/// The `digest` array has to be a cryptographically secure hash of the actual message that's
/// going to be signed. Otherwise the result of signing isn't a [secure signature].
///
Expand All @@ -250,8 +237,6 @@ impl Message {

/// Creates a [`Message`] from a 32 byte slice `digest`.
///
/// **If you just want to sign an arbitrary message use `Message::from_hashed_data` instead.**
///
/// The slice has to be 32 bytes long and be a cryptographically secure hash of the actual
/// message that's going to be signed. Otherwise the result of signing isn't a [secure
/// signature].
Expand All @@ -272,31 +257,9 @@ impl Message {
_ => Err(Error::InvalidMessage),
}
}

/// Constructs a [`Message`] by hashing `data` with hash algorithm `H`.
///
/// Requires the feature `hashes` to be enabled.
///
/// # Examples
///
/// ```
/// # #[cfg(feature = "hashes")] {
/// use secp256k1::hashes::{sha256, Hash};
/// use secp256k1::Message;
///
/// let m1 = Message::from_hashed_data::<sha256::Hash>("Hello world!".as_bytes());
/// // is equivalent to
/// let m2 = Message::from(sha256::Hash::hash("Hello world!".as_bytes()));
///
/// assert_eq!(m1, m2);
/// # }
/// ```
#[cfg(feature = "hashes")]
pub fn from_hashed_data<H: ThirtyTwoByteHash + hashes::Hash>(data: &[u8]) -> Self {
<H as hashes::Hash>::hash(data).into()
}
}

#[allow(deprecated)]
impl<T: ThirtyTwoByteHash> From<T> for Message {
/// Converts a 32-byte hash directly to a message without error paths.
fn from(t: T) -> Message { Message(t.into_32()) }
Expand Down Expand Up @@ -1043,24 +1006,6 @@ mod tests {
let sig = SECP256K1.sign_ecdsa(&msg, &sk);
assert!(SECP256K1.verify_ecdsa(&msg, &sig, &pk).is_ok());
}

#[cfg(feature = "hashes")]
#[test]
fn test_from_hash() {
use hashes::{sha256, sha256d, Hash};

let test_bytes = "Hello world!".as_bytes();

let hash = sha256::Hash::hash(test_bytes);
let msg = Message::from(hash);
assert_eq!(msg.0, hash.to_byte_array());
assert_eq!(msg, Message::from_hashed_data::<hashes::sha256::Hash>(test_bytes));

let hash = sha256d::Hash::hash(test_bytes);
let msg = Message::from(hash);
assert_eq!(msg.0, hash.to_byte_array());
assert_eq!(msg, Message::from_hashed_data::<hashes::sha256d::Hash>(test_bytes));
}
}

#[cfg(bench)]
Expand Down
4 changes: 1 addition & 3 deletions src/schnorr.rs
View file
Expand Up @@ -12,9 +12,7 @@ use crate::ffi::{self, CPtr};
use crate::key::{Keypair, XOnlyPublicKey};
#[cfg(feature = "global-context")]
use crate::SECP256K1;
use crate::{
constants, from_hex, impl_array_newtype, Error, Message, Secp256k1, Signing, Verification,
};
use crate::{constants, from_hex, Error, Message, Secp256k1, Signing, Verification};

/// Represents a schnorr signature.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand Down

0 comments on commit a05078f

Please sign in to comment.