Enable `bitcoin_hashes v0.14.0` #76

tcharding · 2024-07-30T21:58:50Z

Hardware devices like the smallest binary possible, currently if one builds with latest rust-bitcoin and rust-bip39 they get two versions of bitcoin_hashes in the dependency graph because we don't support the latest bitcoin_hashes.

Note using the latest version causes the MSRV to increase to Rust v0.56.1.

tcharding · 2024-07-30T21:59:46Z

cc @afilini

afilini · 2024-07-31T19:03:15Z

ACK, that would be helpful for me, all the other libs like rust-bitcoin and bdk have already upgraded to hashes 0.14

Kixunil

This would be nice for me even just for cleanness sake.

Kixunil · 2024-08-26T12:03:41Z

Cargo.toml

 # Enabling the "rand" feature by default to run the benches
 bip39 = { path = ".", features = ["rand"] }
-bitcoin_hashes = ">=0.12,<0.14" # enable default features for test
+bitcoin_hashes = ">=0.12,<=0.14" # enable default features for test


This must be >= 0.12, <0.15, otherwise 0.14.1 will be unusable even though it'd be compatible.

Oh wow, TIL. Will fix, thanks.

I'm going to go right ahead and 'trust don't verify'.

The verification is easy, just evaluate 0 < 1 expression in your head. :)

Oh I thought we were talking about 0.14 working for 0.14.0 but not for 0.14.1.

0.14 implies 0.14.0

Ok, I have no idea what you were talking about in your original comment then. <= 14 is equivalent to < 15

It's not. 0.14 == 0.14.0
0.14.1 > 0.14.0
0.14.1 < 0.15.0
And the above implies 0.14.1 > 0.14 which is a negation of 0.14.1 <= 0.14
So for <= 14 to be equivalent to < 15 we require all x to satisfy x <= 0.14 <-> x <0.15 and we have a counter example for x = 0.14.1. QED

I get it now. Seems the confusion started when I wrote "I'm going to go right ahead and 'trust don't verify'." and I have no clue what I was thinking then. Your original review comment is actually perfectly clean.

Just call me retarded :)

Kixunil · 2024-08-26T12:04:04Z

Cargo.toml


 # Unexported dependnecies
-bitcoin_hashes = { version = ">=0.12, <=0.13", default-features = false }
+bitcoin_hashes = { version = ">=0.12, <=0.14", default-features = false }


This also needs to be <0.15, it was already broken.

Kixunil

ACK 2b47d40

tcharding · 2024-08-29T19:56:19Z

Oh this PR is no good anyway, bitcoin_hashes v14 has an incompatible MSRV.

tcharding · 2024-09-18T04:33:07Z

Raised #79 to discuss the MSRV bump. Converting to draft until that resolves.

kayabaNerve · 2025-02-04T11:07:28Z

Is this a MSRV bump? Anyone who wants an older MSRV can use an older version of bitcoin_hashes. This may cause some automated workflows to fail on first pass? But I wouldn't consider that breaking, as at worst it would be trivially correctable by an extra line in a Cargo.toml of bitcoin_hashes = "=0.12".

As someone who currently has two copies of bitcoin_hashes in-tree, I'd appreciate this.

Kixunil · 2025-02-04T11:20:23Z

@kayabaNerve that line wouldn't be great, better jut use Cargo.lock correctly.

kayabaNerve · 2025-02-04T12:43:25Z

Precise updates get overriden upon the next call to update. A line in the toml, next to the line adding this very crate as a dependency, would pin it. That's why I said what I said. It's irrelevant to the larger point I don't believe this is a MSRV break and should be able to move forward regardless though.

Kixunil · 2025-02-04T15:11:41Z

Precise updates get overriden upon the next call to update.

MSRV doesn't get bumped on update if you use recent cargo. But yes, this is not MSRV break in the sense that it'd justify major version.

benma · 2025-08-20T08:53:08Z

Could you take this PR out of draft and merge and release please? 😄 Seems like a simple change and I need it for the reason @tcharding outlined.

dmitrmax · 2025-08-28T15:52:53Z

Could you take this PR out of draft and merge and release please? 😄 Seems like a simple change and I need it for the reason @tcharding outlined.

I second this. The simplest fix is resting on the shelf for more than a year now.

Maintainer, please do the v2.2.1 release.

benma · 2025-09-03T08:38:42Z

Ping @tcharding @stevenroose, please let's merge, it's a simple change and very much needed. Thanks

tcharding · 2025-09-04T02:55:48Z

I'm not a maintainer here sorry mate. If you have a direct link to Steven you could hassle him.

benma · 2025-09-21T07:00:43Z

@tcharding I guess the first step would be for you to rebase it (there are conflicts) and take it out of draft.

tcharding · 2025-09-21T23:01:52Z

No sweat, done.

tcharding · 2025-09-21T23:07:31Z

Oh this cannot be undrafted because of the MSRV bump. At the risk of being rude, and @stevenroose you can slap me in person when you see me next, I'd suggest one of the following:

