Skip to content

Update Wallet to 2.4.0 and other deps#248

Open
tvpeter wants to merge 4 commits into
bitcoindevkit:masterfrom
tvpeter:chore/update-wallet-to-2_3
Open

Update Wallet to 2.4.0 and other deps#248
tvpeter wants to merge 4 commits into
bitcoindevkit:masterfrom
tvpeter:chore/update-wallet-to-2_3

Conversation

@tvpeter

@tvpeter tvpeter commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR updates the Wallet API to v2.4.0 and other dependencies. It also adds WalletEvent to both sync and full_scan for all backends.

Fixes #243

Changelog notice

  • Update bdk_wallet to v2.4.0
  • Add WalletEvents to all backends Esplora, Electrum, Rpc and CBF.
  • Add print_wallet_events function to print wallet events to the terminal during sync
  • Update bdk_bitcoind_rpc to v0.22.0
  • Update bdk_electrum to v0.24.0
  • Update bdk_kyoto to v0.15.4
  • Update bdk_esplora to v0.22.2

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@tvpeter

tvpeter commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator Author

@notmandatory, I’m wondering if we should update our signing approach since the current method is deprecated. If we choose to proceed with the update, we will need to add miniscript as a separate dependency, as the version exported from the Wallet (v2.3.0) is 12.3.1. We require at least version 13.0.0. Or should we postpone this update until a later version?

@notmandatory

notmandatory commented Mar 13, 2026

Copy link
Copy Markdown
Member

@notmandatory, I’m wondering if we should update our signing approach since the current method is deprecated. If we choose to proceed with the update, we will need to add miniscript as a separate dependency, as the version exported from the Wallet (v2.3.0) is 12.3.1. We require at least version 13.0.0. Or should we postpone this update until a later version?

Good question. I've created bitcoindevkit/bdk_wallet#405 to revert the deprecating of the signer mod. The new PSBT signer probably won't be available until bdk_wallet 3.1.

@va-an

va-an commented Mar 16, 2026

Copy link
Copy Markdown

Related #243.

@va-an va-an left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocked by #252.

Comment thread src/handlers.rs Outdated
Comment thread src/handlers.rs Outdated
@tvpeter

tvpeter commented Mar 17, 2026

Copy link
Copy Markdown
Collaborator Author

Blocked by #252.

Hi @va-an, could you please verify that you tagged the correct PR? Thank you!

@va-an

va-an commented Mar 17, 2026

Copy link
Copy Markdown

Blocked by #252.

Hi @va-an, could you please verify that you tagged the correct PR? Thank you!

All correct. I've also reproduced error from PR's CI build in my local run just pre-push:

