Implementors of the Tag trait had to use the #[derive(Clone)] attribute.
This change eliminates this need by removing the Clone trait
bound from the Tag trait.
f61e93ccf1 Properly deprecate Hash::from_slice (Tobin C. Harding)
50c0af7138 Stop using Hash::from_slice (Tobin C. Harding)
Pull request description:
The `hashes::error::FromSliceError` error is only returned from `from_slice`. We attempted to deprecate this function but it seems we only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
- stop using the deprecated functions in `bitcoin`
Close: #4053
ACKs for top commit:
apoelstra:
ACK f61e93ccf1db7e7e3c9604fdb09b4e25195d88b2; successfully ran local tests
Tree-SHA512: 61a0e5127019859776ffac66bd4d320c86b8462bb1e908127d0bf42896aaa8df85fd2b06850342b694ca1cd68ed50355c81cad6ae3e9a5fd6e3933efe85498ad
da8b85ed7c Implement Debug for Hkdf (Tobin C. Harding)
85652359e8 hashes: Derive Copy and Clone for Hkdf (Tobin C. Harding)
Pull request description:
Currently the `Hkdf` type does not derive any traits.
Derive `Copy` and `Clone` and implement `Debug` based on secret obfuscation algo in `rust-secp` (in the `secret` module).
ACKs for top commit:
apoelstra:
ACK da8b85ed7cf34c0510c0b64c67477d3819bee369; successfully ran local tests
Kixunil:
ACK da8b85ed7c
Tree-SHA512: 8ae0e8857ea0e32ad5ef8f544979eeb9d530beb1b6f046ce28a286ca2231f8f696a9f4f8d9ea219d3389c4216d6b69766dbd96edbb27e7489803ac583bf3b200
The `hashes::error::FromSliceError` error is only returned from
`from_slice`. We attempted to deprecate this function but it seems we
only did half a job at it.
- deprecate _all_ instances of the method/function
- deprecate the error type
This function is deprecated, stop using it in favour of
`Hash::from_byte_array`.
Patch only touches test code, I'm guessing that is why lint warnings
were no showing up.
Currently the `Hkdf` type does not derive any traits.
We would like to derive the common set of traits but there are a bunch
reasons we can't;
- Don't want to leak secrets in `Debug`.
- Don't want to enable timing attacks with Eq/Ord and friends.
For now just derive `Copy` and `Clone`. We will then implement `Debug`
manually.
The two main public macros can be used in function scope - prove it.
While we are at it prove that additional attributes are supported by
them both as well as visability keywords.
`hex/std` and `hex/alloc` should only be included if optional
dependency `hex` is enabled`.
Add `?` so it is only included if the optional feature `hex` is enabled.
Currently in order to release `hashes v1.0` we need to 1.0 `io` as well.
For multiple reasons, many out of our control, the `io` crate may not
stabalise any time soon.
Instead we can invert the dependency between the two crates.
This is an ingenious idea, props to Kixunil for coming up with it.
Notes
- `io` does not currently re-export the `hashes` crate.
- This work highlights that we cannot call `hash_reader` on a siphash.
- The `Hmac::hash_reader` uses the default key which may not be obvious.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
We would like it if two different pre-tagged engines were considered
different types so it is not possible to mix them up.
Add a new `sha256t::HashEngine<T>` where `T` is a tag the same as on
`sha256t::Hash<T>`.
This function is meant to be used in conjunction with the
`general_hash_type` macro but the `sha256t` module does not use that
macro.
Inline the function. Internal change only.
Now we have an associated const we can do away with the `engine` trait
method all together. Users can call `Hash<FooTag>::engine` instead. This
is better because its an API more similar to the other hash types and
therefor easier to discover and remember.
Instead of requiring users of the `Tag` trait to implement the `engine`
method we can have an associated const and provide an `engine` method
with a default implementation.
Use an associated const for the pre-tagged hash engine. Fro now keep the
`engine` trait method but have a default impl that returns the const. We
will remove it as a separate patch to assist review.
This macro is a maintenance burden. We would like to put a tag on the
hash engine but doing so would require breaking this macro anyway so
lets just delete it.
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`.
Fmt Bot
ndungudedan
merge-script
Tobin C. Harding
Jamil Lambert, PhD
calciumbe