Someone pester Steven to maintain his lib (I'm not going to do this, I've hassled him enough about other libs more important to me).
Suggest to him to bring on another maintainer, or offer to be one.
Fork the crate and sort it out.

I'm not the don of the rust-bitcoin org but I do feel in someway responsible that we have crates in the org that are not maintained. I don't know the solution to that problem.

No one is required to do anything round here, so props to Steven for writing the crate, and all due respect.

tcharding · 2025-10-26T22:37:25Z

We have @tnull as a crate owner on crates.io now. And if my private conversation with @stevenroose is correct then @elsirion has been invited to be a crate owner also. I assume both of you were also given merge perms here so this PR should be able to make some progress now please.

elsirion · 2025-10-27T08:32:46Z

I don't have merge perms or an crates.io invite yet as far as I can tell. Happy to take on the responsibility, but also respect if @stevenroose doesn't want too many people as maintainers.

tnull · 2025-10-27T09:58:24Z

README.md

 - `bitcoin_hashes v0.13`: MSRV v1.48.0
+- `bitcoin_hashes v0.14`: MSRV v1.56.1

-When using older version of Rust, you might have to pin the versions of several crates, for an up-to-date list refer to [`contrib/test.sh`](contrib/test.sh):


Why is this no longer true, i.e., why don't we have to pin back the other dependencies below?

Possibly a faulty rebase conflict resolution? I made changes to these lines in #86 and this PR was open forever. @tcharding

Oh I just clobbered them because over the last year (since edition 2018 came into rust-bitcoin) I've barely ever had to pin anymore. Admittedly not the best reason for the change, I've put it back how it was.

I think it depends which MSRV you are targeting, a lot of the bip39 deps seem to have bumped their MSRV in minor version releases. I only had to pin for the old rust version tests back then.

In rust-bitcoin and related crates we've avoided pinning by using fixed lockfiles, which we basically never update even though we should. That's why you haven't had to deal with this crap @tcharding.

Yeah, eventually we might want to discuss doing a conservative MSRV bump here (1.41 seems very old by now, for every standard), potentially just aligning it with the rust-bitcoin policy.

But, for now, that's out-of-scope of this PR. And if we already have the checks in place, there probably is no rush to do so.

stevenroose

Nice

elsirion

Would like to clarify if the README.md changes were intentional before merging, I have a strong suspicion that they weren't and the described pinning isn't sufficient.

Hardware devices like the smallest binary possible, currently if one builds with `rust-bitcoin v0.32` and `rust-bip39` they get two versions of `bitcoin_hashes` in the dependency graph because we don't support `bitcoin_hashes v0.14`. Note that using `bitcoin_hashes 0.14` bumps the MSRV to Rust v1.56.1

tcharding · 2025-10-27T20:53:18Z

I don't have merge perms or an crates.io invite yet as far as I can tell. Happy to take on the responsibility, but also respect if @stevenroose doesn't want too many people as maintainers.

@apoelstra can you add @elsirion and @tnull as a maintainers please. @stevenroose has confirmed this by signal message with me. He has added them both as owners on crates.io already. @tnull said he would message you directly on signal also about this.

For anyone reading @stevenroose does not check GitHub notifications so will likely not read this message any time soon. You'll just have to believe me that I talked with him.

https://crates.io/crates/bip39

tcharding · 2025-10-27T20:54:35Z

Hey Tobin! Talked to Steven and it seems he can't add me as an owner of the bip39 crate on Github as it's part of the org now. Could you add me and elsirion?

This via signal from @tnull to me.

Docs were fixed, still need to test MSRV though, so no full ACK yet.

There are multiple MSRVs in this crate because of the `bitcoin_hashes` dependency and also the `zeroize` feature. Add MSRV test jobs for the three MSRVs effected by `bitcoin_hashes` and also one for the `zeroize` feature.

tcharding · 2025-10-27T23:22:48Z

Docs were fixed, still need to test MSRV though, so no full ACK yet.

Good eye, thanks. I've added 4 MSRV jobs which is pretty ugly but should get the coverage we want.

tnull · 2025-10-28T08:43:50Z

I don't have merge perms or an crates.io invite yet as far as I can tell. Happy to take on the responsibility, but also respect if @stevenroose doesn't want too many people as maintainers.

@apoelstra can you add @elsirion and @tnull as a maintainers please. @stevenroose has confirmed this by signal message with me. He has added them both as owners on crates.io already. @tnull said he would message you directly on signal also about this.

For anyone reading @stevenroose does not check GitHub notifications so will likely not read this message any time soon. You'll just have to believe me that I talked with him.

https://crates.io/crates/bip39

Can confirm I was added now.

tnull

Mostly looks good, one nit, one comment.

tnull · 2025-10-28T08:51:40Z

README.md


 When using older version of Rust, you might have to pin the versions of several crates, for an up-to-date list refer to [`contrib/test.sh`](contrib/test.sh):
-
+ 


nit: Spurious whitespace here.

tnull · 2025-10-28T08:58:33Z

contrib/test.sh

 fi

+# Pin dependencies as required if we are using MSRV toolchain.
+if cargo --version | grep "1\.48"; then


No super strong opinion, but in LDK we use such a pattern (just an example, see https://github.com/lightningdevkit/rust-lightning/blob/0.2/ci/ci-tests.sh):

RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }') # The backtrace v0.3.75 crate relies on rustc 1.82 [ "$RUSTC_MINOR_VERSION" -lt 82 ] && cargo update -p backtrace --precise "0.3.74" --verbose

Seems this could be a bit cleaner/less error prone if we start handling even more different rust versions.

I just hope that once we hit rust version 1.480 in 45 years we won't be testing 1.48 anymore 😆

elsirion · 2025-10-28T09:50:43Z

LGTM, just some syntax errors in the workflow file

Screenshot 2025-10-28 at 05-49-43 Enable `bitcoin_hashes v0 14 0` · rust-bitcoin_rust-bip39@b3f9c4d

Kixunil suggested changes Aug 26, 2024

View reviewed changes

tcharding force-pushed the 07-30-update-hashes branch from 46ef0ed to 2b47d40 Compare August 26, 2024 23:34

Kixunil approved these changes Aug 27, 2024

View reviewed changes

tcharding mentioned this pull request Sep 18, 2024

Bump MSRV to Rust 1.63 #79

Closed

tcharding marked this pull request as draft September 18, 2024 04:33

tcharding mentioned this pull request Nov 12, 2024

chore: allow using bitcoin_hashes v0.14 #82

Closed

tvolk131 mentioned this pull request Nov 13, 2024

Duplicate dependencies fedimint/fedimint#6293

Open

tcharding force-pushed the 07-30-update-hashes branch 2 times, most recently from 1ca2e1b to 26683c2 Compare September 21, 2025 23:01

tcharding marked this pull request as ready for review September 21, 2025 23:02

tcharding marked this pull request as draft September 21, 2025 23:03

tcharding force-pushed the 07-30-update-hashes branch 2 times, most recently from 5994a4c to 1357566 Compare October 26, 2025 22:33

tcharding marked this pull request as ready for review October 26, 2025 22:33

tcharding changed the title ~~Enable bitcoin_hashes v0.14.0~~ Enable bitcoin_hashes v0.14.0 Oct 26, 2025

tcharding mentioned this pull request Oct 26, 2025

Additional maintainers #37

Closed

tnull reviewed Oct 27, 2025

View reviewed changes

stevenroose approved these changes Oct 27, 2025

View reviewed changes

elsirion previously requested changes Oct 27, 2025

View reviewed changes

tcharding force-pushed the 07-30-update-hashes branch from 1357566 to e30b544 Compare October 27, 2025 20:48

tcharding requested a review from elsirion October 27, 2025 20:49

CI: Test MSRV

b3f9c4d

There are multiple MSRVs in this crate because of the `bitcoin_hashes` dependency and also the `zeroize` feature. Add MSRV test jobs for the three MSRVs effected by `bitcoin_hashes` and also one for the `zeroize` feature.

tnull reviewed Oct 28, 2025

View reviewed changes


		When using older version of Rust, you might have to pin the versions of several crates, for an up-to-date list refer to [`contrib/test.sh`](contrib/test.sh):

Uh oh!

Enable bitcoin_hashes v0.14.0 #76

Are you sure you want to change the base?

Enable bitcoin_hashes v0.14.0 #76

Conversation

tcharding commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Jul 30, 2024

Uh oh!

afilini commented Jul 31, 2024

Uh oh!

Kixunil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kixunil left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding commented Aug 29, 2024

Uh oh!

tcharding commented Sep 18, 2024

Uh oh!

kayabaNerve commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kixunil commented Feb 4, 2025

Uh oh!

kayabaNerve commented Feb 4, 2025

Uh oh!

Kixunil commented Feb 4, 2025

Uh oh!

benma commented Aug 20, 2025

Uh oh!

dmitrmax commented Aug 28, 2025

Uh oh!

benma commented Sep 3, 2025

Uh oh!

tcharding commented Sep 4, 2025

Uh oh!

benma commented Sep 21, 2025

Uh oh!

tcharding commented Sep 21, 2025

Uh oh!

tcharding commented Sep 21, 2025

Uh oh!

tcharding commented Oct 26, 2025

Uh oh!

elsirion commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Enable `bitcoin_hashes v0.14.0` #76

Enable `bitcoin_hashes v0.14.0` #76

tcharding commented Jul 30, 2024 •

edited

Loading

kayabaNerve commented Feb 4, 2025 •

edited

Loading

elsirion commented Oct 27, 2025 •

edited

Loading

tnull Oct 28, 2025 •

edited

Loading

tcharding commented Oct 27, 2025 •

edited

Loading