Currently we are defining the `sha512::Hash` type manually instead of
using the `hash_type` macro.
The generated code using the macro is identical to that without
it (although I didn't actually look at the generated code :)
When enabled this feature swaps the hash implementation of sha512,
sha256 and ripemd160 for a smaller (but also slower) one.
On embedded processors (Cortex-M4) it can lead to up to a 52% size
reduction, from around 37KiB for just the `process_block` methods of the
three hash functions to 17.8KiB.
Recently we made the hash engine fields pub crate so that `sha512_256`
could construct a hash engine with different constants. We can make the
code slightly cleaner by adding a pub crate constructor and making the
fields private again.
Idea from Kixunil:
https://github.com/rust-bitcoin/rust-bitcoin/pull/1413#pullrequestreview-1197207593
We are trying to make error types stable on the way to v1.0
The current `hashes::Error` is a "general" enum error type with a single
variant, better to use a struct and make the error usecase specific.
Improve the `hashes::Error` by doing:
- Make it a struct
- Rename to `FromSliceError`
- Move it to the crate root (remove `error` module)
Includes usage in `bitcoin`.
Whether or not every file needs an explicit license comment is out of
scope for this patch; in the `bitcoin` crate we use SPDX identifiers
because they are a single line with no loss of "benefit" over any longer
form.
Use SPDX identifiers in `hashes`. Drop the mention of re-licensing code
from Apache to CC0-1 (because the original code was written by Andrew
as well as the copied code then if the argument ever comes up it can be
easily countered).
The `hashes` module contains a bunch of arrays, mostly formatted with 8
hex bytes on a line; add `rustfmt::skip` to keep the formatting of
arrays as is.
Remove the exclude for the `hashes` crate. Do not run the formatter,
that will be done as a separate patch to aid review.
Currently we have an associated type on hash types `Inner` with
accompanying methods `into_inner`, `from_inner`, `as_inner`. Also, we
provide a way to create new wrapped hash types. The use of 'inner'
becomes ambiguous with the addition of wrapped types because the inner
could be the inner hash type or the `Inner` byte array of the inner
wrapped hash type.
In an effort to make the API more clear and uniform do the following:
- Rename `Inner` -> `Bytes`
- Rename `*_inner` -> `*_byte_array`
- Rename the inner hash to/from methods to `*_raw_hash`
Correct method prefix `into_` -> `to_` because theses methods convert
owned `Copy` types.
Add the trait Bound `Copy` to the `Bytes` type because we rely on this
trait bound for the conversion methods to be correctly named according
to convention.
Because of the dependency hole created by `secp256k1` this patch changes
the secp dependency to a git tag dependency that includes changes to the
hashes calls required so that we can get green lights on CI in this
repo.
Currently we have a few things mixed up in the feature gating of
`hashes`.
Observe that:
- `io::Write` is not related to allocation.
- "std" should be able to enable "alloc".
Improve feature gating by doing:
- Enable "alloc" from "std" and simplify `cfg` codebase wide.
- Enable "internals/alloc" from "alloc".
- Fix feature gating to use the minimal requirement i.e., "alloc".
Remove `FromHex` from hash and script types
- Remove the `FromHex` implementation from hash types and `ScriptBuf`
- Remove the `FromStr` implementation from `ScriptBuf` because it does not
roundtrip with `Display`.
- Implement a method `from_hex` on `ScriptBuf`.
- Implement `FromStr` on hash types using a fixed size array.
This leaves `FromHex` implementations only on `Vec` and fixed size arrays.
It is easier to maintain code if macros use the fully qualified path to
types and functions instead of relying on code that uses the macro to
import said type or function.
The `ToHex` trait was replaced by either simple `Display`/`LowerHex`
where appropriate or `DisplayHex` from `bitcoin_internals` which is
faster.
This change replaces the usages and removes the trait.
411174c391 Add fuzz target for sha512_256 (Calvin Kim)
31fc1f8638 Add support for sha512/256 (Calvin Kim)
15b5af1117 Export sha512::HashEngine fields/function within the crate (Calvin Kim)
Pull request description:
Adds a new file named `sha512_256.rs` that implements the `sha512/256` hash. This was needed as a part of https://github.com/rust-bitcoin/rust-bitcoin/discussions/1318 to drop the `sha2` dependency.
All the actual hashing code is exactly the same as `sha512.rs`, minus the initial constants and the use of `hash_type!` macro. Some unit tests were added from wikipedia (for the "" input) and the rest were from the Go standard library's tests for sha512_256.
Benchmarks on my Ryzen 3600 machine show that it is faster than sha256.
```
test sha256::benches::sha256_10 ... bench: 37 ns/iter (+/- 0) = 270 MB/s
test sha256::benches::sha256_1k ... bench: 3,338 ns/iter (+/- 24) = 306 MB/s
test sha256::benches::sha256_64k ... bench: 213,605 ns/iter (+/- 1,806) = 306 MB/s
test sha512_256::benches::sha512_256_10 ... bench: 27 ns/iter (+/- 1) = 370 MB/s
test sha512_256::benches::sha512_256_1k ... bench: 2,196 ns/iter (+/- 12) = 466 MB/s
test sha512_256::benches::sha512_256_64k ... bench: 140,552 ns/iter (+/- 777) = 466 MB/s
```
One caveat is that I could not get hongfuzz to build locally so I couldn't test the fuzz on my machine. I ended up only testing through the CI for the fuzz tests.
I thought adding a completely separate file was the easiest and the most straightforward way of implementing it. I'm very much open to changing the implementation if you guys don't think this is the right direction.
ACKs for top commit:
sanket1729:
ACK 411174c391. Reviwed range diff from 43feb9ea7b282d9119708a27fa7a1c7412d1386a that I had ACked
apoelstra:
ACK 411174c391
Tree-SHA512: 98298a7c177cbb616bfbc02cec5c5860f10204df8275cc9f1e4ea07333b901095e574fbc3fe0a03375e0d321a1579e2c2023a5c14addd863e10cc927f155710c
Recently clippy was updated and now new warnings are generated for the
`hashes` crate.
Clippy emits 3 warnings of form:
warning: this expression borrows a value the compiler would automatically borrow
As suggested, remove the explicit borrow.
We would like to bring the `bitcoin_hashes` crate into the
`rust-bitcoin` repository.
Import `bitcoin_hashes` into `rust-bitocin/hashes`, doing so looses all
the commit history from the original crate but if we archive the
original repository then the history will be preserved. We maintain the
same version number obviously and in the changelog we note the change of
repository.
Commit hash that was tip of `bitcoin_hashes` at time of import:
commit 54c16249e06cc6b7870c7fc07d90f489d82647c7
Includes making `embedded` and `fuzzing` per-crate i.e., move them into
`bitcoin` as hashes includes these also.
NOTE: Does _not_ enable fuzzing for `hashes` in CI.
Notes on CI:
Attempts to merge in the github actions from the hashes crate however reduces
coverage by not running hashes tests for beta toolchain. Some additional
work could be done to improve the CI to increase efficiency without
reducing coverage. Leaving for another day.
Tobin C. Harding
Alekos Filini
Andrew Poelstra
Martin Habovstiak
sanket1729
Calvin Kim