Some of our CI shell scripts are meant only to be sourced and not
run directly however they include an initial shebang line, implying that
they should be run.
Remove the shebang line from `crates.sh` and the various `test_vars.sh`
scripts. Add a `shellcheck` directive to inhibit the no-shebang warning.
Fix: #2764
BIP324's peer to peer encryption protocol requires an HMAC-based extract
and expand key derivation function (HKDF). HKDFs were not part of many
bitcoin protocols before BIP324, but the hope is that the encrypted
protocol becomes the dominant standard justifying this implementation.
30a482504b bump nightly-version (Andrew Poelstra)
5ad7c245e3 cargo: whitelist all cfgs used in this repo (Andrew Poelstra)
814786b0a6 crypto: enable and fix accidentally disabled unit test (Andrew Poelstra)
Pull request description:
https://github.com/rust-lang/rust/issues/124800 has been fixed and we can update our nightly version by whitelisting all cfgs that are used.
There was one place where we had an old `cfg(feature = "no-std")` despite having removed the feature. By removing that cfg check we re-enabled a previously disabled test.
ACKs for top commit:
tcharding:
ACK 30a482504b
Tree-SHA512: d25bed819091db74b9d47cb2c23caa3ceb0d7be323b37831326e2ec1608cb1577d41aad2e1cdf59d66df69397537bc3e17a3c2872935d5a4f46f4dc55b5e613c
4446be6fc8 hashes: Add regression tests (Tobin C. Harding)
Pull request description:
We have regression tests spread out throughout the `hashes` module but they are not labelled as such. To give us more confidence and help debug when patching the `hashes` crate we can add a bunch of regression tests in a single place.
Add a module that does a single regression test for each type, simply hash some arbitrary data and check the hex display against a hard coded hex string.
ACKs for top commit:
apoelstra:
ACK 4446be6fc8 nice :)
Tree-SHA512: db643fdf799d23735934ad966ee66b40b3adb72db8f777112005f94d61ab4e382399a66e48d9fc9fdb0cd0abe17b3d4087fe6e6c494836d0c6fc713f4efc6413
We have regression tests spread out throughout the `hashes` module but
they are not labelled as such. To give us more confidence and help
debug when patching the `hashes` crate we can add a bunch of regression
tests in a single place.
Add a module that does a single regression test for each type, simply
hash some arbitrary data and check the hex display against a hard coded
hex string.
021bea89bb ci: shellcheck checks (Jose Storopoli)
Pull request description:
Closes#2739.
I am proposing that we use this GitHub Shellcheck action:
[`ludeeus/action-shellcheck`](https://github.com/ludeeus/action-shellcheck)
since it has most stars (and eyes on it).
I also did all fixes that I could find with
```bash
shellcheck **/*.sh
```
If I've missed any please let me know.
ACKs for top commit:
apoelstra:
ACK 021bea89bb
tcharding:
ACK 021bea89bb
Tree-SHA512: 67e37da9ae3ea0c5551af57b928016a2d9e76761af5558b3057ac47e773189629dd20eea9e659b4323c8568fb48dcdbe9ebd5c730f2c6266fb0db52886c9835f
6def5bc974 CI: Use run_task from maintainer tools (Tobin C. Harding)
c5af52847b CI: Add docs and document start (Tobin C. Harding)
62ba10503a CI: Use correct spacing (Tobin C. Harding)
0c0e88165e CI: Add README file (Tobin C. Harding)
44cb2255d3 CI: Add sanitizer script (Tobin C. Harding)
3407257936 CI: Add WASM script (Tobin C. Harding)
cc14edf63f CI: Run the schemars job directly using cargo (Tobin C. Harding)
1fb12e1917 CI: Remove recent from schemars job (Tobin C. Harding)
8d7117bb0e CI: Use original name (Tobin C. Harding)
Pull request description:
First we do some clean up, then we pull the stuff that is specific to this repo out into separate tests, then as the last patch we switch over to use the new script for `rust-bitcoin-maintainer-tools` that was introduced in https://github.com/rust-bitcoin/rust-bitcoin-maintainer-tools/pull/4.
ACKs for top commit:
apoelstra:
ACK 6def5bc974
Tree-SHA512: 70105d455294d49deb43461958435096d074d19ea8d9adc7036e15d74dfdab466f04b1d9e934574077ca0c32a6fe77d0e5aa11068d8c4d51acef634591227d33
As we did for the wasm job.
In preparation for using the `run_task` script from maintainer tools we
want to have all the things that are particular to `rust-bitcoin` out of
the current `run_task` script.
The address/memory sanitizer test is specific to `hashes`. Add a script
in `hashes/contrib` and call it from the ASAN job.
No test coverage change.
In preparation for using the `run_task` script from maintainer tools we
want to have all the things that are particular to `rust-bitcoin` out of
the current `run_task` script.
The wasm test is specific to `hashes`. Add a script in `hashes/contrib`
and call it from the wasm job.
No test coverage change.
7685461e62 Document the sha256t_hash_newtype direction (Tobin C. Harding)
30e91cc766 Default to forward for tagged hashes (Tobin C. Harding)
5ecc69cd28 Add forward/backward unit test (Tobin C. Harding)
9aee65d1ba Refactor tagged hash tests (Tobin C. Harding)
216422dffc Remove schemars impl for test type (Tobin C. Harding)
Pull request description:
First three patches are preparation, improvements to the units tests in `sha256t`.
From the final patch:
Displaying backward is an anomaly of Bitcoin Core's early days and the
double SHA256 hash type. We should not let this unfortunate beast leak
out into other places.
Default to displaying forward when creating a new tagged hash and remove
all the explicit attributes from `bitcoin` that just clutter the code.
This is an API break and may quietly break some users downstream - eventually we should stop doing that sort of thing.
ACKs for top commit:
apoelstra:
ACK 7685461e62
Tree-SHA512: cb8a41b207aa68ecf63cb7af7f39f7d7c8a3a27f38595867949b288a81a20bff0c17aa4c17bb099e2ecf85194d83bad23c9c9792f511b6c4cd625ff27c1affaa
Since the default display direction is now forward, use
`#[hash_newtype(backward)]`
in the rustdocs on the macro. Also add an example usage to the changelog
in case someone downstream is relying on the old default behaviour of
displaying backwards (unlikely).
Currently we require indexing trait bounds as well as `Borrow` on the
`Hash` trait. We also already implement `AsRef`.
It was observed that `Borrow<[u8]>` does not best describe what we want
from the `Hash` trait implementor but rather `AsRef<[u8]>` does.
Remove all the inexing trait bounds. Remove the `borrow::Borrow<[u8]>`
trait bound. Add a `convert::AsRef<[u8]>` trait bound.
This leaves the `Borrow<[u8]>` implementation for hashes created with
`hash_newtype`, I'm not sure if this should be removed or not.
71bb86232b hashes: Do not import str (Tobin C. Harding)
Pull request description:
Depending on things being in scope for macros to use is bad form, using the fully qualified path is the correct way.
Do not import `str` instead use the fully qualified path to the `core` re-export.
Use fully qualified path instead.
ACKs for top commit:
apoelstra:
ACK 71bb86232b trivial rebase
sanket1729:
ACK 71bb86232b
Tree-SHA512: 401520a5876b83ad4053bbe9b1e8cd9ff2e723cf86f95e47891a6411ad5e9af4f904e19ccaaab80d342dfe4745753c24af168dcbc8170fb6b39da08e577d30ae
26b9782d8b CI: Re-write run_task.sh (Tobin C. Harding)
Pull request description:
Recently we re-wrote CI to increase VM level parallelism, in hindsite this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling to the workflow. Also don't bother testing with beta toolchain.
### Note on review
The diff is hard to read for `rust.yml`, I tried splitting out a bunch of separate patches but it resulted in the same thing (because there are so many identical lines in the yaml file). I suggest just looking at the yaml file and not the diff.
ACKs for top commit:
apoelstra:
ACK 26b9782d8b
sanket1729:
ACK 26b9782d8b.
Tree-SHA512: 1b0a0bab5cf729c5890f7150953499b42aebd3b1c31a1b0d3dfa5b5e78fda11e17a62a2df6b610ab4a950d5709f3af6fff1ae64d9e67379338903497ab77ae0e
Recently we re-wrote CI to increase VM level parallelism, in hindsite
this has proved to be not that great because:
- It resulted in approx 180 jobs
- We are on free tier so only get 20 jobs (VMs) at a time so its slow to run
- The UI is annoying to dig through the long job list to find failures
Have another go at organising the jobs with the main aim of shortening
total run time and making it easier to quickly see fails.
Re-write the `run_task.sh` script, notable moving manifest handling
to the workflow. Also don't bother testing with beta toolchain.
WASM Note
Removes the `cdylib` and `rlib` from the manifest patching during wasm
build - I do not know the following:
- Why this breaks on this PR but not on other PRs
- Why I can't get wasm test to run locally on master but PRs are passing
- What the `cdylib` and `rlib` were meant to be doing
This is the docs from: https://doc.rust-lang.org/reference/linkage.html
* --crate-type=cdylib, #![crate_type = "cdylib"] - A dynamic system
library will be produced. This is used when compiling a dynamic library
to be loaded from another language. This output type will create *.so
files on Linux, *.dylib files on macOS, and *.dll files on Windows.
* --crate-type=rlib, #![crate_type = "rlib"] - A "Rust library" file
will be produced. This is used as an intermediate artifact and can be
thought of as a "static Rust library". These rlib files, unlike
staticlib files, are interpreted by the compiler in future linkage. This
essentially means that rustc will look for metadata in rlib files like
it looks for metadata in dynamic libraries. This form of output is used
to produce statically linked executables as well as staticlib outputs.
Displaying backward is an anomaly of Bitcoin Core's early days and the
double SHA256 hash type. We should not let this unfortunate beast leak
out into other places.
Default to displaying forward when creating a new tagged hash and remove
all the explicit attributes from `bitcoin` that just clutter the code.
In the tagged hash unit tests we are testing two separate things in a
single test. To improve maintainability separate the test into two.
Refactor only, no test coverage change.
We do not test the schemars stuff in `hashes`, instead we do it in a
separate crate `extended_tests/schemars`. There is therefore no reason
to implement `schemars::JsonSchema` for the `TestHashTag`.
Depending on things being in scope for macros to use is bad form,
using the fully qualified path is the correct way.
Do not import `str` instead use the fully qualified path to the `core`
re-export.
Use fully qualified instead.
This lint triggers when parsing a reference to a large struct as a
generic argument, which is wrong.
Allow it crate wide because [subjectively] this lint never warns for
anything useful.
In preparation for release add a changlelog entry and bump the version.
I'm not 100% sure that this release is API breaking, dependencies
definitely changed. The rest might be only additives but I didn't bother
looking exactly because I think its better to bump the minor version and
err on the side of caution.
Note the hashes 0.13.0 dependency stays in the dependency graph because
of secp, we can update secp after releasing `hashes` then update the
secp dependency in `rust-bitcoin` thereby removing the `hashes v0.13.0`
dependency - phew.
56132f59d5 Remove the `:#` formatting for `hex_fmt_impl` macro (448 OG)
Pull request description:
This commit attempts to solve #2505 by ensuring that formatting is not forced using the `:#` in the hex macro code generating in macro rule `hex_fmt_impl` in the hashes/utils.rs file.
The write! macro forces all formatting to add the prefix `0x` by adding an alternate by (#) default
```rust
impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> {
#[inline]
fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result {
write!(f, "{:#}", self) // <-- This is where the formatting is being forced.
}
}
```
By removing this formatting, the `:#` must be specified by the user in order for a prefix to be added.
```rust
let outpoint = bitcoin::OutPoint::default();
println!("{:?}", &outpoint);
println!("{:#?}", &outpoint);
println!("{:#}", &outpoint);
println!("{:x}", &outpoint.txid);
// `{:#}` must be specified to pretty print with a prefix
println!("{:#}", &outpoint.txid);
dbg!(&outpoint);
dbg!(&outpoint.txid);
```
The PR also adds testcase for this when running `cargo test` .
ACKs for top commit:
tcharding:
ACK 56132f59d5
apoelstra:
ACK 56132f59d5
Tree-SHA512: 9e4fc9f30ab0b3cf2651d3c09f7f01d8245ac8ea7ae3a82bb4efd19f25c77662bf279020a31fa61b37587cc0c74284696c56045c59f1ba63b2dd42a210d98ebc
4e3bb7350a Add support for SHA-384 (Matt Corallo)
Pull request description:
Based on #2473 as we need support for 48-byte arrays <-> hex conversions.
Closes#2483
ACKs for top commit:
Kixunil:
ACK 4e3bb7350a
sanket1729:
ACK 4e3bb7350a
Tree-SHA512: e78d97f80ab8afda8a3ea240023338f17f7e95604a879b38fc9bde057fbb45b74b1f3fb3bd2b17af89682b79dda42bf114989e7c63066b3029451ef07894e82f
f8de7954b2 Remove unused pow::TryFromError type (Tobin C. Harding)
43c5eb765c Fix witness_version leaf error type (Tobin C. Harding)
2af764e859 hashes: Fix leaf error type (Tobin C. Harding)
Pull request description:
In light of recent discussion go over the codebase and look for some places that the leaf errors are wrong. Does not do the whole code base, excludes `p2p` and a couple of other places.
ACKs for top commit:
apoelstra:
ACK f8de7954b2
Kixunil:
ACK f8de7954b2
Tree-SHA512: 2905878363869ee205cce49c58c060c712c9b7b55965ee60bb856128842968a4be86c93a194ffffdb35e215b2bea8ad33b04ee47e8e17cc784b0641ea48518e5
This fixes the issue where pretty debug like `dbg` or `{:#}` introduce the use of
`0x` prefix to hex encoded transaction ID.
The transaction id is being forced to pretty print inside the `hex_fmt_impl` macro
using `{:#}` in the line `write!(f, "{:#}", self)` debug formatter.
Resolves: #2505
Currently we have two functions for displaying forwards and backwards and
we also have `fmt_hex_exact`. I do not know why we added the functions.
In the latest version of hex we do not have the ability to construct a
`DisplayArray` type so we have to use `fmt_hex_exact`.
Make the change now, separate from the `hex` upgrade, to assist review.
Different hashes output to hex strings differently depending on whether
they display backward or not but we are not currently testing that our
parsing and formatting impls both correctly handle backwards/forwards.
Add unit tests to roundtrip through a hex string, do so for one forwards
printing hash (sha256), on backwards printing hash (sha256d), and also
test that the `hash_newtype!` macro correctly passes on display backward.
We attempted to release with the current 0.1.0 version forgetting that
we had previously released an empty crate with that version to reserve
the name on crates.io.
Bump the version to 0.1.1 and release the actual code.
Use `bash` instead of `sh` to run shell scripts.
We would like to support Nix users who do not typically have any shell
other than `sh` at a known path, therefore use `/usr/bin/env bash`.
Make the trait level attributes uniform across all released crates in
the repo. Excludes things that are obviously not needed, eg, bench stuff
if there is not bench code.
- Remove `uninhabited_references` - this is allow by default now.
- Remove `unconditional_recursion` and mark the single false positive we
have with an `allow`.
Note, this does not add `missing_docs` to the `io` crate. There is an
open PR at the moment to add that along with the required docs.
94a6caf204 hashes: Add Hash::from_byte_chunks to construct hashes from iterators (Steven Roose)
Pull request description:
Is there any interest in a method like this? I'm not necessarily a fan myself, was just in a situation where I was doing this a lot and it would save the me the "engine, from_engine dance" each time.
ACKs for top commit:
Kixunil:
ACK 94a6caf204
apoelstra:
ACK 94a6caf204
Tree-SHA512: 96e74366dbf64d50fb6642024d18eeeee5b78154fa152c845a1073c904599af69328a141f0a7e2bd1a7b43a091c886da57dfe6e87bab5a5e01b0f167019caae0
Its not immediately obvious why we nest the whole `io` code in an `io`
submodule within `lib.rs`. As far as I can tell we can inline it and
re-export from `rust-bitcoin` same as we do for our other dependencies.
This change would effect other users of the crate but since the `io`
crate is unreleased this effects no-one except us.
Rust version 1.56.0 introduced edition 2021. Shortly afterwards, on
October 21 2021 Rust version 1.56.1 was released.
Debian stable is currently shipping `rustc 1.63.0`.
Our stated MSRV policy is: In Debian stable and at least 2 years old.
Therefore our MSRV policy is met by Rust version 1.56.1 and we can strat
to bump our MSRV org wide.
Start by bumping the `rust-bitcoin` and `hashes` MSRV to Rust 1.56.1,
includes:
- Update docs.
- Update CI and remove pinning.
- Update the build files and remove now stale cfg attributes rust_v_1_x
for values less than the new MSRV.
- Use new `IntoIterator` for arrays so we no longer need to allocate a
vector to iterate.
Links:
- https://blog.rust-lang.org/2021/11/01/Rust-1.56.1.html
- https://blog.rust-lang.org/2021/10/21/Rust-1.56.0.html
- https://packages.debian.org/stable/rust/rustc
We are trying to get rid of the `serde_derive` dependency from our
dependency graph.
Stop using default features for the `schemars` dependency which includes
`schemars_derive` which depends on `serder_derive`.
Manually implement `schemars::JsonSchema` instead of deriving it.
With the new `bitcoin_io` library, implementing `io::Write`
manually is somewhat tricky - for `std` users we really want to
provide an `std::io::Write` implementation, however for `no-std`
users we want to implement against our internal trait.
Sadly we cannot provide a blanket implementation of
`std::io::Write` for all types whcih implement our `io::Write`
trait as its an out-of-crate impl.
Instead, we provide a macro which will either implement
`std::io::Write` or our `io::Write` depending on the feature flags
set on `bitcoin_io`.
In order to support standard (de)serialization of structs, the
`rust-bitcoin` ecosystem uses the standard `std::io::{Read,Write}`
traits. This works great for environments with `std`, however sadly
the `std::io` module has not yet been added to the `core` crate.
Thus, in `no-std`, the `rust-bitcoin` ecosystem has historically
used the `core2` crate to provide copies of the `std::io` module
without any major dependencies. Sadly, its one dependency,
`memchr`, recently broke our MSRV.
Worse, because we didn't want to take on any excess dependencies
for `std` builds, `rust-bitcoin` has had to have
mutually-exclusive `std` and `no-std` builds. This breaks general
assumptions about how features work in Rust, causing substantial
pain for applications far downstream of `rust-bitcoin` crates.
Here, we add a new `bitcoin_io` crate, making it an unconditional
dependency and using its `io` module in the in-repository crates
in place of `std::io` and `core2::io`. As it is not substantial
additional code, the `hashes` io implementations are no longer
feature-gated.
This doesn't actually accomplish anything on its own, only adding
the new crate which still depends on `core2`.
71454d438a Use hash_type macro for sha512::Hash (Tobin C. Harding)
d51c1857fd sha512: Move from_engine functions (Tobin C. Harding)
Pull request description:
This PR does not introduce any logic changes.
- Patch 1 is a code move.
- Patch 2 uses the `hash_type!` macro to define the `sha512::Hash` type.
ACKs for top commit:
Kixunil:
ACK 71454d438a
apoelstra:
ACK 71454d438a
Tree-SHA512: c0148391fbea3602f0c52e5e08883faac6b5cb3f75ae62bfb53b1d2762e28871d349b3d3fb589f59d8bf2912dd63934f8f9a6fe6390afd378e5391eb2c91c237
Make the `sha512_256` module use similar code layout to the other hash
modules by putting the call to `hash_type` at the top followed by the
`from_engine` function.
Code move only, no other changes.
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 :)
685e5101ce Add wasm dev-deps using CI script (Tobin C. Harding)
Pull request description:
We only test WASM in CI using a stable toolchain however because we have a target specific dev-dependencies section the wasm deps get pulled in during MSRV builds - this breaks the MSRV build.
Instead of including WASM dev-dependencies in the manifest we can dynamically modify the manifest when running the WASM tests. We do this already to add the `crate-type` section so this is not really that surprising to see in the CI script.
Doing so allows us to stop pinning the transitive `syn` dependency also which is included in the dependency graph because of `wasm-bingen-test`.
Originally part of #2093
ACKs for top commit:
apoelstra:
ACK 685e5101ce
sanket1729:
ACK 685e5101ce. Removing wasm from lock-files is a win
Tree-SHA512: ad607a8a8af329e9830d9284a95a2540bf15a2ceda826b8d1c0312ee094f1f5f54599ee2faa6b611439997c349bdd9a29e6d5ae447bff96a9a538753434e2248
We only test WASM in CI using a stable toolchain however because we have
a target specific dev-dependencies section the wasm deps get pulled in
during MSRV builds - this breaks the MSRV build.
Instead of including WASM dev-dependencies in the manifest we can
dynamically modify the manifest when running the WASM tests. We do this
already to add the `crate-type` section so this is not really that
surprising to see in the CI script.
Doing so allows us to stop pinning the transitive `syn` dependency also
which is included in the dependency graph because of `wasm-bingen-test`.
On our way to v1.0.0 we are defining a standard for our error types,
this includes:
- Uses the following derives (unless not possible, usually because of `io::Error`)
`#[derive(Debug, Clone, PartialEq, Eq)]`
- Has `non_exhaustive` unless we really know we can commit to not adding
anything.
Furthermore, we are trying to make the codebase easy to read. Error code
is write-once-read-many (well it should be) so if we make all the error
code super uniform the users can flick to an error and quickly see what
it includes. In an effort to achieve this I have made up a style and
over recent times have change much of the error code to that new style,
this PR audits _all_ error types in the code base and enforces the
style, specifically:
- Is layed out: definition, [impl block], Display impl, error::Error impl, From impls
- `error::Error` impl matches on enum even if it returns `None` for all variants
- Display/Error impls import enum variants locally
- match uses *self and `ref e`
- error::Error variants that return `Some` come first, `None` after
Re: non_exhaustive
To make dev and review easier I have added `non_exhaustive` to _every_
error type. We can then remove it error by error as we see fit. This is
because it takes a bit of thinking to do and review where as this patch
should not take much brain power to review.
f2c5f19557 Introduce the `small-hash` feature for `bitcoin_hashes` (Alekos Filini)
Pull request description:
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.
The following numbers were collected on `aarch64-unknown-linux-gnu` with `cargo 1.72.0-nightly`.
## Original
```
RUSTFLAGS='--cfg=bench -C opt-level=z' cargo bench
```
```
test hash160::benches::hash160_10 ... bench: 33 ns/iter (+/- 1) = 303 MB/s
test hash160::benches::hash160_1k ... bench: 2,953 ns/iter (+/- 187) = 346 MB/s
test hash160::benches::hash160_64k ... bench: 188,480 ns/iter (+/- 11,595) = 347 MB/s
test hmac::benches::hmac_sha256_10 ... bench: 33 ns/iter (+/- 2) = 303 MB/s
test hmac::benches::hmac_sha256_1k ... bench: 2,957 ns/iter (+/- 104) = 346 MB/s
test hmac::benches::hmac_sha256_64k ... bench: 192,022 ns/iter (+/- 6,407) = 341 MB/s
test ripemd160::benches::ripemd160_10 ... bench: 25 ns/iter (+/- 1) = 400 MB/s
test ripemd160::benches::ripemd160_1k ... bench: 2,288 ns/iter (+/- 93) = 447 MB/s
test ripemd160::benches::ripemd160_64k ... bench: 146,823 ns/iter (+/- 1,102) = 446 MB/s
test sha1::benches::sha1_10 ... bench: 41 ns/iter (+/- 0) = 243 MB/s
test sha1::benches::sha1_1k ... bench: 3,844 ns/iter (+/- 70) = 266 MB/s
test sha1::benches::sha1_64k ... bench: 245,854 ns/iter (+/- 10,158) = 266 MB/s
test sha256::benches::sha256_10 ... bench: 35 ns/iter (+/- 0) = 285 MB/s
test sha256::benches::sha256_1k ... bench: 3,063 ns/iter (+/- 15) = 334 MB/s
test sha256::benches::sha256_64k ... bench: 195,729 ns/iter (+/- 2,880) = 334 MB/s
test sha256d::benches::sha256d_10 ... bench: 34 ns/iter (+/- 1) = 294 MB/s
test sha256d::benches::sha256d_1k ... bench: 3,071 ns/iter (+/- 107) = 333 MB/s
test sha256d::benches::sha256d_64k ... bench: 188,614 ns/iter (+/- 8,101) = 347 MB/s
test sha512::benches::sha512_10 ... bench: 21 ns/iter (+/- 0) = 476 MB/s
test sha512::benches::sha512_1k ... bench: 1,714 ns/iter (+/- 36) = 597 MB/s
test sha512::benches::sha512_64k ... bench: 110,084 ns/iter (+/- 3,637) = 595 MB/s
test sha512_256::benches::sha512_256_10 ... bench: 22 ns/iter (+/- 1) = 454 MB/s
test sha512_256::benches::sha512_256_1k ... bench: 1,822 ns/iter (+/- 70) = 562 MB/s
test sha512_256::benches::sha512_256_64k ... bench: 116,231 ns/iter (+/- 4,745) = 563 MB/s
test siphash24::benches::siphash24_1ki ... bench: 1,072 ns/iter (+/- 41) = 955 MB/s
test siphash24::benches::siphash24_1ki_hash ... bench: 1,102 ns/iter (+/- 42) = 929 MB/s
test siphash24::benches::siphash24_1ki_hash_u64 ... bench: 1,064 ns/iter (+/- 41) = 962 MB/s
test siphash24::benches::siphash24_64ki ... bench: 69,957 ns/iter (+/- 2,712) = 936 MB/
```
```
0000000000005872 t _ZN84_$LT$bitcoin_hashes..ripemd160..HashEngine$u20$as$u20$bitcoin_hashes..HashEngine$GT$5input17hc4800746a9da7ff4E
0000000000007956 t _ZN81_$LT$bitcoin_hashes..sha256..HashEngine$u20$as$u20$bitcoin_hashes..HashEngine$GT$5input17hf49345f65130ce9bE
0000000000008024 t _ZN14bitcoin_hashes6sha2568Midstate10const_hash17h57317bc8012004b4E.llvm.441255102889972912
0000000000010528 t _ZN81_$LT$bitcoin_hashes..sha512..HashEngine$u20$as$u20$bitcoin_hashes..HashEngine$GT$5input17h9bc868d4392bd9acE
```
Total size: 32380 bytes
## With `small-hash` enabled
```
RUSTFLAGS='--cfg=bench -C opt-level=z' cargo bench --features small-hash
```
```
test hash160::benches::hash160_10 ... bench: 52 ns/iter (+/- 3) = 192 MB/s
test hash160::benches::hash160_1k ... bench: 4,817 ns/iter (+/- 286) = 212 MB/s
test hash160::benches::hash160_64k ... bench: 319,572 ns/iter (+/- 11,031) = 205 MB/s
test hmac::benches::hmac_sha256_10 ... bench: 54 ns/iter (+/- 2) = 185 MB/s
test hmac::benches::hmac_sha256_1k ... bench: 4,846 ns/iter (+/- 204) = 211 MB/s
test hmac::benches::hmac_sha256_64k ... bench: 319,114 ns/iter (+/- 4,451) = 205 MB/s
test ripemd160::benches::ripemd160_10 ... bench: 27 ns/iter (+/- 0) = 370 MB/s
test ripemd160::benches::ripemd160_1k ... bench: 2,358 ns/iter (+/- 150) = 434 MB/s
test ripemd160::benches::ripemd160_64k ... bench: 154,573 ns/iter (+/- 3,954) = 423 MB/s
test sha1::benches::sha1_10 ... bench: 41 ns/iter (+/- 1) = 243 MB/s
test sha1::benches::sha1_1k ... bench: 3,700 ns/iter (+/- 243) = 276 MB/s
test sha1::benches::sha1_64k ... bench: 231,039 ns/iter (+/- 13,989) = 283 MB/s
test sha256::benches::sha256_10 ... bench: 51 ns/iter (+/- 3) = 196 MB/s
test sha256::benches::sha256_1k ... bench: 4,823 ns/iter (+/- 182) = 212 MB/s
test sha256::benches::sha256_64k ... bench: 299,960 ns/iter (+/- 17,545) = 218 MB/s
test sha256d::benches::sha256d_10 ... bench: 52 ns/iter (+/- 2) = 192 MB/s
test sha256d::benches::sha256d_1k ... bench: 4,827 ns/iter (+/- 323) = 212 MB/s
test sha256d::benches::sha256d_64k ... bench: 302,844 ns/iter (+/- 15,796) = 216 MB/s
test sha512::benches::sha512_10 ... bench: 34 ns/iter (+/- 1) = 294 MB/s
test sha512::benches::sha512_1k ... bench: 3,002 ns/iter (+/- 123) = 341 MB/s
test sha512::benches::sha512_64k ... bench: 189,767 ns/iter (+/- 10,396) = 345 MB/s
test sha512_256::benches::sha512_256_10 ... bench: 34 ns/iter (+/- 1) = 294 MB/s
test sha512_256::benches::sha512_256_1k ... bench: 2,996 ns/iter (+/- 198) = 341 MB/s
test sha512_256::benches::sha512_256_64k ... bench: 192,024 ns/iter (+/- 8,181) = 341 MB/s
test siphash24::benches::siphash24_1ki ... bench: 1,081 ns/iter (+/- 65) = 947 MB/s
test siphash24::benches::siphash24_1ki_hash ... bench: 1,083 ns/iter (+/- 63) = 945 MB/s
test siphash24::benches::siphash24_1ki_hash_u64 ... bench: 1,084 ns/iter (+/- 63) = 944 MB/s
test siphash24::benches::siphash24_64ki ... bench: 67,237 ns/iter (+/- 4,185) = 974 MB/s
```
```
0000000000005384 t _ZN81_$LT$bitcoin_hashes..sha256..HashEngine$u20$as$u20$bitcoin_hashes..HashEngine$GT$5input17hae341658cf9b880bE
0000000000005608 t _ZN14bitcoin_hashes9ripemd16010HashEngine13process_block17h3276b13f1e9feef8E.llvm.13618235596061801146
0000000000005616 t _ZN14bitcoin_hashes6sha2568Midstate10const_hash17h3e6fbef64c15ee00E.llvm.7326223909590351031
0000000000005944 t _ZN81_$LT$bitcoin_hashes..sha512..HashEngine$u20$as$u20$bitcoin_hashes..HashEngine$GT$5input17h321a237bfbe5c0bbE
```
Total size: 22552 bytes
## Conclusion
On `aarch64` there's overall a ~30% improvement in size, although ripemd160 doesn't really shrink that much (and its performance also aren't impacted much with only a 6% slowdown). sha512 and sha256 instead are almost 40% slower with `small-hash` enabled.
I don't have performance numbers for other architectures, but in terms of size there was an even larger improvements on `thumbv7em-none-eabihf`, with a 52% size reduction overall:
```
Size Crate Name
25.3KiB bitcoin_hashes <bitcoin_hashes[fe467ef2aa3a1470]::sha512::HashEngine as bitcoin_hashes[fe467ef2aa3a1470]::HashEngine>::input
6.9KiB bitcoin_hashes <bitcoin_hashes[fe467ef2aa3a1470]::sha256::HashEngine as bitcoin_hashes[fe467ef2aa3a1470]::HashEngine>::input
4.8KiB bitcoin_hashes <bitcoin_hashes[fe467ef2aa3a1470]::ripemd160::HashEngine as bitcoin_hashes[fe467ef2aa3a1470]::HashEngine>::input
```
vs
```
Size Crate Name
9.5KiB bitcoin_hashes <bitcoin_hashes[974bb476ef905797]::sha512::HashEngine as bitcoin_hashes[974bb476ef905797]::HashEngine>::input
4.5KiB bitcoin_hashes <bitcoin_hashes[974bb476ef905797]::ripemd160::HashEngine>::process_block
3.8KiB bitcoin_hashes <bitcoin_hashes[974bb476ef905797]::sha256::HashEngine as bitcoin_hashes[974bb476ef905797]::HashEngine>::input
```
I'm assuming this is because on more limited architectures the compiler needs to use more instructions to move data in and out of registers (especially for sha512 which ideally would benefit from 64-bit registers), so reusing the code by moving it into functions saves a lot of those instructions.
Also note that the `const_hash` method on `sha256` causes the compiler to emit two independent implementations. I haven't looked into the code yet, maybe there's a way to merge them so that the non-const `process_block` calls into the const fn.
-----
Note: commits are unverified right now because I don't have the keys available, I will sign them after addressing the review comments.
ACKs for top commit:
apoelstra:
ACK f2c5f19557
tcharding:
ACK f2c5f19557
Tree-SHA512: 1d5eb56324c458660e2571e8cf59895dc31dae9c5427c7ed36f8a0e81ca2e9a0f39026f56b6803df03635cc8b66aee3bf5182d51ab8972d169d56bcfec33771c
546c0122d7 Add simd sha256 intrinsics for x86 machines (sanket1729)
Pull request description:
This is my first time dabbling into architecture specific code and simd. The algorithm is a word to word translation of the C code from 4899efc81d/sha256-x86.c .
Some benchmarks:
With simd
```
test sha256::benches::sha256_10 ... bench: 11 ns/iter (+/- 0) = 909 MB/s
test sha256::benches::sha256_1k ... bench: 712 ns/iter (+/- 2) = 1438 MB/s
test sha256::benches::sha256_64k ... bench: 45,597 ns/iter (+/- 189) = 1437 MB/s
```
Without simd
```
test sha256::benches::sha256_10 ... bench: 47 ns/iter (+/- 0) = 212 MB/s
test sha256::benches::sha256_1k ... bench: 4,243 ns/iter (+/- 17) = 241 MB/s
test sha256::benches::sha256_64k ... bench: 271,263 ns/iter (+/- 1,610) = 241 MB/s
```
ACKs for top commit:
apoelstra:
ACK 546c0122d7
tcharding:
ACK 546c0122d7
Tree-SHA512: 7167c900b77e63cf38135a3960cf9ac2615f73b2ef7020a12b5cc3f4c047910063ba9045217b9ecfa70f7de1eb0f02f2674f291bd023a853bad2b9162fae831e
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.
154552e334 docs: Do not link to std::option::Option (Tobin C. Harding)
24843468c3 Remove rustdocs links to serde (Tobin C. Harding)
Pull request description:
Two minor patches to fix up docs links. These were originally done as part of #1880 but are unrelated so pushing them up separately.
ACKs for top commit:
apoelstra:
ACK 154552e334
RCasatta:
utACK 154552e334
Tree-SHA512: e45e1538c66b59d63a66898896927bb6c1336fb4c8515bb9e2204c8035870ef8e4a6fd32dfc83db2938afda67feb27c48989e382410f9e7ea7a967132941c720
We have just released the `hex-conservative` crate, we can now use it.
Do the following:
- Depend on `hex-conservative` in `bitcoin` and `hashes`
- Re-export `hex-conservative` as `hex` from both crate roots.
- Remove all the old hex code from `hashes`
- Fix all the import statements (makes up the bulk of the lines changed
in this patch)
Not totally necessary but since I went to the trouble of working out the
last working version add it to the docs so the next guy can grep for
`cargo update` to find them.
To help folk work out how to run the `hashes/embedded` test crate copy
over the `script` directory and an updated version of the `README` from
`embedded/bitcoin`.
In order to keep the embedded and schemacs test crates building when we
update their local transient dependencies we need to use a `patch`
section.
- For `bitcoin/embedded` add `patch` section for `internals`, `hashes`
already has an entry.
- For `hashes/embedded` add `patch` section for `internals`.
- For `hashes/extendend_tests/schemars` add `patch` section for
`internals`.
FTR for direct local dependencies we use a `path` field when specifying
the dependency.
We use two different methods for specifying local dependencies, `patch`
and also `path`. There does not seem to be a reason why we use both,
lets be uniform. Elect to use `patch` for all local crates.
8813a63ec9 internals: Bump version to 0.2.0 (Tobin C. Harding)
Pull request description:
In preparation for release bump the version and add a changelog entry. Includes updating the dependency in `bitcoin` and `hashes`.
ACKs for top commit:
apoelstra:
ACK 8813a63ec9
sanket1729:
utACK 8813a63ec9
Tree-SHA512: a9bd9d4d69cba21329f3f63a9948afe566bb97c8c65f5d46c329a696a814e9eb31372d378de1ecf0f43f0cb42f11d53dc51bc467223b34629e61315d48b39a29
In preparation for release bump the version and add a changelog entry.
Includes updating the dependency in `bitcoin` and `hashes` as well as
the minimal/recent lock files.
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
aa7b3a7d0c ci: Remove stal DO_ALLOC_TESTS variable (Tobin C. Harding)
Pull request description:
We never enable the `DO_ALLOC_TESTS` variable, and hence never test the "alloc" feature in `hashes`.
Remove the `DO_ALLOC_TESTS` variable and add "alloc" to the `FEATURES` array.
ACKs for top commit:
yancyribbens:
ACK aa7b3a7d0c
apoelstra:
ACK aa7b3a7d0c
sanket1729:
utACK aa7b3a7d0c
Tree-SHA512: 461735242ec94786624a3653c3ea108d1a8712d5a77943f2ce54b44298624f6d4a43c98a0079782211e9a4b61ac87fbadd9c98608ca0ad82574c4a63b921776f
4dfaec952c hashes: Remove stale status badge (Tobin C. Harding)
Pull request description:
We do not use travis in our CI pipeline anymore, remove the stale badge.
ACKs for top commit:
yancyribbens:
ACK 4dfaec952c
apoelstra:
ACK 4dfaec952c
sanket1729:
utACK 4dfaec952c
Tree-SHA512: 6865c8cc51f90161ea643922df3bbf890811777e3dcbff9e464d6092f0d79a6ae2798d593f6a27e7f18b05ba66a765c871d4d01086e18b8480916573288afc0e
06afd52a12 Improve hashes::Error (Tobin C. Harding)
Pull request description:
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`.
ACKs for top commit:
apoelstra:
ACK 06afd52a12
Kixunil:
ACK 06afd52a12
Tree-SHA512: 20a517daaf3e9e09744e2a65cde6e238c8f2d1224899a6c04142a3a4e635d54112b0a2e846d25256071bb27cb70f7482380f98e9a535a5498aa4c7dc0d52cc54
3717a549f9 hashes: Fix stale repository name (Tobin C. Harding)
Pull request description:
The repository name is stale since we moved the `hashes` crate into the `rust-bitcoin` repo a while ago.
ACKs for top commit:
apoelstra:
ACK 3717a549f9
Kixunil:
ACK 3717a549f9
Tree-SHA512: ac4553547f1912c8242e3b87d3cc8951598999b5512ad1b49494b3c504449939e0e60905d96464b22c71b67ca975d18814a92af6e0aa66ff4f46effb97ac0733
Tobin C. Harding
Nick Johnson
Fmt Bot
Andrew Poelstra
jamil.lambert
Jose Storopoli
Matt Corallo
448 OG
Martin Habovstiak
josibake
Steven Roose
Thabokani
shuoer86
Alekos Filini
Riccardo Casatta
sanket1729