Skip to content

Commit

Permalink
Merge #2746: hashes: Modify trait bounds
Browse files Browse the repository at this point in the history
d094350 hashes: Modify trait bounds (Tobin C. Harding)

Pull request description:

  Currently we require indexing trait bounds as well as `Borrow` on the `Hash` trait. We also already implement `AsRef`.

  It was observed that `Borrow<[u8]>` does not best describe what we want from the `Hash` trait implementor but rather `AsRef<[u8]>` does.

  Remove all the inexing trait bounds. Remove the `borrow::Borrow<[u8]>` trait bound. Add a `convert::AsRef<[u8]>` trait bound.

  This leaves the `Borrow<[u8]>` implementation for hashes created with `hash_newtype`, I'm not sure if this should be removed or not.

ACKs for top commit:
  apoelstra:
    ACK d094350 thanks! FWIW I think we might want to return the indexing traits one day, at least `[..]`, but we can do that post-1.0 and we have not gotten any complaints after removing them from the non-HMAC hashes, so maybe people are good with it as is.

Tree-SHA512: 2704a7e850083e4eae5844c401c6ac6fe32e0939bbe6e0fb5e21faf4745f9c98634e210f522199ceacd663be60685d7f30df6c89a7787ca11ea8294d104d813e
  • Loading branch information
apoelstra committed May 6, 2024
2 parents 1d98472 + d094350 commit 16261c7
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 44 deletions.
8 changes: 4 additions & 4 deletions bitcoin/src/bip32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl_bytes_newtype!(ChainCode, 32);

impl ChainCode {
fn from_hmac(hmac: Hmac<sha512::Hash>) -> Self {
hmac[32..].try_into().expect("half of hmac is guaranteed to be 32 bytes")
hmac.as_ref()[32..].try_into().expect("half of hmac is guaranteed to be 32 bytes")
}
}

