Skip to content

Commit

Permalink
Remove Hash impl for new hashtypes
Browse files Browse the repository at this point in the history
New hash types created with `hash_newtype` are not meant to be general
purpose hashes but currently we implement `Hash` for the newtype, this
makes the API for the newtype misleading.

Instead of implementing `Hash` we can make the `Hash` trait methods
inherent on the new type, this allows the module that creates the new
type to hash stuff ergonomically but does not pollute the public API for
the type.
  • Loading branch information
tcharding committed Apr 18, 2024
1 parent 6194592 commit aead5c3
Show file tree
Hide file tree
Showing 20 changed files with 94 additions and 84 deletions.
1 change: 0 additions & 1 deletion bitcoin/examples/sign-tx-segwit-v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use std::str::FromStr;

use bitcoin::hashes::Hash;
use bitcoin::locktime::absolute;
use bitcoin::secp256k1::{rand, Message, Secp256k1, SecretKey, Signing};
use bitcoin::sighash::{EcdsaSighashType, SighashCache};
Expand Down
1 change: 0 additions & 1 deletion bitcoin/examples/sign-tx-taproot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use std::str::FromStr;

use bitcoin::hashes::Hash;
use bitcoin::key::{Keypair, TapTweak, TweakedKeypair, UntweakedPublicKey};
use bitcoin::locktime::absolute;
use bitcoin::secp256k1::{rand, Message, Secp256k1, SecretKey, Signing, Verification};
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/bip158.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
use core::cmp::{self, Ordering};
use core::fmt::{self, Display, Formatter};

use hashes::{sha256d, siphash24, Hash};
use hashes::{sha256d, siphash24};
use internals::write_err;
use io::{BufRead, Write};

Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/blockdata/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use core::fmt;

use hashes::{sha256d, Hash, HashEngine};
use hashes::{sha256d, HashEngine};
use io::{BufRead, Write};

use super::Weight;
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/blockdata/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! single transaction.
//!

use hashes::{sha256d, Hash};
use hashes::sha256d;
use hex_lit::hex;
use internals::impl_array_newtype;

