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

deps: bump bdk to beta_5 #600

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Conversation

reez
Copy link
Collaborator

@reez reez commented Oct 4, 2024

Description

This pull request updates the bdk-ffi crate to use the latest beta version of bdk, specifically version 1.0.0-beta.5.

The main changes include:

  • Updating the Cargo.toml file.
  • Adjusting the RBF functionality to align with the changes in the new BDK version.
    • Removing deprecated methods like enable_rbf() and enable_rbf_with_sequence().
    • Introducing the new set_exact_sequence method to replace the old RBF-related methods
  • Updating errors, particularly for the RbfSequenceCsv error variant.

Notes to the reviewers

Changelog notice

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

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

@reez reez force-pushed the beta-5 branch 2 times, most recently from 874a522 to 280eecb Compare October 4, 2024 01:35
@reez reez marked this pull request as ready for review October 4, 2024 13:02
Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Very nice cleanup.

I have one tiny thing I'd like to clean up before merging, but that's it.

Nice work; I like the removal of the explicit rbf setter, and we're probably the first ones to do that by default!

bdk_esplora = { version = "0.18.0", default-features = false, features = ["std", "blocking", "blocking-https-rustls"] }
bdk_electrum = { version = "0.18.0", default-features = false, features = ["use-rustls-ring"] }
bdk_bitcoind_rpc = { version = "0.15.0" }
bdk_wallet = { version = "1.0.0-beta.5", features = ["all-keys", "keys-bip39", "rusqlite"] }
Copy link
Member

Choose a reason for hiding this comment

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

Here I think we should pin the bdk_wallet dependency to beta 5. The default cargo behaviour of updating to the latest patch broke my (and all of our) builds as of yesterday when they released beta 5 which contained a breaking change. This is not a bdk problem but really a cargo problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I pushed an update to pin, but just double check its what you were asking for and I won't merge or anything until you give it the go ahead that this is good-

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Sorry I chose "Approve" when I should have clicked "Request changes".

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK 6cfcf23. Ready to merge.

@thunderbiscuit thunderbiscuit merged commit 6cfcf23 into bitcoindevkit:master Oct 7, 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