Skip to content

chore(msrv): bump MSRV to 1.85.0 #2055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

chore(msrv): bump MSRV to 1.85.0 #2055

wants to merge 4 commits into from

Conversation

oleonardolima
Copy link
Contributor

@oleonardolima oleonardolima commented Oct 2, 2025 •
edited
Loading

fixes #2009

Description

It bumps the project MSRV to 1.85.0, as the latest Debian trixie release establishes that as stable, see: https://tracker.debian.org/pkg/rustc.

This PR does:

  • update all references to the previous 1.63.0 MSRV.
  • update the rust-version to 1.85.0.
  • update the CI to not exclude bdk_electrum anymore.
  • update the ci/pin-msrv.sh to 1.85.0.

Notes to the reviewers

Should we update something in the GitHub repository configuration to get rid of old mandatory CI jobs ?

Changelog notice

### Changed

- Update all references to the previous 1.63.0 MSRV.
- Update the rust-version to 1.85.0.
- Update the CI to not exclude `bdk_electrum` anymore.
- Update the ci/pin-msrv.sh to 1.85.0.

Checklists

All Submissions:

@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch from b04bbed to b6e75cb Compare October 2, 2025 04:53
@oleonardolima oleonardolima marked this pull request as ready for review October 2, 2025 05:14
@oleonardolima oleonardolima self-assigned this Oct 2, 2025
@oleonardolima oleonardolima added chore Non-coding related work github_actions Pull requests that update GitHub Actions code labels Oct 2, 2025
@oleonardolima oleonardolima moved this to Needs Review in BDK Chain Oct 2, 2025
evanlinjin
evanlinjin approved these changes Oct 2, 2025
View reviewed changes
Copy link
Member

@evanlinjin evanlinjin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b6e75cb

Ran cargo test --all-features with:

❯ cargo --version
cargo 1.85.0 (d73d2caf9 2024-12-31)

@evanlinjin evanlinjin mentioned this pull request Oct 2, 2025
Closed
2 tasks
@oleonardolima oleonardolima mentioned this pull request Oct 2, 2025
Open
4 tasks
@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch 2 times, most recently from 8bb3255 to 93c2172 Compare October 8, 2025 00:08
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 8, 2025
View reviewed changes
@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch 2 times, most recently from 3e626f3 to adc99e7 Compare October 8, 2025 00:22
@oleonardolima
Copy link
Contributor Author

I've added new commits bumping the criterion version and updating the toolchain action to match the one used in bdk_wallet.

Still need to debug the CI failure now.

@oleonardolima
Copy link
Contributor Author

oleonardolima commented Oct 8, 2025 •
edited
Loading

Still need to debug the CI failure now.

So, IIUC we can't have no-std for bdk_electrum, because we're using both Arc and Mutex from std::sync. I'm wondering why it didn't failed before 🤔.

Scratch that, I didn't notice it was only in the test module.

@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch 2 times, most recently from b4f3c30 to 0f5ac52 Compare October 8, 2025 00:48
@notmandatory
Copy link
Member

cACK, will do a deeper review when I get a chance but don't wait for me if you think it's ready to go.

@ValuedMammal ValuedMammal mentioned this pull request Oct 8, 2025
Merged
6 tasks
ValuedMammal
ValuedMammal approved these changes Oct 8, 2025
View reviewed changes
Copy link
Collaborator

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 0f5ac52

@oleonardolima
chore(msrv): bump MSRV to 1.85.0
8c15308 
- update all references to previous 1.63.0 MSRV.
- update the rust-version to 1.85.0.
- update the CI to not exclude bdk_electrum anymore.
- update the ci/pin-msrv.sh to 1.85.0.
@oleonardolima
fix(clippy): use is_none_or instead of map_or
6b67a34 
- use `is_non_or` instead of `map_or`
- update lifetime on `find_direct_anchor`
@oleonardolima
chore(deps): bump criterion to 0.7
7c189d0
@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch from 0f5ac52 to 9af94e0 Compare October 9, 2025 03:23
@oleonardolima
Copy link
Contributor Author

I had to do a rebase due to the ci/pin-msrv.sh conflict introduced by #2058.

@oleonardolima oleonardolima force-pushed the chore/bump-msrv-to-1.85.0 branch from 9af94e0 to 81ed4d0 Compare October 9, 2025 03:29
@oleonardolima
Copy link
Contributor Author

I've also updated the zizmor.yml as @ValuedMammal commented here: #2056 (comment)

ValuedMammal
ValuedMammal approved these changes Oct 9, 2025
View reviewed changes
Copy link
Collaborator

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 81ed4d0

@ValuedMammal
Copy link
Collaborator

ValuedMammal commented Oct 10, 2025 •
edited
Loading

There's a few additional cleanups: oleonardolima/bdk@chore/bump-msrv-to-1.85.0...ValuedMammal:review_2055

  • ci: drop actions-rs from clippy check
  • ci: Remove unneeded clippy: true from build-test
  • ci(coverage): Switch to actions-rust-lang/setup-rust-toolchain

📌 Look into bumping rust stable version to 1.90.0

thunderbiscuit
thunderbiscuit reviewed Oct 10, 2025
View reviewed changes
Copy link
Member

@thunderbiscuit thunderbiscuit left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small nit: the actions-rust-lang/setup-rust-toolchain@v1 sets the config value cache to true by default, but you need to know this about the action to understand that the cache is activated if the value is not set explicitly.

In some of your steps you configure it explicitly, and that's my preferred approach. It doesn't change anything but signals to contributors that the cache is active without requiring they know in depth the action.

I would personally set the field to true on all uses of the action (or remove it entirely if you prefer the implicit way), but at the moment you set it to true on some and don't set it on others, making it less obvious why/when the cache is active.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thunderbiscuit thunderbiscuit thunderbiscuit left review comments

@ValuedMammal ValuedMammal ValuedMammal approved these changes

@notmandatory notmandatory Awaiting requested review from notmandatory

@evanlinjin evanlinjin Awaiting requested review from evanlinjin

Assignees

@oleonardolima oleonardolima

Labels

chore Non-coding related work github_actions Pull requests that update GitHub Actions code

Projects

BDK Chain
Status: Needs Review

Milestone

Chain 0.24.0

Development

Successfully merging this pull request may close these issues.

Update MSRV to 1.85

5 participants

@oleonardolima @notmandatory @ValuedMammal @evanlinjin @thunderbiscuit