feat: expose `Wallet::keychains` by reez · Pull Request #1031 · bitcoindevkit/bdk-ffi

reez · 2026-06-17T00:17:16Z

Description

Exposes Wallet::keychains so users can inspect each wallet keychain and its public descriptor

Notes to the reviewers

Documentation

bdk_wallet

https://docs.rs/bdk_wallet/3.0.0/src/bdk_wallet/wallet/mod.rs.html#592-594

Changelog

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 exactly one changelog:* label
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

thunderbiscuit

This is great! I'm surprised we didn't have this one in before.

My one suggestion would be to consider returning the full Descriptor type instead of a string. This would allow users to then call methods like Descriptor::descriptor_id and Descriptor::desc_type and whatnot.

The trick is to populate the key_map type with an empty keymap, which is what happens on the constructors of Descriptor when the provided string is a public descriptor. I convinced myself of this by hacking on the Descriptor::new method and adding things like

eprintln!("The keymap at this point is: {:?}", &key_map);
if key_map.is_empty() {
    eprintln!("####### The key_map is empty! #######")
}

Trying this locally and fixing the tests, I realized that our current method Wallet::public_descriptor does return a string, but rather than matching this behaviour I would suggest we modify the Wallet::public_descriptor to return the full type on our next major release (it'd be a breaking change).

Tests had to be modified:

    assert_eq!(
        external.public_descriptor.to_string(),
        wallet.public_descriptor(KeychainKind::External).to_string()
    );
    assert_eq!(
        internal.public_descriptor.to_string(),
        wallet.public_descriptor(KeychainKind::Internal).to_string()
    );
    assert!(!external.public_descriptor.to_string().contains("tprv"));
    assert!(!internal.public_descriptor.to_string().contains("tprv"));

I would also suggest we derive/write the Display trait for this new struct.

/// A wallet keychain and its public descriptor.
#[derive(uniffi::Record)]
pub struct WalletKeychain {
    /// Type of keychain.
    pub keychain: KeychainKind,
    /// Public descriptor for the keychain.
    pub public_descriptor: Arc<Descriptor>,
}

thunderbiscuit · 2026-06-17T19:53:02Z

+    /// Type of keychain.
+    pub keychain: KeychainKind,
+    /// Public descriptor for the keychain.
+    pub public_descriptor: String,


Suggestion: we could use the full Descriptor type here (expanded upon in my main comment).

totally, I can't believe I still do this old String thing sometimes, good call out, should be fixed now

thunderbiscuit · 2026-06-17T19:53:59Z

+            .keychains()
+            .map(|(keychain, descriptor)| WalletKeychain {
+                keychain,
+                public_descriptor: descriptor.to_string(),


These lines would become

.map(|(keychain, descriptor)| WalletKeychain { keychain, public_descriptor: Arc::new(Descriptor { extended_descriptor: descriptor.clone(), key_map: KeyMap::default(), }), })

yes sir, updated 👍

reez · 2026-06-17T20:22:17Z

This is great! I'm surprised we didn't have this one in before.

My one suggestion would be to consider returning the full Descriptor type instead of a string. This would allow users to then call methods like Descriptor::descriptor_id and Descriptor::desc_type and whatnot.

The trick is to populate the key_map type with an empty keymap, which is what happens on the constructors of Descriptor when the provided string is a public descriptor. I convinced myself of this by hacking on the Descriptor::new method and adding things like
eprintln!("The keymap at this point is: {:?}", &key_map);

if key_map.is_empty() {

    eprintln!("####### The key_map is empty! #######")

}
Trying this locally and fixing the tests, I realized that our current method Wallet::public_descriptor does return a string, but rather than matching this behaviour I would suggest we modify the Wallet::public_descriptor to return the full type on our next major release (it'd be a breaking change).

Tests had to be modified:
    assert_eq!(

        external.public_descriptor.to_string(),

        wallet.public_descriptor(KeychainKind::External).to_string()

    );

    assert_eq!(

        internal.public_descriptor.to_string(),

        wallet.public_descriptor(KeychainKind::Internal).to_string()

    );

    assert!(!external.public_descriptor.to_string().contains("tprv"));

    assert!(!internal.public_descriptor.to_string().contains("tprv"));
I would also suggest we derive/write the Display trait for this new struct.
/// A wallet keychain and its public descriptor.

#[derive(uniffi::Record)]

pub struct WalletKeychain {

    /// Type of keychain.

    pub keychain: KeychainKind,

    /// Public descriptor for the keychain.

    pub public_descriptor: Arc<Descriptor>,

}

Yes! Great call. Let me update Pr for them

reez · 2026-06-17T23:46:17Z

All great comments @thunderbiscuit , v good review, I think I fixed them all so this should be good to go (other than rebase, I've got a bunch of prs to rebase though and not sure which order I'll merge them in so I'll wait to rebase this atm)

thunderbiscuit

Just one last little thing.

thunderbiscuit · 2026-06-18T13:12:15Z


+/// A wallet keychain and its public descriptor.
+#[derive(uniffi::Record)]
+pub struct WalletKeychain {


The Display trait is defined on line 221 but not actually exported to the bindings. We need to add #[uniffi::export(Display)].

In this case for Kotlin the WalletKeychain type is a data class and so gets a toString() implementation for free so we get a nice display, but it's not exactly the string you defined in the Display trait implementation on line 221, and I don't think all languages produce this sort of automated toString for simple data types like this one.

// Without exporting Display [WalletKeychain(keychain=EXTERNAL, publicDescriptor=wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu), WalletKeychain(keychain=INTERNAL, publicDescriptor=wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/1/*)#n4cuwhcy)] // After exporting Display [External: wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/0/*)#zpaanzgu, Internal: wpkh([9122d9e0/84'/1'/0']tpubDCYVtmaSaDzTxcgvoP5AHZNbZKZzrvoNH9KARep88vESc6MxRqAp4LmePc2eeGX6XUxBcdhAmkthWTDqygPz2wLAyHWisD299Lkdrj5egY6/1/*)#n4cuwhcy]

really nice catch, making a mental note to make sure this doesnt get missed in future prs, fixing

updated code

thunderbiscuit

ACK 72fe618.

I don't know if there is value in it for Swift users (and maybe Python?) but I know the debug trait populates another field (I forget the name of it) for those (and doesn't do anything for Kotlin).

Anyway something to look at as part of #1020 maybe.

Just needs a rebase.

thunderbiscuit · 2026-06-23T15:52:57Z

Ready for rebase + merge.

github-project-automation Bot added this to BDK-Bindings Jun 17, 2026

reez added the changelog: added label Jun 17, 2026

reez marked this pull request as ready for review June 17, 2026 00:45

reez requested a review from thunderbiscuit June 17, 2026 00:45

thunderbiscuit reviewed Jun 17, 2026

View reviewed changes

reez force-pushed the wallet-keychains branch from d8204b0 to 87d898a Compare June 17, 2026 23:43

reez requested a review from thunderbiscuit June 17, 2026 23:46

thunderbiscuit requested changes Jun 18, 2026

View reviewed changes

reez force-pushed the wallet-keychains branch from 87d898a to 72fe618 Compare June 18, 2026 13:33

thunderbiscuit approved these changes Jun 18, 2026

View reviewed changes

reez force-pushed the wallet-keychains branch from 72fe618 to 63b57e9 Compare June 24, 2026 16:52

feat: expose Wallet::keychains

268457b

reez force-pushed the wallet-keychains branch from 63b57e9 to 268457b Compare June 24, 2026 16:53

reez merged commit 268457b into bitcoindevkit:master Jun 24, 2026
8 checks passed

github-project-automation Bot moved this to Done in BDK-Bindings Jun 24, 2026

reez deleted the wallet-keychains branch June 24, 2026 17:55

Uh oh!

Conversation

reez commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Documentation

Changelog

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

thunderbiscuit left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

reez Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

reez Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

reez commented Jun 17, 2026

Uh oh!

reez commented Jun 17, 2026

Uh oh!

thunderbiscuit left a comment

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

reez Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

reez Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit commented Jun 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

reez commented Jun 17, 2026 •

edited

Loading

thunderbiscuit left a comment •

edited

Loading

thunderbiscuit left a comment •

edited

Loading