-
Notifications
You must be signed in to change notification settings - Fork 114
Upgrade to BDK 2.0 #551
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.
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
Upgrade to BDK 2.0 #551
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
4a7abc7 to
3030b0c
Compare
|
|
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.
3030b0c to
e6fa232
Compare
Thanks for letting me know! Updated the branch here. |
| ) -> Result<(), Error> { | ||
| let mut locked_wallet = self.inner.lock().unwrap(); | ||
| locked_wallet.apply_unconfirmed_txs(unconfirmed_txs); | ||
| locked_wallet.apply_evicted_txs(evicted_txids); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
|
FYI, |
Yeah, unfortunately has been flaky in github for a while and I haven't gotten to fixing it yet, see #527. |
We upgrade BDK to the latest release.
Draft for now as we have yet to wait for the actual BDK 2.0.0.