We are trying to make the `hashes` crate easier to read and also
possibly parseable by machines to see what is different where.
Move the test and bench code into separate files. Make special effort to
keep formatting as is so the diff is easier to review. We will run the
formatter in the next patch.
Internal change only.
There are five modules in `hashes` that implement cryptography (i.e.
have a `process_block` function). For each of them create a new
submodule called `crypto` and move the code there.
Code organisational refactor only, no logic changes.
In preparation for removing a bunch of macros move all the modules to
`<mod>/mod.rs`.
Do so by running the following shell:
```bash
for mod in hash160 ripemd160 sha1 sha256 sha256d sha256t \
sha384 sha512 sha512_256 siphash24 hkdf hmac; do
mkdir $mod
mv "$mod.rs" "$mod/mod.rs"
done
```
Internal change only.
The other hashes that require initial state (keys etc) both have an
`engine` function on the hash type because they cannot use the
`GeneralHash::engine` function.
Add an `engine` function to the `siphash24::Hash` type.
b98c489066 hashes: Move from_engine functions (Tobin C. Harding)
Pull request description:
In order to use the `general_hash_type` macro the must exist a standalone `from_engine` function. Currently this function is in different places in different modules. In an effort to make the `hashes` code easier to reason about put the functions right below the macro.
Code move only, no other changes.
ACKs for top commit:
Kixunil:
ACK b98c489066
apoelstra:
ACK b98c489066e8916a383099e5037e5a24832548ba; successfully ran local tests
Tree-SHA512: 8dfbf2b422d078d687708fa94a478ca597fae141f5c1f0a318a36152ca33f4760bb0545ab67523c558a8c3b8d258356975c5e357600d0ac980d473250a2af20e
18619a6d0b api: Run just check-api (Tobin C. Harding)
1eb8f1f9e0 Add a Hmac::engine function (Tobin C. Harding)
c352d376ed Do not implement Default for HmacEngine (Tobin C. Harding)
Pull request description:
The `HmacEngine` should be created using a key. Currently we are providing a `Default` impl that uses `&[]` as the key. This is, I believe, a hangover from when we had a `Default` trait bound somewhere else. It is incorrect and an API footgun - remove it.
Note this PR includes changes to the bench code in `hmac` that highlights the footgun - pity the poor user we even shot ourselves.
Patch 2 adds a constructor `Hmac::engine` and uses it in the bench code.
ACKs for top commit:
Kixunil:
ACK 18619a6d0b
apoelstra:
ACK 18619a6d0b0bca7b7e3603e260b254b4aae6cebf; successfully ran local tests
Tree-SHA512: c96640e7ffba52d5b13b76a6dd9e1381788efcf56ee76300c5111541466bab8018d2546bcecf237c42dbd82e9372a0e43e1ecec37147508e879365d92a4c1451
The `HmacEngine` cannot be constructed by way of the `GeneralHash` trait
method because it requires a key. However we can still add an inherent
function to the type to construct an engine.
Add the engine constructor and use it in bench code.
The `HmacEngine` should be created using a key. Currently we are
providing a `Default` impl that uses `&[]` as the key. This is, I
believe, a hangover from when we had a `Default` trait bound somewhere
else. It is incorrect and an API footgun - remove it.
In order to use the `general_hash_type` macro the must exist a
standalone `from_engine` function. Currently this function is in
different places in different modules. In an effort to make the `hashes`
code easier to reason about put the functions right below the macro.
Code move only, no other changes.
85e04315d5 Remove test_ prefix from unit tests (Tobin C. Harding)
Pull request description:
There is a loose convention in Rust to not use `test_` prefix. The reason being that `cargo test` outputs 'test <test name>' using the prefix makes the output stutter.
This patch smells a bit like code-churn but having the prefix in some places and not others is confusing to new contributors and is leading me to explain this many times now. Lets just fix it.
Remove the prefix unless doing so breaks the code.
ACKs for top commit:
shinghim:
ACK 85e04315d5
apoelstra:
ACK 85e04315d5eb90075ce55bf18fab8876a4583def; successfully ran local tests
Tree-SHA512: d90ae5ef75cc5e5a8f43f60819544f1a447f13cbe660ba71e84b8f27bfcc04a11d3afde0ed56e4eea5c73ebc3925024b800a1b995f73142cab892f97a414f14a
There is a loose convention in Rust to not use `test_` prefix. The
reason being that `cargo test` outputs 'test <test name>' using the
prefix makes the output stutter.
This patch smells a bit like code-churn but having the prefix in some
places and not others is confusing to new contributors and is leading me
to explain this many times now. Lets just fix it.
Remove the prefix unless doing so breaks the code.
Rust macros, while at times useful, are a maintenance nightmare. And
we have been bitten by calling macros from other crates multiple times
in the past.
In a push to just use less macros remove the usage of the
`impl_from_infallible` macro in all the leaf crates and just write the
code.
We have two macro definitions feature gated on `serde`. At some stage we
added the `doc(hidden)` attribute to one of them but forgot to add it to
the other. This technically makes our features non-additive. This macro
is "internal" so its unlikely that this is being used in the wild.
Add `doc(hidden)` to the `serde_impl` macro that is missing it.
Found by `cargo semver-checks` after recent upgrade to 0.38
We need to do a quick release because it turns out I was wrong in
thinking that making `hex` an optional dependency is not a breaking
change.
```
the package `bitcoin` depends on `bitcoin_hashes`, with features: `hex`
but `bitcoin_hashes` does not have these features. It has a required
dependency with that name, but only optional dependencies can be used as
features.
```
Add a changelog, bump the version, depend on the new version repo wide,
and update the lock files - like its our job.
To conform to Rust API guidelines examples should Examples use ?, not
try!, not unwrap (C-QUESTION-MARK).
Label the examples as `# Examples`.
Replace one `unwrap()` with `expect()` . The others don't technically
conform to the guidelines but are warranted.
The only reason we need `hex-conservative` is to parse strings and
format them as hex. For users that do not require this functionality we
can make the `hex-conservative` crate an optional dependency.
The `serde` feature requires `Display` so we enable `hex` from the
`serde` feature.
If `hex` feature is not enabled we still need to be able to debug so
provide `fmt::Debug` functionality by way of macros.
Close: #2654
For the `hashes` crate we would like to make `hex` an optional
dependency. In preparation for doing so do the following:
- Remove the trait bounds from `GeneralHash`
- Split the hex/string stuff out of `impl_bytelike_traits` into a
separate macro.
The `impl_bytelike_traits` macro is public and it is used in the
`hash_newtype` macro, also public.
Currently if a user calls the `hash_newtype` macro in a crate that
depends on `hashes` without the `serde` feature enabled and with no
`serde` dependency everything works. However if the user then adds a
dependency that happens to enable the `serde` feature in `hashes` their
build will blow up because `serde` code will start getting called from
the original crate's call to `hash_newtype`.
Pull the serde stuff out of `hash_newtype` and provide a macro to
implement it `impl_serde_for_newtype`.
1649b68589 Standardize wording to `constructs a new` (Jamil Lambert, PhD)
27f94d5540 Replace `creates` with `constructs` (Jamil Lambert, PhD)
Pull request description:
As discussed in issue #3575 there are various ways of saying a new object is created.
These have all be standardized to the agreed version.
Close#3575
ACKs for top commit:
apoelstra:
ACK 1649b68589834dfe9d5b63812da3e9f0e5930107; successfully ran local tests
tcharding:
ACK 1649b68589
Tree-SHA512: 0ed9b56819c95f1fc14da1e0fdbbe03c4af2d97a95ea6b56125f72913e8d832db5d2882d713ae139d00614e651f3834a4d72528bdf776231cceb6772bf2f9963
fe8ca21ec2 hashes: Duplicate impl_from_infallible (Tobin C. Harding)
7652d0ddfc hashes: Hide innards of FromSliceError (Tobin C. Harding)
5232bba62b hashes: Move FromSliceError to submodule (Tobin C. Harding)
Pull request description:
Hide the internals of the `hashes::FromSliceError`.
ACKs for top commit:
apoelstra:
ACK fe8ca21ec282cfe50e3f39b7f86c619d30d7f542; successfully ran local tests
Tree-SHA512: dac33777353dd81c8c86de331b2ab30d0a5268f2be7685f85405d29809ec36eeab31b0e71c9f09e820e06a93c3f05b7d675e5e729b780e8600b960cad4a02c77
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
We don't want a dependency on `internals` for `hashes` unless its really
needed.
Duplicated the `impl_from_infallible` macro into `hashes::error` and
use it to implement from infallible for `FromSliceError`.
Add a public `error` module and move the one error for the `hashes`
crate into it.
Re-export at the crate root.
Done in preparation for adding an additional error type.
We have a couple of problems:
1. There are two macros currently for fmt stuff that do similar things,
`arr_newtype_fmt_impl` and `hex_fmt_impl` - the difference is not
immediately obvious, its the way that the byte array is iterated.
2. Our hash types are missing `AsRef<[u8; len]>` and `Borrow<[u8; len]>`.
Introduce a new macro and remove a bunch of other macros. Include
extensive docs but hide the macro from public docs because its not
really for consumers of the library.
The macro requires `$crate::hex` to point to `hex-conservative`.
Note the macro is pretty generic (as in general purpose), `hashes` might
not be the right home for it. Potentially a better place would be in
`hex` itself?
6aa8c2b023 Remove needless_borrows_for_generic_args (Tobin C. Harding)
Pull request description:
This has been fixed and we use nightly to lint so we have access to the merged fix.
ACKs for top commit:
apoelstra:
ACK 6aa8c2b023619534437043e76ec4252f27295875; successfully ran local tests
Tree-SHA512: 011677b6c1a2dc6c63097cafd1682fab5d9bdd94133b872bdd49699a55c80a01a8e6e5e844ae1cfe1ca9da47a2ba2ed6b910719b1ccbb06e60e22ecb01ec0efc
80b5d8b7b3 hashes: Add changelog for release 0.15.0 (Tobin C. Harding)
Pull request description:
The version has already been bumped on `master`.
In preparation for releasing `hashes v0.15.0` add a changelog entry.
ACKs for top commit:
apoelstra:
ACK 80b5d8b7b36ea97007f062971a732ec8a512cfa5; successfully ran local tests; again lots of stuff!
Tree-SHA512: ab1bbdf7df819c587dc905e4cdcda2232ede2a495a0e395e5d6f6bd730a7c3c0675e05f8ab616e5e6dee0a87836ab486d025f80608e73b713007391711ef15ba
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
We are about to release `bitcoin_hashes 0.15.0`, replace the TBD string
with the version number.
Requires changing `allow(deprecated_in_future)` attribute to
`allow(deprecated)` (in functions that are them self deprecated).
We have two files one for public macros and one for private macros. Move
the `engine_input_impl` macro to the private macros file.
Requires change to call sites because we do not have `use_macros`
attribute on the `internal_macros` file.
These three macros are solely provided to reduce code duplication, they
are only part of the public API because they are used by the "real"
public macro `hash_newtype`.
Roll the `serde_macros` module into `macros`, requires making `macros`
public but since it explicitly holds public macros this is reasonable.
Keep the original module and deprecate it.
Tobin C. Harding
Jamil Lambert, PhD
merge-script
calciumbe
Leo Nash