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

Bump bdk_wallet to alpha 13 #561

Merged
merged 11 commits into from
Jul 1, 2024
Merged

Conversation

thunderbiscuit
Copy link
Member

@thunderbiscuit thunderbiscuit commented Jun 19, 2024

This is one of our biggest bumps on the alpha train. Take your time with the review, there is just so much. I'm sorry I wasn't really able to break it into many smaller commits, it was all a jungle of changes that needed to happen together.

Notes for reviewers

  • The Wallet constructors now require both an external and internal descriptor. I have pulled those up as private variables in all tests. I think it might make sense to have them all use the same variable defined maybe in a separate file rather than having the exact same descriptor defined in all tests files, but that's for another PR.
  • A lot of errors in Rust are simply called Error, and you know what type of error they are from their fully qualified name (for example bdk_wallet::bitcoin::address::Error is an error tied to the Address type. This metadata available to Rust users is erased when building the bindings because all our errors exist as top-level errors. This means that at face value we'd have a ton of specific errors just called Error. When these happen, I instead try to rename the error using part of the fully qualified name so users of the bindings have a better idea what those errors are at first glance; but this means there is a bit of art/subjectivity to it rather than pure exposing of the exact same name. Feel free to comment on my choice of names for the errors.
  • A lot of our constructs use a tuple struct construction, with a single field that contains the Rust BDK type we need to operate on. The inner field is never necessary for outside users, and should not be visible. A commit on this PR fixes a few places where the inner struct was public but did not need to be.
  • We lost the Address.network() method with bitcoin 0.32. I don't know what's going on with this type honestly, they just keep changing it. I can't remember exactly how that might impact the workflow Jurvis was telling us about, but the method does not exist on the type anymore, so I assume there must be other ways to achieve the same security as what he was intending to do. Will need to confirm.
  • Lots of changes in the errors, including the removal of some of the Rust tests that checked whether the error was printing the exact message given to the thiserror macro. We now know they do, and keeping those tests up to date was more pain than they were worth IMO, so in cases where the errors changed I simply removed the tests. We should probably consider removing them entirely at some point. Again out-of-scope for this PR.

Description

This PR bumps the Rust libraries to the alpha 13 release.

Changelog notice

Added:
    - Bump bdk_wallet to alpha 13 [#561]
    - Bump bdk_esplora to 0.15.0 [#561]
    - Bump bdk_electrum to 0.15.0 [#561]
    - Bump bdk_sqlite to 0.2.0 [#561]
    - Bump bdk_bitcoind_rpc to 0.12.0 [#561]
    - New `FromScriptError` [#561]
    - New type `ChangeSet` [#561]
    - Wallet constructors do not take a persistence path anymore [#561]
    - `Wallet.get_balance()` method renamed to `balance()` [#561]
    - Removed the `Wallet.commit()` method [#561]
    - Added `Wallet.take_staged()` [#561]
    - Added `SqliteStore` type [#561]

Changed
    - `AddressError` is replaced by `AddressParseError` [#561]
    - New variants in `CalculateFeeError` [#561]
    - New variants in `CreateTxError` [#561]
    - New variants in `ParseAmountError` [#561]
    - New variants in `SignerError` [#561]
    - New variants in `WalletCreationError` [#561]
    - `Wallet.calculate_fee()` returns an `Amount` [#561]
    - Renamed `Transaction.txid()` to `Transaction.compute_txid()` [#561]

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

Checklists

All Submissions:

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

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@thunderbiscuit thunderbiscuit force-pushed the chore/alpha13 branch 4 times, most recently from acefa76 to 542ac66 Compare June 20, 2024 18:22
@thunderbiscuit thunderbiscuit mentioned this pull request Jun 24, 2024
3 tasks
@thunderbiscuit thunderbiscuit force-pushed the chore/alpha13 branch 2 times, most recently from 0420b95 to 2c70892 Compare June 24, 2024 14:13
@thunderbiscuit thunderbiscuit force-pushed the chore/alpha13 branch 2 times, most recently from 5bc6483 to 7ed3366 Compare June 26, 2024 20:06
@thunderbiscuit thunderbiscuit requested a review from reez June 26, 2024 20:19
@reez
Copy link
Collaborator

reez commented Jul 1, 2024

Tested ACK 323eb08

Incredible job thunder, this was a big one!

@reez reez merged commit 323eb08 into bitcoindevkit:master Jul 1, 2024
25 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.

2 participants