Expand Down Expand Up @@ -297,7 +297,7 @@ mod test {
// The *_chain_hash tests are sanity/regression tests, they verify that the const byte array
// representing the genesis block is the same as that created by hashing the genesis block.
fn chain_hash_and_genesis_block(network: Network) {
use hashes::sha256;
use hashes::{sha256, Hash};

// The genesis block hash is a double-sha256 and it is displayed backwards.
let genesis_hash = genesis_block(network).block_hash();
Expand Down
1 change: 0 additions & 1 deletion bitcoin/src/blockdata/script/borrowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use core::ops::{
Bound, Index, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive,
};

use hashes::Hash;
use secp256k1::{Secp256k1, Verification};

use super::PushBytes;
Expand Down
1 change: 0 additions & 1 deletion bitcoin/src/blockdata/script/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use core::str::FromStr;

use hashes::Hash;
use hex_lit::hex;

use super::*;
Expand Down
1 change: 0 additions & 1 deletion bitcoin/src/blockdata/script/witness_program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

use core::fmt;

use hashes::Hash as _;
use internals::array_vec::ArrayVec;
use secp256k1::{Secp256k1, Verification};

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 @@ -13,7 +13,7 @@

use core::{cmp, fmt, str};

use hashes::{sha256d, Hash};
use hashes::sha256d;
use internals::write_err;
use io::{BufRead, Write};
use units::parse;
Expand Down
3 changes: 2 additions & 1 deletion bitcoin/src/consensus/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,8 @@ impl Encodable for TapLeafHash {

impl Decodable for TapLeafHash {
fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, Error> {
Ok(Self::from_byte_array(<<Self as Hash>::Bytes>::consensus_decode(r)?))
let digest = <[u8; Self::digest_length()]>::consensus_decode(r)?;
Ok(Self::from_byte_array(digest))
}
}

Expand Down
6 changes: 3 additions & 3 deletions bitcoin/src/crypto/sighash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1344,10 +1344,10 @@ impl<E> EncodeSigningDataResult<E> {
///
/// ```rust
/// # use bitcoin::consensus::deserialize;
/// # use bitcoin::hashes::{Hash, hex::FromHex};
/// # use bitcoin::sighash::{LegacySighash, SighashCache};
/// # use bitcoin::hashes::{sha256d, Hash, hex::FromHex};
/// # use bitcoin::sighash::SighashCache;
/// # use bitcoin::Transaction;
/// # let mut writer = LegacySighash::engine();
/// # let mut writer = sha256d::Hash::engine();
/// # let input_index = 0;
/// # let script_pubkey = bitcoin::ScriptBuf::new();
/// # let sighash_u32 = 0u32;
Expand Down
1 change: 0 additions & 1 deletion bitcoin/src/hash_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ pub use crate::{
#[cfg(test)]
mod tests {
use super::*;
use crate::hashes::Hash;
use crate::{
LegacySighash, PubkeyHash, ScriptHash, SegwitV0Sighash, TapSighash, WPubkeyHash,
WScriptHash, XKeyIdentifier,
Expand Down
6 changes: 2 additions & 4 deletions bitcoin/src/internal_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ macro_rules! impl_hashencode {

impl $crate::consensus::Decodable for $hashtype {
fn consensus_decode<R: $crate::io::BufRead + ?Sized>(r: &mut R) -> core::result::Result<Self, $crate::consensus::encode::Error> {
use $crate::hashes::Hash;
Ok(Self::from_byte_array(<<$hashtype as $crate::hashes::Hash>::Bytes>::consensus_decode(r)?))
let digest = <[u8; { $hashtype::digest_length() }]>::consensus_decode(r)?;
Ok(Self::from_byte_array(digest))
}
}
};
Expand All @@ -213,14 +213,12 @@ macro_rules! impl_asref_push_bytes {
$(
impl AsRef<$crate::blockdata::script::PushBytes> for $hashtype {
fn as_ref(&self) -> &$crate::blockdata::script::PushBytes {
use $crate::hashes::Hash;
self.as_byte_array().into()
}
}

impl From<$hashtype> for $crate::blockdata::script::PushBytesBuf {
fn from(hash: $hashtype) -> Self {
use $crate::hashes::Hash;
hash.as_byte_array().into()
}
}
Expand Down
1 change: 0 additions & 1 deletion bitcoin/src/merkle_tree/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

use core::fmt;

use hashes::Hash;
use io::{BufRead, Write};

use self::MerkleBlockError::*;
Expand Down
3 changes: 1 addition & 2 deletions bitcoin/src/merkle_tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
//!
//! ```
//! # use bitcoin::{merkle_tree, Txid};
//! # use bitcoin::hashes::Hash;
//! # let tx1 = Txid::all_zeros(); // Dummy hash values.
//! # let tx2 = Txid::all_zeros();
//! let tx_hashes = vec![tx1, tx2]; // All the hashes we wish to merkelize.
//! let root = merkle_tree::calculate_root(tx_hashes.into_iter());
//! let root = merkle_tree::calculate_root(tx_hashes.iter().map(|txid| txid.to_raw_hash()));
//! ```

mod block;
Expand Down
3 changes: 0 additions & 3 deletions bitcoin/src/pow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ impl Target {
/// to the target.
#[cfg_attr(all(test, mutate), mutate)]
pub fn is_met_by(&self, hash: BlockHash) -> bool {
use hashes::Hash;
let hash = U256::from_le_bytes(hash.to_byte_array());
hash <= self.0
}
Expand Down Expand Up @@ -1712,8 +1711,6 @@ mod tests {
fn target_is_met_by_for_target_equals_hash() {
use std::str::FromStr;

use hashes::Hash;

let hash =
BlockHash::from_str("ef537f25c895bfa782526529a9b63d97aa631564d5d789c2b765448c8635fb6c")
.expect("failed to parse block hash");
Expand Down
4 changes: 1 addition & 3 deletions bitcoin/src/taproot/merkle_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

//! Contains `TaprootMerkleBranch` and its associated types.

use hashes::Hash;

use super::{
TapNodeHash, TaprootBuilderError, TaprootError, TAPROOT_CONTROL_MAX_NODE_COUNT,
TAPROOT_CONTROL_NODE_SIZE,
Expand Down Expand Up @@ -87,7 +85,7 @@ impl TaprootMerkleBranch {
for hash in self {
writer.write_all(hash.as_ref())?;
}
Ok(self.len() * TapNodeHash::LEN)
Ok(self.len() * TapNodeHash::digest_length())
}

/// Serializes `self` as bytes.
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/taproot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use core::cmp::Reverse;
use core::fmt;
use core::iter::FusedIterator;

use hashes::{sha256t_hash_newtype, Hash, HashEngine};
use hashes::{sha256t_hash_newtype, HashEngine};
use internals::write_err;
use io::Write;
use secp256k1::{Scalar, Secp256k1};
Expand Down Expand Up @@ -1443,8 +1443,8 @@ impl std::error::Error for TaprootError {
mod test {
use core::str::FromStr;

use hashes::sha256;
use hashes::sha256t::Tag;
use hashes::{sha256, Hash};
use hex::FromHex;
use secp256k1::VerifyOnly;

Expand Down
2 changes: 1 addition & 1 deletion hashes/src/sha256t.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn from_engine<T: Tag>(e: sha256::HashEngine) -> Hash<T> {
/// The syntax is:
///
/// ```
/// # use bitcoin_hashes::sha256t_hash_newtype;
/// # use bitcoin_hashes::{sha256t_hash_newtype, Hash};
/// sha256t_hash_newtype! {
/// /// Optional documentation details here.
/// /// Summary is always generated.
Expand Down

0 comments on commit aead5c3

Please sign in to comment.