Skip to content

Commit

Permalink
Merge rust-bitcoin#3296: Remove the SliceIndex implementation from …
Browse files Browse the repository at this point in the history
…hash types

3b7ba4f Remove the SliceIndex implementation from hash types (Tobin C. Harding)

Pull request description:

  If folk really want to index into a hash they can us `as_byte_array` then index that.

  Includes a bump to the version number of `hashes` to `v0.15.0` - this is because otherwise `secp` won't build since we are breaking an API that is used in the current release of secp.

  Fix: rust-bitcoin#3115

ACKs for top commit:
  apoelstra:
    ACK 3b7ba4f successfully ran local tests

Tree-SHA512: 0ba93268cd8133fe683183c5e39ab8b3bf25c15bfa5767d2934d67a5f6a0d2f65f6c9304952315fe8a33abfce488d810a8d28400a28facfb658879ed06acca63
  • Loading branch information
apoelstra committed Oct 10, 2024
2 parents d5ebb19 + 3b7ba4f commit fe62d94
Show file tree
Hide file tree
Showing 31 changed files with 78 additions and 94 deletions.
17 changes: 13 additions & 4 deletions Cargo-minimal.lock
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ name = "base58ck"
version = "0.1.0"
dependencies = [
"bitcoin-internals",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"hex-conservative",
]

