From 5230d3309c12139d344d2fd1ee297f437afad832 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 21 Aug 2024 15:30:29 +1000 Subject: [PATCH 1/8] Remove hash_reader from sha256t_hash_newtype The `hash_reader` function is new and unreleased, it should never have been put into the `sha256t_hash_newtype` macro, and its broken. --- hashes/src/sha256t.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/hashes/src/sha256t.rs b/hashes/src/sha256t.rs index 2a0a24870..bb31d466d 100644 --- a/hashes/src/sha256t.rs +++ b/hashes/src/sha256t.rs @@ -240,13 +240,6 @@ macro_rules! sha256t_hash_newtype { { <$hash_name as $crate::GeneralHash>::hash_byte_chunks(byte_slices) } - - /// Hashes the entire contents of the `reader`. - #[cfg(feature = "bitcoin-io")] - #[allow(unused)] // the user of macro may not need this - fn hash_reader(reader: &mut R) -> Result { - <$hash_name as $crate::GeneralHash>::hash_reader(reader) - } } impl $crate::GeneralHash for $hash_name { From 1af6ff43942758a75acb4d0b6b1e781bf75de1f1 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 9 Aug 2024 12:11:42 +1000 Subject: [PATCH 2/8] hashes: Feature gate hash_reader unit test The `hash_reader` function is only available when `bitcoin-io` is enabled - it should be feature gated. --- hashes/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index fb78fdbd7..897f97b8f 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -324,7 +324,7 @@ impl std::error::Error for FromSliceError {} #[cfg(test)] mod tests { use super::*; - use crate::{sha256, sha256d}; + use crate::sha256d; hash_newtype! { /// A test newtype @@ -351,7 +351,10 @@ mod tests { } #[test] + #[cfg(feature = "bitcoin-io")] fn hash_reader() { + use crate::sha256; + let mut reader: &[u8] = b"hello"; assert_eq!(sha256::Hash::hash_reader(&mut reader).unwrap(), sha256::Hash::hash(b"hello"),) } From b6fda6517c154a49e3a901f8409a4196ac66ed22 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Sat, 10 Aug 2024 06:57:04 +1000 Subject: [PATCH 3/8] Implement JsonSchema for siphash::Hash We lost this impl at some stage in the last few weeks and did not notice because of a bug in `run_task` [0]. Implement `JsonSchema` for `siphash` as we do for all the other hash types. [0] https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools/issues/10 --- hashes/src/siphash24.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hashes/src/siphash24.rs b/hashes/src/siphash24.rs index 2e57adcdd..9895ee0f5 100644 --- a/hashes/src/siphash24.rs +++ b/hashes/src/siphash24.rs @@ -14,6 +14,21 @@ use crate::HashEngine as _; #[repr(transparent)] pub struct Hash([u8; 8]); +#[cfg(feature = "schemars")] +impl schemars::JsonSchema for Hash { + fn schema_name() -> String { "Hash".to_owned() } + + fn json_schema(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + let mut schema: schemars::schema::SchemaObject = ::json_schema(gen).into(); + schema.string = Some(Box::new(schemars::schema::StringValidation { + max_length: Some(8 * 2), + min_length: Some(8 * 2), + pattern: Some("[0-9a-fA-F]+".to_owned()), + })); + schema.into() + } +} + #[cfg(not(hashes_fuzz))] fn from_engine(e: HashEngine) -> Hash { Hash::from_u64(Hash::from_engine_to_u64(e)) } From 42c7617a46b888cd6d7da5ba7ffca37c501d2ddb Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Sat, 10 Aug 2024 06:40:26 +1000 Subject: [PATCH 4/8] Fix shchemars test In #3010 we added a `length` field to the `sha256::Midstate` which broke the `schemars` test but we did not notice because of the current bug [0] in the `run_task` CI script. [0] https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools/issues/10 --- hashes/extended_tests/schemars/src/main.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hashes/extended_tests/schemars/src/main.rs b/hashes/extended_tests/schemars/src/main.rs index 2af4690ed..c9f73d40a 100644 --- a/hashes/extended_tests/schemars/src/main.rs +++ b/hashes/extended_tests/schemars/src/main.rs @@ -2,6 +2,7 @@ fn main() {} #[cfg(test)] mod tests { use bitcoin_hashes::*; + use bitcoin_hashes::sha256::Midstate; #[test] fn hash160() { @@ -117,6 +118,9 @@ mod tests { 147, 108, 71, 99, 110, 96, 125, 179, 62, 234, 221, 198, 240, 201, ]; + // The midstate of an empty hash engine tagged with "TapLeaf". + const TAP_LEAF_MIDSTATE: Midstate = Midstate::new(TEST_MIDSTATE, 64); + #[derive( Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default, Hash, schemars::JsonSchema, )] @@ -124,9 +128,7 @@ mod tests { impl sha256t::Tag for TestHashTag { fn engine() -> sha256::HashEngine { - // The TapRoot TapLeaf midstate. - let midstate = sha256::Midstate::from_byte_array(TEST_MIDSTATE); - sha256::HashEngine::from_midstate(midstate, 64) + sha256::HashEngine::from_midstate(TAP_LEAF_MIDSTATE) } } From 321d82ca5315b668578d737fa83c100201bc59f2 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Sat, 10 Aug 2024 07:22:37 +1000 Subject: [PATCH 5/8] hashes: Pin in extra_test Note: - The `extra_test.sh` script runs for all toolchains. - The `schemars` crate does not use the repo lock files. - We need to pin some deps when building the `schemars` test. Pin in the `extra_test.sh` script, and mention it in the docs so the docs don't go stale next MSRV update. This was previously unnoticed because of the `run_task` bug. ref: rust-bitcoin/rust-bitcoin-maintainer-tools#10 --- hashes/contrib/extra_tests.sh | 7 +++++++ hashes/extended_tests/schemars/README.md | 10 +++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/hashes/contrib/extra_tests.sh b/hashes/contrib/extra_tests.sh index deea345b6..4a39a1f1b 100755 --- a/hashes/contrib/extra_tests.sh +++ b/hashes/contrib/extra_tests.sh @@ -5,5 +5,12 @@ set -euox pipefail REPO_DIR=$(git rev-parse --show-toplevel) pushd "$REPO_DIR/hashes/extended_tests/schemars" > /dev/null + +# This comment mentions Rust 1.63 to assist grepping when doing MSRV update. +# +if cargo --version | grep -q '1\.63'; then + cargo update -p regex --precise 1.7.3 +fi + cargo test popd > /dev/null diff --git a/hashes/extended_tests/schemars/README.md b/hashes/extended_tests/schemars/README.md index 51dc97c42..e93cd0b0a 100644 --- a/hashes/extended_tests/schemars/README.md +++ b/hashes/extended_tests/schemars/README.md @@ -4,10 +4,6 @@ Run as usual with `cargo test`. ## Minimum Supported Rust Version (MSRV) -To run the tests with the MSRV you will need to pin `serde`: - -```bash -cargo update -p serde --precise 1.0.156 -cargo update -p regex --precise 1.7.3 -cargo update -p chrono --precise 0.4.24 -``` +To run the tests with the current MSRV you will need to pin some +dependencies. See the `hashes/contrib/extra_tests.sh` script for the +current list of pins. From f8846101ae285ea975a647a815ab5172865a20c2 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 9 Aug 2024 11:47:51 +1000 Subject: [PATCH 6/8] siphash: Make functions inherent In recent work making functions on hash types inherent it seems we missed `siphash`. Add inherent getters/setters to the `siphash::Hash` type and call through to them from the `Hash` trait impl. --- hashes/src/siphash24.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hashes/src/siphash24.rs b/hashes/src/siphash24.rs index 9895ee0f5..d0258a06c 100644 --- a/hashes/src/siphash24.rs +++ b/hashes/src/siphash24.rs @@ -226,6 +226,15 @@ impl Hash { /// Creates a hash from its (little endian) 64-bit integer representation. pub fn from_u64(hash: u64) -> Hash { Hash(hash.to_le_bytes()) } + /// Returns the underlying byte array. + pub const fn to_byte_array(self) -> [u8; 8] { self.0 } + + /// Returns a reference to the underlying byte array. + pub const fn as_byte_array(&self) -> &[u8; 8] { &self.0 } + + /// Constructs a hash from the underlying byte array. + pub const fn from_byte_array(bytes: [u8; 8]) -> Self { Self(bytes) } + fn from_slice(sl: &[u8]) -> Result { let mut ret = [0; 8]; ret.copy_from_slice(sl); @@ -241,11 +250,11 @@ impl crate::Hash for Hash { fn from_slice(sl: &[u8]) -> Result { Self::from_slice(sl) } - fn to_byte_array(self) -> Self::Bytes { self.0 } + fn to_byte_array(self) -> Self::Bytes { self.to_byte_array() } - fn as_byte_array(&self) -> &Self::Bytes { &self.0 } + fn as_byte_array(&self) -> &Self::Bytes { self.as_byte_array() } - fn from_byte_array(bytes: Self::Bytes) -> Self { Hash(bytes) } + fn from_byte_array(bytes: Self::Bytes) -> Self { Self::from_byte_array(bytes) } } impl> Index for Hash { From aed61bd2d42aee5de1909daebcd5eed105628a51 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 21 Aug 2024 15:58:29 +1000 Subject: [PATCH 7/8] Implement FromStr and serde impls for siphash The `serde` impls and `FromStr` are missing from `siphash`, add them. --- hashes/src/siphash24.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hashes/src/siphash24.rs b/hashes/src/siphash24.rs index d0258a06c..6aaed346a 100644 --- a/hashes/src/siphash24.rs +++ b/hashes/src/siphash24.rs @@ -4,8 +4,11 @@ use core::ops::Index; use core::slice::SliceIndex; +use core::str::FromStr; use core::{cmp, mem, ptr}; +use hex::FromHex; + use crate::internal_macros::arr_newtype_fmt_impl; use crate::HashEngine as _; @@ -264,7 +267,16 @@ impl> Index for Hash { fn index(&self, index: I) -> &Self::Output { &self.0[index] } } +impl FromStr for Hash { + type Err = hex::HexToArrayError; + fn from_str(s: &str) -> Result { + let bytes = <[u8; 8]>::from_hex(s)?; + Ok(Self::from_byte_array(bytes)) + } +} + arr_newtype_fmt_impl!(Hash, 8); +serde_impl!(Hash, 8); borrow_slice_impl!(Hash); /// Load an u64 using up to 7 bytes of a byte slice. From 009e74732321f9f3e6d09e158dd577a7d9ebca35 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 9 Aug 2024 12:12:42 +1000 Subject: [PATCH 8/8] CI: Put fuzz last in CRATES list There is a bug in `run_task` [0] that drastically reduces our test coverage. We can bypass it by putting `fuzz` last in the list. [0] rust-bitcoin/rust-bitcoin-maintainer-tools#10 --- contrib/crates.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/crates.sh b/contrib/crates.sh index b3bb34178..c3d605eee 100644 --- a/contrib/crates.sh +++ b/contrib/crates.sh @@ -5,4 +5,4 @@ # shellcheck disable=SC2034 # Crates in this workspace to test (note "fuzz" is only built not tested). -CRATES=("addresses" "base58" "bitcoin" "fuzz" "hashes" "internals" "io" "units") +CRATES=("addresses" "base58" "bitcoin" "hashes" "internals" "io" "units" "fuzz")