Skip to content

3.0#971

Open
reez wants to merge 10 commits intobitcoindevkit:masterfrom
reez:rc1
Open

3.0#971
reez wants to merge 10 commits intobitcoindevkit:masterfrom
reez:rc1

Conversation

@reez
Copy link
Copy Markdown
Collaborator

@reez reez commented Mar 11, 2026

Description

3.0 draft. WIP.

Notes to the reviewers

Documentation

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added a changelog in the next release tracking issue (see example)
  • I've linked the relevant upstream docs or specs above

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 changed the title (DRAFT) Rc1 (DRAFT) 3.0 Rc1 Mar 11, 2026
@thunderbiscuit
Copy link
Copy Markdown
Member

🚀 🚀 🚀 🚀 🚀

@reez
Copy link
Copy Markdown
Collaborator Author

reez commented Mar 17, 2026

bitcoindevkit/bdk-kyoto#155 I opened this Draft PR (and a related issue)

Just using my fork of bdk-kyoto with the change rn

@reez reez changed the title (DRAFT) 3.0 Rc1 (DRAFT) 3.0 RC's Mar 27, 2026
@reez reez linked an issue Mar 30, 2026 that may be closed by this pull request
@reez
Copy link
Copy Markdown
Collaborator Author

reez commented Apr 13, 2026

updated to official 3.0

@rustaceanrob
Copy link
Copy Markdown
Collaborator

bdk_kyoto = 0.16.0 is out and includes the 3.0.0 release

@reez
Copy link
Copy Markdown
Collaborator Author

reez commented Apr 14, 2026

bdk_kyoto = 0.16.0 is out and includes the 3.0.0 release

thanks I updated to 0.16.0 and made one small change update_rx: Mutex<UpdateSubscriber<bdk_kyoto::wallets::Single>> necessary along with it in 27be17f

@reez reez changed the title (DRAFT) 3.0 RC's (DRAFT) 3.0 Apr 14, 2026
Copy link
Copy Markdown
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.

I compiled and ran Rust tests locally today. Just started looking at this and left a few comments. I haven't gone through all files yet (tx_builder, types, wallet left to do). I'll try and migrate the Android app to this potential alpha to see how the migration goes! Looks like it should be pretty smooth.

Comment thread bdk-ffi/Cargo.toml Outdated
[package]
name = "bdk-ffi"
version = "2.4.0-alpha.0"
version = "3.0.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one here I think should read 3.0.0-alpha.0 since this would merge on master.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ahhh, I guess I did no -alpha just because in my head I was thinking we only bumped to -alpha after the release work was merged to master and the release was cut, but I might have been remembering that wrong.

No strong opinion from me, just wanted to double check w ya, but im good with whatever you comment back on this for me to change it to.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(go w thunders suggestion here)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated w commit 061266b

Comment thread bdk-ffi/src/descriptor.rs
let secp = Secp256k1::new();
let (extended_descriptor, key_map) = descriptor.into_wallet_descriptor(&secp, network)?;
let (extended_descriptor, key_map) =
descriptor.into_wallet_descriptor(&secp, network.into())?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still need to get to grips with the NetworkKind type. Are there situations where we'd like to use that instead of the Network for example? Not sure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don’t have any concrete ffi facing cases that come to mind, since NetworkKind only has Main and Test... I’ve been thinking of it as mostly an internal/upstream type for apis that only care about that distinction, where bdk-ffi consumers would usually want the full Network to preserve the actual chain. But I need to keep thinking about it more too so im glad you commented on it here

Comment thread bdk-ffi/src/store.rs
#[derive(Debug, Clone, uniffi::Record)]
pub struct PreV1WalletKeychain {
/// The wallet keychain.
pub keychain: bdk_wallet::KeychainKind,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use our own KeychainKind from the types module here? I think they're functionally equivalent because it's using remote enums in uniffi, but could still be a tiny cleanup unless I am missing something.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think the small uniffi wrinkle i found here was crate::types::KeychainKind isn’t currently usable from this module because the backing alias in types.rs is private so switching this line wasn't a easy one line cleanup, and since the rest of the code was also using bdk_wallet::KeychainKind directly I decided to leave it as is in this PR.

But its definitely something we could do, it just seemed like it might end up needing to be a separate/bigger refactor

Comment thread bdk-ffi/src/store.rs
}

/// Retrieve keychain metadata from a pre-v1 BDK SQLite wallet database.
pub fn get_pre_v1_wallet_keychains(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd love to see a test for this one in the Android or Swift parts of the codebase if possible.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that makes sense. I looked around the code and I think we already have Swift/Android persistence tests but I don’t think we had a pre-v1 SQLite database in the repo, so adding a bindings test for this I think would have meant adding that first. And then also I wasn't totally sure if we were going to expose this in 3.0 or not.

But im with you on if we do want to expose this for sure in our ffi 3.0 (or whenever we expose it in a release) that it would be quite nice to have a test for this, and prob worth putting in time to figure out how to test it well

Comment thread bdk-ffi/src/store.rs

/// Metadata describing a keychain in a pre-v1 BDK SQLite wallet database.
#[derive(Debug, Clone, uniffi::Record)]
pub struct PreV1WalletKeychain {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was looking at the Rust code for this type and I see that the API docs have changed since you added it I think. See here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is deep in the weeds, I love it, good catch!

updated in 6cca30e

@reez reez changed the title (DRAFT) 3.0 3.0 Apr 20, 2026
@reez reez marked this pull request as ready for review April 20, 2026 17:19
@reez
Copy link
Copy Markdown
Collaborator Author

reez commented Apr 20, 2026

@thunderbiscuit I rebased onto current master and dropped commits superseded by #978

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

bdk_wallet 3.0

3 participants