Expand Down Expand Up @@ -566,7 +566,7 @@ impl Xpriv {
depth: 0,
parent_fingerprint: Default::default(),
child_number: ChildNumber::from_normal_idx(0)?,
private_key: secp256k1::SecretKey::from_slice(&hmac_result[..32])?,
private_key: secp256k1::SecretKey::from_slice(&hmac_result.as_ref()[..32])?,
chain_code: ChainCode::from_hmac(hmac_result),
})
}
Expand Down Expand Up @@ -621,7 +621,7 @@ impl Xpriv {

hmac_engine.input(&u32::from(i).to_be_bytes());
let hmac_result: Hmac<sha512::Hash> = Hmac::from_engine(hmac_engine);
let sk = secp256k1::SecretKey::from_slice(&hmac_result[..32])
let sk = secp256k1::SecretKey::from_slice(&hmac_result.as_ref()[..32])
.expect("statistically impossible to hit");
let tweaked =
sk.add_tweak(&self.private_key.into()).expect("statistically impossible to hit");
Expand Down Expand Up @@ -742,7 +742,7 @@ impl Xpub {

let hmac_result: Hmac<sha512::Hash> = Hmac::from_engine(hmac_engine);

let private_key = secp256k1::SecretKey::from_slice(&hmac_result[..32])?;
let private_key = secp256k1::SecretKey::from_slice(&hmac_result.as_ref()[..32])?;
let chain_code = ChainCode::from_hmac(hmac_result);
Ok((private_key, chain_code))
}
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/psbt/map/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ where
if <H as hashes::Hash>::hash(&val) != key_val {
return Err(psbt::Error::InvalidPreimageHashPair {
preimage: val.into_boxed_slice(),
hash: Box::from(key_val.borrow()),
hash: Box::from(key_val.as_ref()),
hash_type,
});
}
Expand Down
39 changes: 7 additions & 32 deletions hashes/src/hmac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! Hash-based Message Authentication Code (HMAC).
//!

use core::{borrow, fmt, ops, str};
use core::{convert, fmt, str};

#[cfg(feature = "serde")]
use serde::{Deserialize, Deserializer, Serialize, Serializer};
Expand Down Expand Up @@ -72,10 +72,10 @@ impl<T: Hash> HmacEngine<T> {

if key.len() > T::Engine::BLOCK_SIZE {
let hash = <T as Hash>::hash(key);
for (b_i, b_h) in ipad.iter_mut().zip(&hash[..]) {
for (b_i, b_h) in ipad.iter_mut().zip(hash.as_ref()) {
*b_i ^= *b_h;
}
for (b_o, b_h) in opad.iter_mut().zip(&hash[..]) {
for (b_o, b_h) in opad.iter_mut().zip(hash.as_ref()) {
*b_o ^= *b_h;
}
} else {
Expand Down Expand Up @@ -124,33 +124,8 @@ impl<T: Hash> fmt::LowerHex for Hmac<T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0, f) }
}

impl<T: Hash> ops::Index<usize> for Hmac<T> {
type Output = u8;
fn index(&self, index: usize) -> &u8 { &self.0[index] }
}

impl<T: Hash> ops::Index<ops::Range<usize>> for Hmac<T> {
type Output = [u8];
fn index(&self, index: ops::Range<usize>) -> &[u8] { &self.0[index] }
}

impl<T: Hash> ops::Index<ops::RangeFrom<usize>> for Hmac<T> {
type Output = [u8];
fn index(&self, index: ops::RangeFrom<usize>) -> &[u8] { &self.0[index] }
}

impl<T: Hash> ops::Index<ops::RangeTo<usize>> for Hmac<T> {
type Output = [u8];
fn index(&self, index: ops::RangeTo<usize>) -> &[u8] { &self.0[index] }
}

impl<T: Hash> ops::Index<ops::RangeFull> for Hmac<T> {
type Output = [u8];
fn index(&self, index: ops::RangeFull) -> &[u8] { &self.0[index] }
}

impl<T: Hash> borrow::Borrow<[u8]> for Hmac<T> {
fn borrow(&self) -> &[u8] { &self[..] }
impl<T: Hash> convert::AsRef<[u8]> for Hmac<T> {
fn as_ref(&self) -> &[u8] { self.0.as_ref() }
}

impl<T: Hash> Hash for Hmac<T> {
Expand All @@ -159,7 +134,7 @@ impl<T: Hash> Hash for Hmac<T> {

fn from_engine(mut e: HmacEngine<T>) -> Hmac<T> {
let ihash = T::from_engine(e.iengine);
e.oengine.input(&ihash[..]);
e.oengine.input(ihash.as_ref());
let ohash = T::from_engine(e.oengine);
Hmac(ohash)
}
Expand Down Expand Up @@ -318,7 +293,7 @@ mod tests {
let mut engine = HmacEngine::<sha256::Hash>::new(&test.key);
engine.input(&test.input);
let hash = Hmac::<sha256::Hash>::from_engine(engine);
assert_eq!(&hash[..], &test.output[..]);
assert_eq!(hash.as_ref(), &test.output[..]);
assert_eq!(hash.as_byte_array(), test.output.as_slice());
}
}
Expand Down
9 changes: 2 additions & 7 deletions hashes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub mod sha512;
pub mod sha512_256;
pub mod siphash24;

use core::{borrow, fmt, hash, ops};
use core::{convert, fmt, hash};

pub use hmac::{Hmac, HmacEngine};

Expand Down Expand Up @@ -158,12 +158,7 @@ pub trait Hash:
+ fmt::Debug
+ fmt::Display
+ fmt::LowerHex
+ ops::Index<ops::RangeFull, Output = [u8]>
+ ops::Index<ops::RangeFrom<usize>, Output = [u8]>
+ ops::Index<ops::RangeTo<usize>, Output = [u8]>
+ ops::Index<ops::Range<usize>, Output = [u8]>
+ ops::Index<usize, Output = u8>
+ borrow::Borrow<[u8]>
+ convert::AsRef<[u8]>
{
/// A hashing engine which bytes can be serialized into. It is expected
/// to implement the `io::Write` trait, and to never return errors under
Expand Down

0 comments on commit 16261c7

Please sign in to comment.