Expand Down Expand Up @@ -64,7 +64,7 @@ dependencies = [
"bitcoin-io",
"bitcoin-primitives",
"bitcoin-units",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"bitcoinconsensus",
"hex-conservative",
"hex_lit",
Expand Down Expand Up @@ -114,7 +114,7 @@ dependencies = [
"bitcoin-internals",
"bitcoin-io",
"bitcoin-units",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"hex-conservative",
"mutagen",
"ordered",
Expand All @@ -135,6 +135,15 @@ dependencies = [
[[package]]
name = "bitcoin_hashes"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bb18c03d0db0247e147a21a6faafd5a7eb851c743db062de72018b6b7e8e4d16"
dependencies = [
"hex-conservative",
]

[[package]]
name = "bitcoin_hashes"
version = "0.15.0"
dependencies = [
"bitcoin-io",
"hex-conservative",
Expand Down Expand Up @@ -365,7 +374,7 @@ version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0e0cc0f1cf93f4969faf3ea1c7d8a9faed25918d96affa959720823dfe86d4f3"
dependencies = [
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"rand",
"secp256k1-sys",
"serde",
Expand Down
17 changes: 13 additions & 4 deletions Cargo-recent.lock
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ name = "base58ck"
version = "0.1.0"
dependencies = [
"bitcoin-internals",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"hex-conservative",
]

Expand Down Expand Up @@ -63,7 +63,7 @@ dependencies = [
"bitcoin-io",
"bitcoin-primitives",
"bitcoin-units",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"bitcoinconsensus",
"hex-conservative",
"hex_lit",
Expand Down Expand Up @@ -113,7 +113,7 @@ dependencies = [
"bitcoin-internals",
"bitcoin-io",
"bitcoin-units",
"bitcoin_hashes",
"bitcoin_hashes 0.15.0",
"hex-conservative",
"mutagen",
"ordered",
Expand All @@ -134,6 +134,15 @@ dependencies = [
[[package]]
name = "bitcoin_hashes"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bb18c03d0db0247e147a21a6faafd5a7eb851c743db062de72018b6b7e8e4d16"
dependencies = [
"hex-conservative",
]

[[package]]
name = "bitcoin_hashes"
version = "0.15.0"
dependencies = [
"bitcoin-io",
"hex-conservative",
Expand Down Expand Up @@ -348,7 +357,7 @@ version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0e0cc0f1cf93f4969faf3ea1c7d8a9faed25918d96affa959720823dfe86d4f3"
dependencies = [
"bitcoin_hashes",
"bitcoin_hashes 0.14.0",
"rand",
"secp256k1-sys",
"serde",
Expand Down
2 changes: 1 addition & 1 deletion base58/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ std = ["alloc", "hashes/std", "internals/std"]
alloc = ["hashes/alloc", "internals/alloc"]

[dependencies]
hashes = { package = "bitcoin_hashes", version = "0.14.0", default-features = false }
hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false }
internals = { package = "bitcoin-internals", version = "0.4.0" }

[dev-dependencies]
Expand Down
4 changes: 2 additions & 2 deletions base58/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub fn decode_check(data: &str) -> Result<Vec<u8>, Error> {
let check_start = ret.len() - 4;

let hash_check =
sha256d::Hash::hash(&ret[..check_start])[..4].try_into().expect("4 byte slice");
sha256d::Hash::hash(&ret[..check_start]).as_byte_array()[..4].try_into().expect("4 byte slice");
let data_check = ret[check_start..].try_into().expect("4 byte slice");

let expected = u32::from_le_bytes(hash_check);
Expand Down Expand Up @@ -165,7 +165,7 @@ pub fn encode_check_to_fmt(fmt: &mut fmt::Formatter, data: &[u8]) -> fmt::Result

fn encode_check_to_writer(fmt: &mut impl fmt::Write, data: &[u8]) -> fmt::Result {
let checksum = sha256d::Hash::hash(data);
let iter = data.iter().cloned().chain(checksum[0..4].iter().cloned());
let iter = data.iter().cloned().chain(checksum.as_byte_array()[0..4].iter().cloned());
let reserve_len = encoded_check_reserve_len(data.len());
if reserve_len <= SHORT_OPT_BUFFER_LEN {
format_iter(fmt, iter, &mut ArrayVec::<u8, SHORT_OPT_BUFFER_LEN>::new())
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ arbitrary = ["dep:arbitrary", "units/arbitrary", "primitives/arbitrary"]
[dependencies]
base58 = { package = "base58ck", version = "0.1.0", default-features = false, features = ["alloc"] }
bech32 = { version = "0.11.0", default-features = false, features = ["alloc"] }
hashes = { package = "bitcoin_hashes", version = "0.14.0", default-features = false, features = ["alloc", "bitcoin-io"] }
hashes = { package = "bitcoin_hashes", version = "0.15.0", default-features = false, features = ["alloc", "bitcoin-io"] }
hex = { package = "hex-conservative", version = "0.2.0", default-features = false, features = ["alloc"] }
internals = { package = "bitcoin-internals", version = "0.4.0", features = ["alloc"] }
io = { package = "bitcoin-io", version = "0.2.0", default-features = false, features = ["alloc"] }
Expand Down
7 changes: 7 additions & 0 deletions bitcoin/contrib/whitelist_deps.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# No shebang, this file should not be executed.
# shellcheck disable=SC2148
#
# disable verify unused vars, despite the fact that they are used when sourced
# shellcheck disable=SC2034

DUPLICATE_DEPS=("hashes")
6 changes: 3 additions & 3 deletions bitcoin/src/bip152.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl ShortId {
// 2. Running SipHash-2-4 with the input being the transaction ID and the keys (k0/k1)
// set to the first two little-endian 64-bit integers from the above hash, respectively.
(
u64::from_le_bytes(h[0..8].try_into().expect("8 byte slice")),
u64::from_le_bytes(h[8..16].try_into().expect("8 byte slice")),
u64::from_le_bytes(h.as_byte_array()[0..8].try_into().expect("8 byte slice")),
u64::from_le_bytes(h.as_byte_array()[8..16].try_into().expect("8 byte slice")),
)
}

Expand All @@ -124,7 +124,7 @@ impl ShortId {

// 3. Dropping the 2 most significant bytes from the SipHash output to make it 6 bytes.
let mut id = ShortId([0; 6]);
id.0.copy_from_slice(&hash[0..6]);
id.0.copy_from_slice(&hash.as_byte_array()[0..6]);
id
}
}
Expand Down
4 changes: 2 additions & 2 deletions bitcoin/src/bip32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ impl Xpriv {

/// Returns the first four bytes of the identifier
pub fn fingerprint<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> Fingerprint {
self.identifier(secp)[0..4].try_into().expect("4 is the fingerprint length")
self.identifier(secp).as_byte_array()[0..4].try_into().expect("4 is the fingerprint length")
}
}

Expand Down Expand Up @@ -886,7 +886,7 @@ impl Xpub {

/// Returns the first four bytes of the identifier
pub fn fingerprint(&self) -> Fingerprint {
self.identifier()[0..4].try_into().expect("4 is the fingerprint length")
self.identifier().as_byte_array()[0..4].try_into().expect("4 is the fingerprint length")
}
}

Expand Down
1 change: 1 addition & 0 deletions bitcoin/src/consensus/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ impl Decodable for Box<[u8]> {
/// Does a double-SHA256 on `data` and returns the first 4 bytes.
fn sha2_checksum(data: &[u8]) -> [u8; 4] {
let checksum = <sha256d::Hash as GeneralHash>::hash(data);
let checksum = checksum.to_byte_array();
[checksum[0], checksum[1], checksum[2], checksum[3]]
}

Expand Down
1 change: 1 addition & 0 deletions bitcoin/src/p2p/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ impl RawNetworkMessage {
let payload_len = payload.consensus_encode(&mut engine).expect("engine doesn't error");
let payload_len = u32::try_from(payload_len).expect("network message use u32 as length");
let checksum = sha256d::Hash::from_engine(engine);
let checksum = checksum.to_byte_array();
let checksum = [checksum[0], checksum[1], checksum[2], checksum[3]];
Self { magic, payload, payload_len, checksum }
}
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/hashes/ripemd160.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn do_test(data: &[u8]) {
let eng_hash = ripemd160::Hash::from_engine(engine);

let hash = ripemd160::Hash::hash(data);
assert_eq!(&hash[..], &eng_hash[..]);
assert_eq!(hash.as_byte_array(), eng_hash.as_byte_array());
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/hashes/sha1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn do_test(data: &[u8]) {
let eng_hash = sha1::Hash::from_engine(engine);

let hash = sha1::Hash::hash(data);
assert_eq!(&hash[..], &eng_hash[..]);
assert_eq!(hash.as_byte_array(), eng_hash.as_byte_array());
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/hashes/sha256.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn do_test(data: &[u8]) {
let eng_hash = sha256::Hash::from_engine(engine);

let hash = sha256::Hash::hash(data);
assert_eq!(&hash[..], &eng_hash[..]);
assert_eq!(hash.as_byte_array(), eng_hash.as_byte_array());
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/hashes/sha512.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn do_test(data: &[u8]) {
let eng_hash = sha512::Hash::from_engine(engine);

let hash = sha512::Hash::hash(data);
assert_eq!(&hash[..], &eng_hash[..]);
assert_eq!(hash.as_byte_array(), eng_hash.as_byte_array());
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/hashes/sha512_256.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn do_test(data: &[u8]) {
let eng_hash = sha512_256::Hash::from_engine(engine);

let hash = sha512_256::Hash::hash(data);
assert_eq!(&hash[..], &eng_hash[..]);
assert_eq!(hash.as_byte_array(), eng_hash.as_byte_array());
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion hashes/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "bitcoin_hashes"
version = "0.14.0"
version = "0.15.0"
authors = ["Andrew Poelstra <[email protected]>"]
license = "CC0-1.0"
repository = "https://github.com/rust-bitcoin/rust-bitcoin"
Expand Down
16 changes: 8 additions & 8 deletions hashes/src/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,55 +105,55 @@ mod benches {
fn bench_32b_constant_time_cmp_ne(bh: &mut Bencher) {
let hash_a = sha256::Hash::hash(&[0; 1]);
let hash_b = sha256::Hash::hash(&[1; 1]);
bh.iter(|| fixed_time_eq(&hash_a[..], &hash_b[..]))
bh.iter(|| fixed_time_eq(hash_a.as_byte_array(), hash_b.as_byte_array()))
}

#[bench]
fn bench_32b_slice_cmp_ne(bh: &mut Bencher) {
let hash_a = sha256::Hash::hash(&[0; 1]);
let hash_b = sha256::Hash::hash(&[1; 1]);
bh.iter(|| &hash_a[..] == &hash_b[..])
bh.iter(|| hash_a.as_byte_array() == hash_b.as_byte_array())
}

#[bench]
fn bench_32b_constant_time_cmp_eq(bh: &mut Bencher) {
let hash_a = sha256::Hash::hash(&[0; 1]);
let hash_b = sha256::Hash::hash(&[0; 1]);
bh.iter(|| fixed_time_eq(&hash_a[..], &hash_b[..]))
bh.iter(|| fixed_time_eq(hash_a.as_byte_array(), hash_b.as_byte_array()))
}

#[bench]
fn bench_32b_slice_cmp_eq(bh: &mut Bencher) {
let hash_a = sha256::Hash::hash(&[0; 1]);
let hash_b = sha256::Hash::hash(&[0; 1]);
bh.iter(|| &hash_a[..] == &hash_b[..])
bh.iter(|| hash_a.as_byte_array() == hash_b.as_byte_array())
}

#[bench]
fn bench_64b_constant_time_cmp_ne(bh: &mut Bencher) {
let hash_a = sha512::Hash::hash(&[0; 1]);
let hash_b = sha512::Hash::hash(&[1; 1]);
bh.iter(|| fixed_time_eq(&hash_a[..], &hash_b[..]))
bh.iter(|| fixed_time_eq(hash_a.as_byte_array(), hash_b.as_byte_array()))
}

#[bench]
fn bench_64b_slice_cmp_ne(bh: &mut Bencher) {
let hash_a = sha512::Hash::hash(&[0; 1]);
let hash_b = sha512::Hash::hash(&[1; 1]);
bh.iter(|| &hash_a[..] == &hash_b[..])
bh.iter(|| hash_a.as_byte_array() == hash_b.as_byte_array())
}

#[bench]
fn bench_64b_constant_time_cmp_eq(bh: &mut Bencher) {
let hash_a = sha512::Hash::hash(&[0; 1]);
let hash_b = sha512::Hash::hash(&[0; 1]);
bh.iter(|| fixed_time_eq(&hash_a[..], &hash_b[..]))
bh.iter(|| fixed_time_eq(hash_a.as_byte_array(), hash_b.as_byte_array()))
}

#[bench]
fn bench_64b_slice_cmp_eq(bh: &mut Bencher) {
let hash_a = sha512::Hash::hash(&[0; 1]);
let hash_b = sha512::Hash::hash(&[0; 1]);
bh.iter(|| &hash_a[..] == &hash_b[..])
bh.iter(|| hash_a.as_byte_array() == hash_b.as_byte_array())
}
}
9 changes: 3 additions & 6 deletions hashes/src/hash160.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@

//! HASH160 (SHA256 then RIPEMD160) implementation.
use core::ops::Index;
use core::slice::SliceIndex;

use crate::{ripemd160, sha256};

crate::internal_macros::hash_type! {
Expand Down Expand Up @@ -39,10 +36,10 @@ impl crate::HashEngine for HashEngine {

fn from_engine(e: HashEngine) -> Hash {
let sha2 = sha256::Hash::from_engine(e.0);
let rmd = ripemd160::Hash::hash(&sha2[..]);
let rmd = ripemd160::Hash::hash(sha2.as_byte_array());

let mut ret = [0; 20];
ret.copy_from_slice(&rmd[..]);
ret.copy_from_slice(rmd.as_byte_array());
Hash(ret)
}

Expand Down Expand Up @@ -90,7 +87,7 @@ mod tests {
// Hash through high-level API, check hex encoding/decoding
let hash = hash160::Hash::hash(&test.input[..]);
assert_eq!(hash, test.output_str.parse::<hash160::Hash>().expect("parse hex"));
assert_eq!(hash[..], test.output);
assert_eq!(hash.as_byte_array(), &test.output);
assert_eq!(hash.to_string(), test.output_str);

// Hash through engine, checking that we can input byte by byte
Expand Down
9 changes: 0 additions & 9 deletions hashes/src/internal_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,6 @@ macro_rules! hash_trait_impls {
}
}

impl<I: SliceIndex<[u8]> $(, $gen: $gent)*> Index<I> for Hash<$($gen),*> {
type Output = I::Output;

#[inline]
fn index(&self, index: I) -> &Self::Output {
&self.0[index]
}
}

impl<$($gen: $gent),*> $crate::GeneralHash for Hash<$($gen),*> {
type Engine = HashEngine;

Expand Down
4 changes: 1 addition & 3 deletions hashes/src/ripemd160.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
//! RIPEMD160 implementation.
use core::cmp;
use core::ops::Index;
use core::slice::SliceIndex;

use crate::{incomplete_block_len, HashEngine as _};

Expand Down Expand Up @@ -479,7 +477,7 @@ mod tests {
// Hash through high-level API, check hex encoding/decoding
let hash = ripemd160::Hash::hash(test.input.as_bytes());
assert_eq!(hash, test.output_str.parse::<ripemd160::Hash>().expect("parse hex"));
assert_eq!(hash[..], test.output);
assert_eq!(hash.as_byte_array(), &test.output);
assert_eq!(hash.to_string(), test.output_str);
assert_eq!(ripemd160::Hash::from_bytes_ref(&test.output), &hash);
assert_eq!(ripemd160::Hash::from_bytes_mut(&mut test.output), &hash);
Expand Down
Loading

0 comments on commit fe62d94

Please sign in to comment.