5ba763f1a2 Update Github CI to rustc nightly-2025-05-02 (Jamil Lambert, PhD)
09132b80e1 Fix rustdoc compile_fail example (Jamil Lambert, PhD)
282434d4bd Use variable directly in format! string (Jamil Lambert, PhD)
2fbbc825c9 Allow uninlined format args (Jamil Lambert, PhD)
Pull request description:
There is a new lint error on nightly-2025-04-25 "variables can be used directly in the `format!` string".
The existing syntax `format!("{}", x)` is more commonly used than `format!("{x}")` therefore allow it in existing code.
Also the rustdoc example in #4259 now causes the new nightly to fail CI because of the unused variable.
Patches in the PR:
- Exclude the lint to allow the existing syntax in `format!` strings in all crate `lib.rs`, `build.rs.` and test files.
- Use the variables in the `format!` string for all cases in `bitcoin/examples/` since there are no other allowed lints in examples.
- Correct the function names in the rustdoc example and prefix the unused variable with an underscore.
- Update rustc to nightly-2025-05-02 (2025-04-25 had a bug which is fixed in 2025-05-02).
ACKs for top commit:
apoelstra:
ACK 5ba763f1a2ebea2cb80ee50a80228e6bda11936f; successfully ran local tests
Kixunil:
ACK 5ba763f1a2
Tree-SHA512: 20b97d2bedc631715c2b541285559a6ab84bbdb8f2f11d7282bdfecadba0cc8781a1973f0c01c25432aaceaad09e3ddbf59afe54c0bba54768e93ed9d5e50d5a
There is a new lint error on nightly-2025-04-25 "variables can be used
directly in the `format!` string".
Exclude the lint to allow the existing syntax in `format!` strings.
This method is used to implement `from_byte_array`. But there is no need
for the method to exist. We can just inline it in `from_byte_array` and
reduce the amount of indirection in our macros.
Also make the same change in sha256t.
This macro is pretty weird -- it requires that a freestanding
`from_engine` method exists, which it uses to implement a `from_engine`
method within an impl block, by just calling through to the freestanding
method.
To reduce indirection, at a very small cost in increased repeated code
(we now need to add a `impl Hash {` and `}` and doccomment around each
freestanding function, we just remove this from the macro entirely.
This will allow us to implement `from_engine` for `sha256t::Hash` in a
different way than for the non-generic hash types.
To minimize the diff, we do not re-indent the freestanding
`from_engine`. We will do that in the next commit. However, the diff is
still a bit noisy because I had to replace `fn from_engine` with `pub fn
from_engine` in every case. I took the opportunity to also change the
return type from `Hash` to `Self` to make it clearer that these are
constructors.
There was and inconsistent usage of `#`, `##` and `###` in rustdoc
headings. The difference in the rendered rustdocs is a minimal font
size change.
Change all headings to be H1 `#`.
Change all subheadings to be `###` to have a noticeable difference in
font size in the rendered docs.
Update serde from 1.0 to 1.0.103 to align with versions used in other
workspace crates. This makes the dependency constraint match reality
since it was almost always implicitly raised by the other crates.
6ba0e46b53 chore: remove unused serde_json dep (lfg2)
Pull request description:
checked by [shear](https://github.com/boshen/cargo-shear)
```
bitcoin_hashes -- hashes/Cargo.toml:
serde_json
Fixed 1 dependencies!
```
cargo +nightly build in local env is ok, local cargo.lock is ok(not include serde_json) so no need update cargo.lock
```
[[package]]
name = "bitcoin_hashes"
version = "0.16.0"
dependencies = [
"bitcoin-internals",
"hex-conservative 0.3.0",
"serde",
"serde_test",
]
```
but check CI seems need update
`error: the lock file /home/runner/work/rust-bitcoin/rust-bitcoin/Cargo.lock needs to be updated but --locked was passed to prevent this
If you want to try to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.`
ACKs for top commit:
Kixunil:
ACK 6ba0e46b53
apoelstra:
ACK 6ba0e46b53fbc9d6ea0c3e23a553920581930e75; successfully ran local tests
Tree-SHA512: 68a444e05876733af9606d4489ca30a18c9d3439dedf932e40db67dcfdb5ae40ab0f8cf937146f6b1ef92edc626dcb27a441b63c283e6945aa8071500bcc43a4
2d9e240fb6 [hashes] Use `fixed_time_eq` for `Hmac::eq` (Matt Corallo)
7ac7273013 [hashes] Disable fixed-time equality cmp when building for fuzzers (Matt Corallo)
Pull request description:
Fuzzers want to break memcmp calls into separate comparisons for coverage monitoring, allowing them to not-quite-brute-force find inputs that fully match. Thus, we disable our fancy fixed-time comparison when built with the `hashes_fuzz` cfg.
ACKs for top commit:
Kixunil:
ACK 2d9e240fb6
apoelstra:
ACK 2d9e240fb6ae13e6139713f9bb8ccb51e5dc0bff; successfully ran local tests
Tree-SHA512: 372e344ae5497a10ca03a50baadb9d2e8a6dac914bbc4ca91fd5c9b839fb036f42c8b47c252ca3466c15286e889a6f5b51390cc0d938ba24dc50b13e8b863463
492073f288 Strengthen the type of `taproot_control_block()` (Martin Habovstiak)
e8a42d5851 Unify/reduce usage of `unsafe` (Martin Habovstiak)
d42364bd9d Swap around the fields in `Address` (Martin Habovstiak)
7a115e3cf1 Make `Address` obey sanity rules (Martin Habovstiak)
bc6da1fe07 Swap around the fields in `sha256t::Hash` (Martin Habovstiak)
8ee088df74 Make `sha256t` obey sanity rules (Martin Habovstiak)
Pull request description:
Well, I thought this PR will be just the last commit... 😅
Anyway, this implements a bunch of changes to allow returning `ControlBlock` from `Witness` method(s). One cool side effect is that this PR also reduces the number of `unsafe` blocks.
ACKs for top commit:
apoelstra:
ACK 492073f28876406f8fe5a07a8a2495c8e0ba1fb3; successfully ran local tests
Tree-SHA512: 11979517cc310abf25644fc93a75deccacae66af8ba2d9b4011fdc3f414b15fac7e748399c7eef492ca850c11b7aacc3f24ec46fccf95e6d57a400212979637e
When someone is checking if an `Hmac` is equal to some other
`Hmac`, its fairly common for them to be doing a
constant-time-ness-sensitive operation. Thus, here we default to
our existing `fixed_time_eq` method for `PartialEq` on `Hamc`,
rather than the naive Rust slice comparison.
While we should consider doing the same for all hash types, we do
not yet do so here.
2867a7074f chore: fix some typos in comment (todaymoon)
Pull request description:
I fix some typos in the comments to improve readability.
ACKs for top commit:
apoelstra:
ACK 2867a7074ffef9c77d0a660d788b30d9afaf7494; successfully ran local tests
Tree-SHA512: 64a418b36443b0c1fd653cfc66fb35c13fafd0f3dff388df365ded35a83b759c5a6a3565add626f7d9813f3985019df92635fb18acd1893e453e02687717de18
Fuzzers want to break memcmp calls into separate comparisons for
coverage monitoring, allowing them to not-quite-brute-force find
inputs that fully match. Thus, we disable our fancy fixed-time
comparison when built with the `hashes_fuzz` cfg.
Since the introduction of `Script` `unsafe` started slowly creeping in
as more types with similar semantics were added. The `unsafe` in these
cases is just for trivial conversions between various pointer-like
types. As such, it's possible to move these into a single macro that
takes care of the conversions at one place and avoid repeating the same
`unsafe` code in the codebase. This decreases the cost of audits which
now only need to happen in `internals`, focuses any changes to happen in
that single macro and decreases the chance that we will mess up
similarly to the recent `try_into().expect()` issue (but this time with
UB rather than panic).
The new macro accepts syntax very similar to the already-existing struct
declarations with these differences:
* The struct MUST NOT have `#[repr(transparent)]` - it's added by the
macro
* If the struct uses `PhantomData` it must be the first field and the
real data must be the second field (to allow unsized types).
* The struct must be immediately followed by an impl block containing at
least on conversion function.
* If the struct has generics the impl block has to use the same names of
generics.
* The conversion functions don't have bodies (similarly to required
trait methods) and have a fixed set of allowed signatures.
* Underscore (`_`) must be used in place of the inner type in the
conversion function parameters.
The existing code can simply call the macro with simple changes and get
the same behavior without any direct use of `unsafe`. This change
already calls the macro for all relevant existing types. There are still
some usages left unrelated to the macro, except one additional
conversion in reverse direction on `Script`. It could be moved as well
but since it's on a single place so far it's not really required.
There's a restriction that for structs containing unsized types the
unsized type has to be the last field. `sha256t::Hash` is not an unsized
type but we are going to introduce a macro that will assume this order
to work equally well with both sized and unsized types. Thus we swap it
upfront here.
We provide the from/to_byte_array functions for casting between arrays.
We shouldn't be supporting calls to `into` to quickly do the cast.
We already removed the other direction, now remove this one.
The `hash_newtype` macro is explicitly designed to produce a hash that
is not a general purpose hash type to try and prevent users hashing
arbitrary stuff with it. E.g., `Txid` isn't meant to be just hash
arbitrary data. However we provide a `From` impl that will convert any
instance of the inner hash type into the new type. This kind of defeats
the purpose. We provide `from_byte_array` and `to_byte_array` to allow
folk to 'cast' from one hash type to another if they really want to and
its ugly on purpose.
Also, it is becoming apparent that we may be able to remove the `hashes`
crate from the public API of `primitives` allowing us to stabalise
`primitives` without stabalising `hashes`.
For both these reasons remove the `From` impl from the `hash_newtype`
macro. Note that deprecating doesn't seem to work so we just delete it.
2aac5a1f81 Fix some comments (NinaLua)
Pull request description:
I fixed some typos in the comments, please review it.
ACKs for top commit:
Kixunil:
ACK 2aac5a1f81
apoelstra:
ACK 2aac5a1f81a9bb217c4dfb7e45b96188ea60e35b; successfully ran local tests
Tree-SHA512: 50a55451b166189e8ca3d2725ed7bb8ff95a8f1ebef0296c0003414871f1b211e6ffcc3b7225302dd3d6760bfc3f65cf8ed730327ceab60cd55b868ccb0cea9a
a013700527 Replace uses of `chunks_exact` with `as_chunks` (Martin Habovstiak)
Pull request description:
This is now ready for review.
In the past we've been using `chunks_exact` because const generics were unstable but then, when they were stabilized we didn't use `as_chunks` (or `array_chunks`) since they were unstable. But the instability was only because Rust devs don't know how to handle `0` being passed in. The function is perfectly implementable on stable. (With a tiny, easy-to-understand `unsafe` block.) `core` doesn't want to make a decision for all other crates yet but we can make it for our own crates because we know that we simply never pass zero. (And even if we did, we could just change the decision.)
It also turns out there's a hack to simulate `const {}` block in our MSRV, so we can make compilation fail early.
This commit adds an extension trait to internals to provide the methods, so we no longer have to use `chunks_exact`. It also cleans up the code quite nicely.
Previous unresolved question, leaving for reference:
> One issue with this change is that the names collide which could lead to hard error in future Rust versions. How do we solve it?
> * ignore and just backport the fix once that actually happens
> * rename the methods to something reasonable (e.g. `as_array_chunks`) - this risks that they'll rename the methods to the same thing by accident and it'll break anyway
> * rename the methods to something silly (`bitcoin_as_chunks`) - yeah, the risk above is not there but then we have silly-looking code.
We've decide to just rename the methods to something that won't possibly collide.
ACKs for top commit:
tcharding:
ACK a013700527
apoelstra:
ACK a01370052715b6733f07011f28944105493bda63; successfully ran local tests; nice!
Tree-SHA512: cc3359518f97e510da5ee9a33495e26c338bfc3e4162aaffcc72ed9c7daad0daf5e9ca3d23bce50877b0d3881792e98e28d21174a4426bb01281f12285ce08d1
In the past we've been using `chunks_exact` because const generics were
unstable but then, when they were stabilized we didn't use `as_chunks`
(or `array_chunks`) since they were unstable. But the instability was
only because Rust devs don't know how to handle `0` being passed in. The
function is perfectly implementable on stable. (With a tiny,
easy-to-understand `unsafe` block.) `core` doesn't want to make a
decision for all other crates yet but we can make it for our own crates
because we know that we simply never pass zero. (And even if we did, we
could just change the decision.)
It also turns out there's a hack to simulate `const {}` block in our
MSRV, so we can make compilation fail early.
This commit adds an extension trait to internals to provide the methods,
so we no longer have to use `chunks_exact`. It also cleans up the code
quite nicely.
Now that we are able to unambiguously go from a hash engine to its
associated hash type there is no longer any need for the `GeneralHash`
trait.
Please note that IMO this concept of a general hash type as opposed to
one where one can hash arbitrary data still exists in the codebase - it
is implicitly in the `hash_newtype` macro.
Remove the `GeneralHash` trait.
Add a standalone `hash_byte_chunks` function that is a drop in
replacement for `GeneralHash::hash_byte_chunks`. Do not add it to
`hmac` - this is in parity with the current code because `Hmac` does
not implement `GeneralHash::hash_byte_chunks`.
Add a standalone `hash` function that is a drop in replacement for
`GeneralHash::hash`. Do not add it to `hmac` - this is in parity with
the current code because `Hmac` does not implement `GeneralHash::hash`.
Use the new function in `bitcoin` removing all occurrences of
`GeneralHash` from `bitcoin`.
In `hashes` replace usage of `GeneralHash::hash` with the new `hash`
function.
We would like to do away with the `GeneralHash` trait. Currently we
bound `Hmac` and `HmacEngine` on it but this is unnecessary now that we
have added `HashEngine::finalize` and `HashEngine::Hash`.
Bound the `HmacEngine` on `HashEngine` (which has an associated `Hash`
type returned by `finilalize`).
Bound `Hmac` type on `T::Hash` where `T` is `HashEngine`.
Includes some minor shortening of local variable names around hmac
engine usage.
Note this means that `Hmac` no longer implements `GeneralHash`.
Add an associated const `Hash` to the `HashEngine` trait. Also add a
`finalize` method that converts the engine to the associated hash.
For now just use the existent `from_engine` stuff. We can refactor
later.
83d071e54b chacha20: Add whitespace (Tobin C. Harding)
4451724d31 chacha20: Add a docs heading (Tobin C. Harding)
d4417f9666 io: Improve crate docs heading (Tobin C. Harding)
c466554948 hashes: Improve crate docs heading (Tobin C. Harding)
6f4eb60936 Improve docs crate headings (Tobin C. Harding)
Pull request description:
Make them all uniform after taking 2 minutes online to try find a nice format.
ACKs for top commit:
apoelstra:
ACK 83d071e54be0bc4ebd760a490a3ca887c0bf90a8; successfully ran local tests; lgtm
Tree-SHA512: 6f08c6cda91a7a870f1080b497f89607ac3d6b3c0234cbd2ba2da8710d46816398acac0bca2a49a3bc9466b814ae446842d3d304a3735df9f983e3ff5df005db
539d45420a Typo fix in: README.md (leonarddt05)
Pull request description:
Hi,
I suggest some typo fix' for this doc:
1- "since these are needed to display hashes anway." Should be "since these are needed to display hashes anyway." (spelling error).
2- "bench mark" and "bench marks" Should be "benchmark" and "benchmarks" (incorrect spacing; "benchmark" is a single word).
Thanks.
ACKs for top commit:
apoelstra:
ACK 539d45420a4540e13099a61996db87aeb3887002; successfully ran local tests
tcharding:
ACK 539d45420a
Tree-SHA512: 36fe65a9ea4d8d2fce90fb91e7966bc41ab5ab1cf9b5ea39efe88b1756d46724428d5dccfb1e7718721747e032ee3c52d848908652d82816f7f990f527c47485
Hi,
I suggest some typo fix' for this doc:
1- "since these are needed to display hashes anway."
Should be "since these are needed to display hashes anyway." (spelling error).
2- "bench mark" and "bench marks" Should be "benchmark" and "benchmarks" (incorrect spacing; "benchmark" is a single word).
Thanks.
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
David Klank
Fallengirl
merge-script
Jamil Lambert, PhD
Tobin C. Harding
Nick Johnson
lfgtwo
Fmt Bot
Matt Corallo
Martin Habovstiak
Eval EXEC
todaymoon
NinaLua
leonarddt05
ndungudedan