Skip to content

chore: bump MSRV to 1.85.0#317

Merged
ValuedMammal merged 8 commits intobitcoindevkit:masterfrom
ValuedMammal:chore/bump_msrv
Sep 29, 2025
Merged

chore: bump MSRV to 1.85.0#317
ValuedMammal merged 8 commits intobitcoindevkit:masterfrom
ValuedMammal:chore/bump_msrv

Conversation

@ValuedMammal
Copy link
Copy Markdown
Collaborator

@ValuedMammal ValuedMammal commented Sep 24, 2025 •
edited
Loading

Bump bdk_wallet MSRV to 1.85.0.

Restructure repository to be a single rust project.

Small cleanups to .github/workflows by moving away from unmaintained actions-rs/toolchain in favor of dtolnay/rust-toolchain.

Follow-ups

  • Address zizmor code scanning results
  • Use rust edition 2024. May have to remove some "unnecessary binding modifiers"
  • Consider updating .github/CODEOWNERS

fix #289

Notes to the reviewers

Changelog notice

Changed

- Raise project MSRV to 1.85.0
- ci: Migrate to `dtolnay/rust-toolchain`
- ci: Add zizmor.yml config

Checklists

All Submissions:

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 24, 2025
View reviewed changes
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 24, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 18041569734

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 85.211%
Totals Coverage Status
Change from base Build 17957725258: 0.4%
Covered Lines: 3895
Relevant Lines: 4571
💛 - Coveralls

@ValuedMammal ValuedMammal moved this to In Progress in BDK Wallet Sep 24, 2025
@ValuedMammal ValuedMammal added this to the Wallet 3.0.0 milestone Sep 24, 2025
@ValuedMammal ValuedMammal added the chore Non-coding related work label Sep 24, 2025
@ValuedMammal ValuedMammal self-assigned this Sep 24, 2025
thunderbiscuit
thunderbiscuit reviewed Sep 24, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Concept ACK. See comment on workspace. Good move away from the deprecated action-rs/rust-toolchain action.

@ValuedMammal
Rename example-crates directory
70560b5 
This is to avoid a clash with wallet/examples when
the workspace goes away.
@ValuedMammal
Update justfile
a489e35
thunderbiscuit
thunderbiscuit reviewed Sep 25, 2025
View reviewed changes
Copy link
Copy Markdown
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.

I'm personally not a fan of the "pin the hash on the actions" that zizmor recommends here.

If you want to address the zizmor warning you're getting here, you could do what we do for bdk-ffi. You add a file called zizmor.yml at the root of the .github/ directory with the following:

# This is a configuration file for the zizmor action defined in .github/workflows/zizmor.yaml

rules:
  # Disable the rule requiring all actions be pinned to a specific hash
  unpinned-uses:
    config:
      policies:
        "*": any

thunderbiscuit
thunderbiscuit approved these changes Sep 25, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Tested ACK 6260274.

I'm getting one little clippy warning locally:

error: the following explicit lifetimes could be elided: 'a
    --> src/wallet/mod.rs:1192:25
     |
1192 |     pub fn transactions<'a>(&'a self) -> impl Iterator<Item = WalletTx<'a>> + 'a {
     |                         ^^   ^^                                        ^^     ^^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
     = note: `-D clippy::needless-lifetimes` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::needless_lifetimes)]`
help: elide the lifetimes
     |
1192 -     pub fn transactions<'a>(&'a self) -> impl Iterator<Item = WalletTx<'a>> + 'a {
1192 +     pub fn transactions(&self) -> impl Iterator<Item = WalletTx<'_>> + '_ {

But this is related to my default toolchain being 1.85. The warning disappears if I build using 1.89. See #315 for my recommendation on how to fix that sort of issue. Not required for this PR, just pointing it out.

@thunderbiscuit
Copy link
Copy Markdown
Member

thunderbiscuit commented Sep 25, 2025

Also note that I think the rust-version file was there to help the actions-rs/toolchain action (that's a file it uses to decide on rust compiler version). Since you've moved away from this action it might be possible to remove the file entirely (or at least we should know why we need to keep it if another tool/workflow/dev is using it).

@ValuedMammal
Copy link
Copy Markdown
Collaborator Author

ValuedMammal commented Sep 25, 2025

In 2a5c492 I added an exception to the unpinned-uses rule specifically for dtolnay/rust-toolchain. One slight problem is that the upstream repo only has one v1 tag that seems to be continually overwritten (i.e. insecure from a code scanning perspective). We could pin it to a hash but that would require more hands-on involvement if we ever want to update it. If there's alternatives to dtolnay for installing a rust toolchain we could explore that too.

@oleonardolima oleonardolima mentioned this pull request Sep 26, 2025
Closed
@ValuedMammal ValuedMammal marked this pull request as ready for review September 26, 2025 14:04
@ValuedMammal ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Sep 26, 2025
ValuedMammal
ValuedMammal commented Sep 26, 2025
View reviewed changes
thunderbiscuit
thunderbiscuit approved these changes Sep 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK eceb1a3. Nice cleanup.

ValuedMammal
ValuedMammal commented Sep 29, 2025
View reviewed changes
README.md

<div align="center">
<h1>BDK Wallet</h1>
<h1>BDK</h1>
Copy link
Copy Markdown
Collaborator Author

@ValuedMammal ValuedMammal Sep 29, 2025

Choose a reason for hiding this comment

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

📌 Accidentally clobbered when I merged the two READMEs together. Same on line 4.

@ValuedMammal ValuedMammal merged commit e2cf34d into bitcoindevkit:master Sep 29, 2025
16 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in BDK Wallet Sep 29, 2025
@ValuedMammal ValuedMammal deleted the chore/bump_msrv branch September 29, 2025 14:18
@ValuedMammal ValuedMammal mentioned this pull request Mar 4, 2026
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thunderbiscuit thunderbiscuit thunderbiscuit approved these changes

Assignees

@ValuedMammal ValuedMammal

Labels

chore Non-coding related work

Projects

BDK Wallet
Archived in project

Milestone

Wallet 3.0.0

Development

Successfully merging this pull request may close these issues.

Upgrade MSRV to 1.85

4 participants

@ValuedMammal @coveralls @thunderbiscuit @github-advanced-security