Skip to content

Conversation

@tnull
Copy link
Collaborator

@tnull tnull commented Jun 3, 2025

We upgrade BDK to the latest release.

Draft for now as we have yet to wait for the actual BDK 2.0.0.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jun 3, 2025

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tnull tnull marked this pull request as draft June 3, 2025 09:40
@tnull tnull force-pushed the 2025-06-upgrade-to-bdk-2.0 branch 2 times, most recently from 4a7abc7 to 3030b0c Compare June 3, 2025 09:56
@notmandatory
Copy link
Contributor

notmandatory commented Jun 5, 2025

bdk_wallet 2.0 is now published, see release page for compatible bdk chain client versions: https://github.com/bitcoindevkit/bdk_wallet/releases/tag/wallet-2.0.0

tnull added 2 commits June 6, 2025 09:22
With version 2.0, BDK introduced a new `Wallet::apply_evicted_txs` API
that requires that we tell the `Wallet` about any mempool evictions.

Here, we implement this as part of our mempool polling, which should fix
a bug that left the wallet unable to spend funds if
previously-canoncical mempool transactions ended up being evicted.
@tnull tnull force-pushed the 2025-06-upgrade-to-bdk-2.0 branch from 3030b0c to e6fa232 Compare June 6, 2025 07:23
@tnull
Copy link
Collaborator Author

tnull commented Jun 6, 2025

bdk_wallet 2.0 is now published, see release page for compatible bdk chain client versions: https://github.com/bitcoindevkit/bdk_wallet/releases/tag/wallet-2.0.0

Thanks for letting me know! Updated the branch here.

@tnull tnull marked this pull request as ready for review June 6, 2025 07:23
@tnull tnull requested a review from jkczyz June 6, 2025 07:23
) -> Result<(), Error> {
let mut locked_wallet = self.inner.lock().unwrap();
locked_wallet.apply_unconfirmed_txs(unconfirmed_txs);
locked_wallet.apply_evicted_txs(evicted_txids);
Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't clear from the bdk_wallet docs or release page that this is a new requirement. When upgrading, how do you typically see what changes are needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case we knew that we were waiting on exactly that change, but I also went through all the commit history of all the BDK crates since the last upgrade to double-check I'm not overlooking something fundamental. So yeah, I agree the changelog could do a better job listing all the changes that need to be considered (cc @notmandatory).

@jkczyz
Copy link
Contributor

jkczyz commented Jun 6, 2025

FYI, check-cln is failing

@tnull
Copy link
Collaborator Author

tnull commented Jun 9, 2025

FYI, check-cln is failing

Yeah, unfortunately has been flaky in github for a while and I haven't gotten to fixing it yet, see #527.

@tnull tnull merged commit ede415f into lightningdevkit:main Jun 9, 2025
17 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants