From ae93e226e359d0228fdff05559499198a38b2144 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 4 Sep 2024 15:51:32 +1000 Subject: [PATCH] Remove hashes io feature Currently we only get `std::io::Write` impls when the `bitcoin-io` dependency is used. This is overly restrictive, it would be nice to have `std::io::Write` imlps even without the `bitcoin-io` dependency. Copy the logic out of the `bitcoin_io::impl_write` macro into `hashes` but feature gate it differently. Call the new macro inside `hash_type` (and in `hmac`), remove the `impls` module, and move the tests to the integration test directory. Remove the `io` feature from `hashes`, now if users enable `std` they get `std::io::Write` impls and if they enable `bitcoin-io` they get `bitcoin_io::Write` impls as well. --- bitcoin/Cargo.toml | 2 +- hashes/Cargo.toml | 6 +- hashes/contrib/sanitizer.sh | 2 +- hashes/contrib/test_vars.sh | 4 +- hashes/embedded/Cargo.toml | 2 +- hashes/src/hmac.rs | 10 +++ hashes/src/impls.rs | 99 +++++--------------------- hashes/src/internal_macros.rs | 41 +++++++++++ hashes/src/lib.rs | 2 - hashes/tests/io.rs | 127 ++++++++++++++++++++++++++++++++++ hashes/tests/regression.rs | 2 + 11 files changed, 205 insertions(+), 92 deletions(-) create mode 100644 hashes/tests/io.rs diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index 3b9ebd23a..e2e748f06 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -27,7 +27,7 @@ arbitrary = ["dep:arbitrary", "units/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", "io"] } +hashes = { package = "bitcoin_hashes", version = "0.14.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.3.0", features = ["alloc"] } io = { package = "bitcoin-io", version = "0.1.1", default-features = false, features = ["alloc"] } diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 4d048ea84..299b675d6 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -15,11 +15,9 @@ exclude = ["tests", "contrib"] [features] default = ["std"] -std = ["alloc", "hex/std", "bitcoin-io?/std"] -alloc = ["hex/alloc"] +std = ["alloc", "bitcoin-io?/std", "hex/std"] +alloc = ["bitcoin-io?/alloc", "hex/alloc"] schemars = ["dep:schemars", "alloc"] -# If you want I/O you must enable either "std" or "io". -io = ["bitcoin-io"] # Smaller (but slower) implementation of sha256, sha512 and ripemd160 small-hash = [] diff --git a/hashes/contrib/sanitizer.sh b/hashes/contrib/sanitizer.sh index 5a1e56c2f..e3d1e8152 100755 --- a/hashes/contrib/sanitizer.sh +++ b/hashes/contrib/sanitizer.sh @@ -5,7 +5,7 @@ set -euox pipefail # Run the sanitizer with these features. -FEATURES="std io serde" +FEATURES="std bitcoin-io serde" cargo clean CC='clang -fsanitize=address -fno-omit-frame-pointer' \ diff --git a/hashes/contrib/test_vars.sh b/hashes/contrib/test_vars.sh index b74aed8e6..67bd292da 100644 --- a/hashes/contrib/test_vars.sh +++ b/hashes/contrib/test_vars.sh @@ -5,10 +5,10 @@ # shellcheck disable=SC2034 # Test all these features with "std" enabled. -FEATURES_WITH_STD="io serde small-hash schemars" +FEATURES_WITH_STD="bitcoin-io serde small-hash schemars" # Test all these features without "std" enabled. -FEATURES_WITHOUT_STD="alloc serde small-hash" +FEATURES_WITHOUT_STD="alloc bitcoin-io serde small-hash" # Run these examples. EXAMPLES="" diff --git a/hashes/embedded/Cargo.toml b/hashes/embedded/Cargo.toml index 0aa39f0ae..3cebf9643 100644 --- a/hashes/embedded/Cargo.toml +++ b/hashes/embedded/Cargo.toml @@ -18,7 +18,7 @@ cortex-m-rt = "0.6.10" cortex-m-semihosting = "0.3.3" panic-halt = "0.2.0" alloc-cortex-m = { version = "0.4.1", optional = true } -bitcoin_hashes = { path="../", default-features = false, features = ["io"] } +bitcoin_hashes = { path="../", default-features = false, features = ["bitcoin-io"] } bitcoin-io = { path = "../../io", default_features = false } [[bin]] diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index f11b9f9ba..73ff4b13a 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -162,6 +162,16 @@ impl<'de, T: GeneralHash + Deserialize<'de>> Deserialize<'de> for Hmac { } } +crate::internal_macros::impl_io_write!( + HmacEngine, + |us: &mut HmacEngine, buf| { + us.input(buf); + Ok(buf.len()) + }, + |_us| { Ok(()) }, + T: crate::GeneralHash +); + #[cfg(test)] mod tests { #[test] diff --git a/hashes/src/impls.rs b/hashes/src/impls.rs index 2e69309c8..c352aff22 100644 --- a/hashes/src/impls.rs +++ b/hashes/src/impls.rs @@ -4,92 +4,15 @@ //! //! Implementations of traits defined in `std` / `io` and not in `core`. -use bitcoin_io::impl_write; - -use crate::{hash160, hmac, ripemd160, sha1, sha256, sha256d, sha512, siphash24, HashEngine}; - -impl_write!( - hash160::HashEngine, - |us: &mut hash160::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - sha1::HashEngine, - |us: &mut sha1::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - sha256::HashEngine, - |us: &mut sha256::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - sha256d::HashEngine, - |us: &mut sha256d::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - sha512::HashEngine, - |us: &mut sha512::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - ripemd160::HashEngine, - |us: &mut ripemd160::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - siphash24::HashEngine, - |us: &mut siphash24::HashEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) } -); - -impl_write!( - hmac::HmacEngine, - |us: &mut hmac::HmacEngine, buf| { - us.input(buf); - Ok(buf.len()) - }, - |_us| { Ok(()) }, - T: crate::GeneralHash -); - -#[cfg(all(test, feature = "alloc"))] // right now every test here depends on `alloc` +#[cfg(test)] +#[cfg(feature = "alloc")] mod tests { use alloc::format; use bitcoin_io::Write; - use crate::{ - hash160, hmac, ripemd160, sha1, sha256, sha256d, sha512, siphash24, GeneralHash as _, - }; + use crate::GeneralHash as _; + use crate::{hash160, hmac, ripemd160, sha1, sha256, sha256d, sha384, sha512, sha512_256, siphash24}; macro_rules! write_test { ($mod:ident, $exp_empty:expr, $exp_256:expr, $exp_64k:expr,) => { @@ -131,6 +54,13 @@ mod tests { "0050d4148ad7a0437ca0643fad5bf4614cd95d9ba21fde52370b37dcc3f03307", ); + write_test!( + sha384, + "38b060a751ac96384cd9327eb1b1e36a21fdb71114be07434c0cc7bf63f6e1da274edebfe76f65fbd51ad2f14898b95b", + "82135637ef6d6dd31a20e2bc9998681a3eecaf8f8c76d45e545214de38439d9a533848ec75f53e4b1a8805709c5124d0", + "fb7511d9a98c5686f9c2f55e242397815c9229d8759451e1710b8da6861e08d52f0357176f4b74f8cad9e23ab65411c7", + ); + write_test!( sha512, "cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce\ @@ -141,6 +71,13 @@ mod tests { 4761df8d04ed04bb734ba48dd2106bb9ea54524f1394cdd18e6da3166e71c3ee", ); + write_test!( + sha512_256, + "c672b8d1ef56ed28ab87c3622c5114069bdd3ad7b8f9737498d0c01ecef0967a", + "8d4bb96e7956cf5f08bf5c45f7982630c46b0b022f25cbaf722ae97c06a6e7a2", + "3367646f3e264653f7dd664ac2cb6d3b96329e86ffb7a29a1082e2a4ddc9ee7a", + ); + write_test!( ripemd160, "9c1185a5c5e9fc54612808977ee8f548b2258d31", diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index f07ff176a..ecb55d579 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -246,6 +246,47 @@ macro_rules! hash_type_no_default { } $crate::internal_macros::hash_trait_impls!($bits, $reverse); + + $crate::internal_macros::impl_io_write!( + HashEngine, + |us: &mut HashEngine, buf| { + crate::HashEngine::input(us, buf); + Ok(buf.len()) + }, + |_us| { Ok(()) } + ); }; } pub(crate) use hash_type_no_default; + +// We do not use the `bitcoin_io::impl_write` macro because we don't have an unconditional +// dependency on `bitcoin-io` and we want to implement `std:io::Write` even when we don't depend on +// `bitcoin-io`. +macro_rules! impl_io_write { + ($ty: ty, $write_fn: expr, $flush_fn: expr $(, $bounded_ty: ident : $bounds: path),*) => { + #[cfg(feature = "bitcoin-io")] + impl<$($bounded_ty: $bounds),*> bitcoin_io::Write for $ty { + #[inline] + fn write(&mut self, buf: &[u8]) -> bitcoin_io::Result { + $write_fn(self, buf) + } + #[inline] + fn flush(&mut self) -> bitcoin_io::Result<()> { + $flush_fn(self) + } + } + + #[cfg(feature = "std")] + impl<$($bounded_ty: $bounds),*> std::io::Write for $ty { + #[inline] + fn write(&mut self, buf: &[u8]) -> std::io::Result { + $write_fn(self, buf) + } + #[inline] + fn flush(&mut self) -> std::io::Result<()> { + $flush_fn(self) + } + } + } +} +pub(crate) use impl_io_write; diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 2c2371bab..8c7e38c19 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -115,8 +115,6 @@ pub mod cmp; pub mod hash160; pub mod hkdf; pub mod hmac; -#[cfg(feature = "bitcoin-io")] -mod impls; pub mod ripemd160; pub mod sha1; pub mod sha256; diff --git a/hashes/tests/io.rs b/hashes/tests/io.rs new file mode 100644 index 000000000..66d21b030 --- /dev/null +++ b/hashes/tests/io.rs @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Test the `bitcoin-io` implementations. + +#![cfg(feature = "bitcoin-io")] + +use bitcoin_io::Write; + +use bitcoin_hashes::{hash160, hmac, ripemd160, sha1, sha256, sha256d, sha384, sha512, sha512_256, siphash24, GeneralHash as _}; + +macro_rules! write_test { + ($mod:ident, $exp_empty:expr, $exp_256:expr, $exp_64k:expr,) => { + #[test] + fn $mod() { + let mut engine = $mod::Hash::engine(); + engine.write_all(&[]).unwrap(); + assert_eq!(format!("{}", $mod::Hash::from_engine(engine)), $exp_empty); + + let mut engine = $mod::Hash::engine(); + engine.write_all(&[1; 256]).unwrap(); + assert_eq!(format!("{}", $mod::Hash::from_engine(engine)), $exp_256); + + let mut engine = $mod::Hash::engine(); + engine.write_all(&[99; 64000]).unwrap(); + assert_eq!(format!("{}", $mod::Hash::from_engine(engine)), $exp_64k); + } + }; +} + +write_test!( + sha1, + "da39a3ee5e6b4b0d3255bfef95601890afd80709", + "ac458b067c6b021c7e9358229b636e9d1e4cb154", + "e4b66838f9f7b6f91e5be32a02ae78094df402e7", +); + +write_test!( + sha256, + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "2661920f2409dd6c8adeb0c44972959f232b6429afa913845d0fd95e7e768234", + "5c5e904f5d4fd587c7a906bf846e08a927286f388c54c39213a4884695271bbc", +); + +write_test!( + sha256d, + "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", + "374000d830c75d10d9417e493a7652920f30efbd300e3fb092f24c28c20baf64", + "0050d4148ad7a0437ca0643fad5bf4614cd95d9ba21fde52370b37dcc3f03307", +); + +write_test!( + sha384, + "38b060a751ac96384cd9327eb1b1e36a21fdb71114be07434c0cc7bf63f6e1da274edebfe76f65fbd51ad2f14898b95b", + "82135637ef6d6dd31a20e2bc9998681a3eecaf8f8c76d45e545214de38439d9a533848ec75f53e4b1a8805709c5124d0", + "fb7511d9a98c5686f9c2f55e242397815c9229d8759451e1710b8da6861e08d52f0357176f4b74f8cad9e23ab65411c7", +); + +write_test!( + sha512, + "cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce\ + 47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e", + "57ecf739d3a7ca647639adae80a05f4f361304bfcbfa1ceba93296b096e74287\ + 45fc10c142cecdd3bb587a3dba598c072f6f78b31cc0a06a3da0105ee51f75d6", + "dd28f78c53f3bc9bd0c2dca9642a1ad402a70412f985c1f6e54fadb98ce9c458\ + 4761df8d04ed04bb734ba48dd2106bb9ea54524f1394cdd18e6da3166e71c3ee", +); + +write_test!( + sha512_256, + "c672b8d1ef56ed28ab87c3622c5114069bdd3ad7b8f9737498d0c01ecef0967a", + "8d4bb96e7956cf5f08bf5c45f7982630c46b0b022f25cbaf722ae97c06a6e7a2", + "3367646f3e264653f7dd664ac2cb6d3b96329e86ffb7a29a1082e2a4ddc9ee7a", +); + +write_test!( + ripemd160, + "9c1185a5c5e9fc54612808977ee8f548b2258d31", + "e571a1ca5b780aa52bafdb9ec852544ffca418ba", + "ddd2ecce739e823629c7d46ab18918e9c4a51c75", +); + +write_test!( + hash160, + "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb", + "671356a1a874695ad3bc20cae440f4360835bd5a", + "a9608c952c8dbcc20c53803d2ca5ad31d64d9313", +); + +#[test] +fn hmac() { + let mut engine = hmac::HmacEngine::::new(&[0xde, 0xad, 0xbe, 0xef]); + engine.write_all(&[]).unwrap(); + assert_eq!( + format!("{}", hmac::Hmac::from_engine(engine)), + "bf5515149cf797955c4d3194cca42472883281951697c8375d9d9b107f384225" + ); + + let mut engine = hmac::HmacEngine::::new(&[0xde, 0xad, 0xbe, 0xef]); + engine.write_all(&[1; 256]).unwrap(); + assert_eq!( + format!("{}", hmac::Hmac::from_engine(engine)), + "59c9aca10c81c73cb4c196d94db741b6bf2050e0153d5a45f2526bff34675ac5" + ); + + let mut engine = hmac::HmacEngine::::new(&[0xde, 0xad, 0xbe, 0xef]); + engine.write_all(&[99; 64000]).unwrap(); + assert_eq!( + format!("{}", hmac::Hmac::from_engine(engine)), + "30df499717415a395379a1eaabe50038036e4abb5afc94aa55c952f4aa57be08" + ); +} + +#[test] +fn siphash24() { + let mut engine = siphash24::HashEngine::with_keys(0, 0); + engine.write_all(&[]).unwrap(); + assert_eq!(format!("{}", siphash24::Hash::from_engine(engine)), "d70077739d4b921e"); + + let mut engine = siphash24::HashEngine::with_keys(0, 0); + engine.write_all(&[1; 256]).unwrap(); + assert_eq!(format!("{}", siphash24::Hash::from_engine(engine)), "3a3ccefde9b5b1e3"); + + let mut engine = siphash24::HashEngine::with_keys(0, 0); + engine.write_all(&[99; 64000]).unwrap(); + assert_eq!(format!("{}", siphash24::Hash::from_engine(engine)), "ce456e4e4ecbc5bf"); +} + diff --git a/hashes/tests/regression.rs b/hashes/tests/regression.rs index e92364631..360bfd7f3 100644 --- a/hashes/tests/regression.rs +++ b/hashes/tests/regression.rs @@ -1,4 +1,6 @@ //! Regression tests for each hash type. +//! +//! Note that if `bitcoin-io` is enabled then we get more regression-like testing from `./io.rs`. use bitcoin_hashes::{ hash160, ripemd160, sha1, sha256, sha256d, sha256t, sha384, sha512, sha512_256, siphash24,