Skip to content
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

Update bdk_wallet to alpha 12 #544

Merged

Conversation

thunderbiscuit
Copy link
Member

@thunderbiscuit thunderbiscuit commented May 28, 2024

This PR upgrades the (newly named!) bdk_wallet library to the alpha 12 version.

Also part of this are the upgrade to bdk_esplora (0.14.0), bdk_electrum (0.14.0), and the introduction of the new bdk_sqlite (0.1.0) persistence crate.

Note:

  1. The diff is pretty big because all the imports using bdk are now switched to bdk_wallet.
  2. I changed the names of the persistence files in the tests to .sqlite, as is common for those types of files (but is not required). This is also the naming used on the Rust bdk side, so I think it's a good fit anyway.
  3. The core upgrade here for the sqlite persistence is surprisingly small: lines 41-42 in the wallet module.
  4. I added the SqliteError type which is not used yet (should I remove it and wait to add it when we have a need?). The current places where the codebase can throw a sqlite error is on the wallet creation method, but this one can only throw one of the variants of the BdkSqliteError, the one where it's a wrapper for the rusqlite error. So I added that as a variant on the WalletCreationError (that's required), but I also added just the normal bdk Sqlite error just so this PR has it and we can use it in the future.

Changelog notice

Upgrades
  - Update bdk_esplora client to 0.14.0 [#544]
  - Update bdk_electrum client to 0.14.0 [#544]

Added
  - bdk_sqlite persistence [#544]

Removed
  - flat file persistence [#544]

[#544]: https://github.com/bitcoindevkit/bdk-ffi/pull/544

Checklists

All Submissions:

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

@thunderbiscuit thunderbiscuit force-pushed the chore/bump-alpha12 branch 2 times, most recently from 8fb3f3f to 0a0aa10 Compare June 4, 2024 13:10
This commit also introduces the sqlite store and removes the flat file
 store
@thunderbiscuit thunderbiscuit requested a review from reez June 4, 2024 13:25
@thunderbiscuit thunderbiscuit force-pushed the chore/bump-alpha12 branch 2 times, most recently from b78dd95 to ea0f0ab Compare June 4, 2024 14:37
@thunderbiscuit thunderbiscuit self-assigned this Jun 4, 2024
@@ -296,7 +296,7 @@ impl TxBuilder {
})
}

pub(crate) fn fee_absolute(&self, fee_amount: u64) -> Arc<Self> {
pub(crate) fn fee_absolute(&self, fee_amount: Arc<Amount>) -> Arc<Self> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

love the new Amount type here and other places, v cool

@@ -606,7 +618,7 @@ pub enum TxidParseError {
InvalidTxid { txid: String },
}

// This error combines the Rust bdk::wallet::NewOrLoadError and bdk_file_store::FileError
// This error combines the Rust bdk::wallet::NewOrLoadError and bdk_wallet::rusqlite::Error
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for even remembering to update these spots, you're on it!

fn from(
tx: BdkCanonicalTx<'_, Arc<bdk::bitcoin::Transaction>, ConfirmationTimeHeightAnchor>,
) -> Self {
impl From<BdkCanonicalTx<'_, Arc<BdkTransaction>, ConfirmationTimeHeightAnchor>> for CanonicalTx {
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for doing things like this in this PR so we can have things like Arc<BdkTransaction> instead of Arc<bdk::bitcoin::Transaction>, makes it clean and I need to make sure I'm paying attention to this sort of stuff to for any of my new PR's

@reez
Copy link
Collaborator

reez commented Jun 5, 2024

ACK 65702f4

Really good job!

for :

I added the SqliteError type which is not used yet (should I remove it and wait to add it when we have a need?). The current places where the codebase can throw a sqlite error is on the wallet creation method, but this one can only throw one of the variants of the BdkSqliteError, the one where it's a wrapper for the rusqlite error. So I added that as a variant on the WalletCreationError (that's required), but I also added just the normal bdk Sqlite error just so this PR has it and we can use it in the future.

...I'm good with either decision. In testing on the iOS app i did happen to run into the variant on the WalletCreationError so that was nice.

I tested the bindings by building them locally and then using them on the BDK iOS app, and made some slight adjustments to make it transition smoothly on the iOS client just in case anyone is interested in taking a peek at that: bitcoindevkit/BDKSwiftExampleWallet#146

@thunderbiscuit thunderbiscuit merged commit 65702f4 into bitcoindevkit:master Jun 6, 2024
25 checks passed
@thunderbiscuit thunderbiscuit deleted the chore/bump-alpha12 branch June 6, 2024 01:17
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.

2 participants