error[E0061]: this method takes 1 argument but 2 arguments were supplied
    --> src/payjoin/mod.rs:605:30
     |
 605 |   ...                   .check_payment(
     |                          ^^^^^^^^^^^^^
...
 618 | / ...                       |outpoint| {
 619 | | ...                           let utxo = self.wallet.get_utxo(outpoint);
 620 | | ...                           match utxo {
 621 | | ...                               Some(_) => Ok(false),
...    |
 624 | | ...                       }
     | |___________________________- unexpected argument #2 of type `{closure@src/payjoin/mod.rs:618:33: 618:43}`
     |
note: method defined here
    --> /home/vaan/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/payjoin-1.0.0-rc.2/src/core/receive/v2/mod.rs:1262:12
     |
1262 |     pub fn check_payment(
     |            ^^^^^^^^^^^^^
help: remove the extra argument
     |
 617 -                                 },
 618 -                                 |outpoint| {
 619 -                                     let utxo = self.wallet.get_utxo(outpoint);
 620 -                                     match utxo {
 621 -                                         Some(_) => Ok(false),
 622 -                                         None => Ok(true),
 623 -                                     }
     |

I've applied the fix from #252 to my local code to verify that it helps.

@va-an

va-an commented Mar 31, 2026

Copy link
Copy Markdown

@tvpeter #252 is merged now, so this should be fine now.
Could you rebase on master? Other PRs will likely need to rebase on top of this once it's merged.

tvpeter added 2 commits March 31, 2026 21:32
- Add wallet events to full_scan and sync
subcommands
- update bdk_bitcoind_rpc to v0.22.0
- update bdk_electrum to v0.23.2
- update bdk_kyoto to v0.15.4
@tvpeter tvpeter force-pushed the chore/update-wallet-to-2_3 branch from f7817a9 to bfdabf7 Compare March 31, 2026 20:43
@codecov

codecov Bot commented Mar 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 11.38%. Comparing base (07fd32f) to head (5def41f).
⚠️ Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
src/utils.rs 0.00% 44 Missing ⚠️
src/handlers.rs 0.00% 39 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #248      +/-   ##
==========================================
+ Coverage   11.13%   11.38%   +0.24%     
==========================================
  Files           8        8              
  Lines        2488     2803     +315     
==========================================
+ Hits          277      319      +42     
- Misses       2211     2484     +273     
Flag Coverage Δ
rust 11.38% <0.00%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@va-an

va-an commented Apr 1, 2026

Copy link
Copy Markdown

Thanks for the rebase, @tvpeter!

Unfortunately, CI is now failing because bdk_wallet 2.3.0 introduced a deprecation that causes warnings-as-errors to fail. The revert has already been merged but 2.4.0 hasn't been released yet.

Pinning back to 2.1.0 won't work either - this branch already uses apply_update_events which was introduced in 2.3.0, so downgrading breaks the build:

error[E0599]: no method named `apply_update_events` found for mutable reference `&mut Wallet`

So it looks like we're stuck waiting for 2.4.0 to be released. Until then, CI will fail on this PR (and on any PR that rebases after merging this one).
We could just keep this in mind during reviews and merges - the CI failure is a known issue, not a code problem.

@va-an va-an left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

utACK 4a41057

Comment thread src/handlers.rs
Comment on lines +1541 to +1545
let wallet_events = wallet
.apply_update_events(update)
.map_err(|e| Error::Generic(e.to_string()))?;
print_wallet_events(&wallet_events);
Ok(())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We could use tap for convenience:

Suggested change
let wallet_events = wallet
.apply_update_events(update)
.map_err(|e| Error::Generic(e.to_string()))?;
print_wallet_events(&wallet_events);
Ok(())
wallet
.apply_update_events(update)
.map_err(|e| Error::Generic(e.to_string()))?
.tap(print_wallet_events);
Ok(())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While this will make it cleaner, I think it is not worth the cost of pulling in a whole library. I'm always for using a minimal number of external libraries.

Comment thread src/utils.rs Outdated
Ok((wallet_opts, network))
}
#[cfg(any(feature = "electrum", feature = "esplora",))]
pub fn print_wallet_events(events: &Vec<WalletEvent>) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
pub fn print_wallet_events(events: &Vec<WalletEvent>) {
pub fn print_wallet_events(events: &[WalletEvent]) {

@tvpeter

tvpeter commented Apr 1, 2026

Copy link
Copy Markdown
Collaborator Author

Unfortunately, CI is now failing because bdk_wallet 2.3.0 introduced a deprecation that causes warnings-as-errors to fail.

Yes, I updated the warnings to be treated as errors in this commit 8d20cf2 because many Clippy warnings were being neglected and had to be merged into master. It's not coming from bdk_wallet

@va-an

va-an commented Apr 1, 2026

Copy link
Copy Markdown

Yes, I updated the warnings to be treated as errors in this commit 8d20cf2 because many Clippy warnings were being neglected and had to be merged into master. It's not coming from bdk_wallet

The deny(warnings) change is great, glad you added it - lax warnings tend to pile up fast. The issue here is a different thing though: bdk_wallet 2.3.0 itself emits a deprecation warning on its own API (apply_update), and that warning hits the deny(warnings) flag. It'll be resolved once 2.4.0 is released (bitcoindevkit/bdk_wallet#405).

@notmandatory

Copy link
Copy Markdown
Member

Unfortunately, CI is now failing because bdk_wallet 2.3.0 introduced a deprecation that causes warnings-as-errors to fail. The revert has already been merged but 2.4.0 hasn't been released yet.

The Wallet 2.4.0 lib was just published and should fix this issue

@tvpeter tvpeter added this to the CLI V4.0.0 milestone May 6, 2026
@tvpeter tvpeter moved this to In Progress in BDK-CLI May 6, 2026
- add WalletEvent to rpc and cbf clients
- Update Wallet to v2.4.0
@tvpeter tvpeter force-pushed the chore/update-wallet-to-2_3 branch from 5dc6a28 to 5def41f Compare June 28, 2026 18:56
@tvpeter tvpeter changed the title Update Wallet to 2.3.0 and other deps Update Wallet to 2.4.0 and other deps Jun 28, 2026
@tvpeter tvpeter moved this from In Progress to Ready to Review in BDK-CLI Jun 28, 2026
@tvpeter tvpeter requested a review from va-an June 29, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready to Review

Development

Successfully merging this pull request may close these issues.

Update out of date dependencies

3 participants