feat(wallet): add Wallet::sign_psbt<K> wrapping bitcoin::Psbt::sign by EliteCoder18 · Pull Request #438 · bitcoindevkit/bdk_wallet

EliteCoder18 · 2026-04-10T20:14:59Z

Description

Related to #331
Incorporates get_keymap() from #334

Adds Wallet::sign_psbt<K>, a thin wrapper around bitcoin::Psbt::sign that passes the wallet's internal secp256k1 context to the caller's GetKey implementor (e.g. Xpriv, a hardware wallet adapter).

Without this, callers driving signing themselves via the bitcoin crate must construct their own Secp256k1<All> — setup the wallet already owns.

Notes to the reviewers

sign vs sign_psbt: Wallet::sign uses BDK's internal signer infrastructure (InputSigner/TransactionSigner). sign_psbt delegates entirely to bitcoin::Psbt::sign; callers supply their own GetKey. Both methods coexist without conflict.

Changelog notice

Added Wallet::sign_psbt<K> wrapper around bitcoin::Psbt::sign using the wallet's internal secp256k1 context.
Added Wallet::get_keymap() to retrieve a combined KeyMap from all wallet signers for use with KeyMapWrapper and Wallet::sign_psbt.

Checklists

All Submissions:

I followed the contribution guidelines

codecov · 2026-04-10T21:52:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.01%. Comparing base (58fe631) to head (43702e7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
+ Coverage   80.96%   81.01%   +0.04%     
==========================================
  Files          24       24              
  Lines        5489     5503      +14     
  Branches      247      247              
==========================================
+ Hits         4444     4458      +14     
  Misses        968      968              
  Partials       77       77

Flag	Coverage Δ
rust	`81.01% <100.00%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ValuedMammal · 2026-04-10T22:28:01Z

Concept ACK. I've not had an extensive look at the implementation.

Callers must first run wallet.sign()

sign_psbt should be written in a way that assumes the PSBT has been updated and totally independent of wallet.sign(). For the tests you can create the PSBT to sign by parsing it from a string in base64.

EliteCoder18 · 2026-04-11T08:52:50Z

@ValuedMammal Thanks for the feedback!

I've updated the tests to use a base64 PSBT with pre-populated bip32_derivation, so sign_psbt is now fully independent of wallet.sign.

Would appreciate a review when you get a chance.

ValuedMammal

Concept ACK

ValuedMammal · 2026-04-23T00:35:09Z

+    /// [`bitcoin::Psbt::sign`] uses the `bip32_derivation` fields in each PSBT input to
+    /// locate the correct child keys. PSBTs received from external coordinator tools or


the bip32_derivation fields in each PSBT input to locate the correct child keys.

Or tap_key_origins in the case of taproot.

Updated the doc comment to mention both bip32_derivation (legacy/segwit) and tap_key_origins (taproot). Thanks for catching that!

oleonardolima

cACK ce3d189

noahjoeris

cACK 0884a8d
Thanks for adding this. It will be useful after this is merged: rust-bitcoin/rust-miniscript#872

noahjoeris · 2026-06-18T14:30:36Z

+    /// Sign a PSBT using an external key provider via [`bitcoin::Psbt::sign`].
+    ///
+    /// This is a thin wrapper around [`bitcoin::Psbt::sign`] that supplies the wallet's
+    /// internal [`secp256k1`] context. It lets callers sign with any type that implements
+    /// [`psbt::GetKey`], such as [`bitcoin::bip32::Xpriv`], without having to manage their
+    /// own secp256k1 context.
+    ///
+    /// # BIP32 derivation metadata
+    ///
+    /// [`bitcoin::Psbt::sign`] uses the `bip32_derivation` fields (for legacy and segwit inputs)
+    /// or the `tap_key_origins` fields (for taproot inputs) in each PSBT input to locate
+    /// the correct child keys. PSBTs received from external coordinator tools or
+    /// hardware wallet flows typically carry this metadata already.
+    ///
+    /// # Returns
+    ///
+    /// On success, a [`psbt::SigningKeysMap`] mapping each signed input index to the keys
+    /// that were used. On failure, a tuple of the partial success map and a
+    /// [`psbt::SigningErrors`] map of per-input errors.
+    ///
+    /// [`secp256k1`]: bitcoin::secp256k1


nit: What do you think of those docs changes? I added that it's used for software signing and pointed to the correct approach for external signing.

Suggested change

/// Sign a PSBT using an external key provider via [`bitcoin::Psbt::sign`].

///

/// This is a thin wrapper around [`bitcoin::Psbt::sign`] that supplies the wallet's

/// internal [`secp256k1`] context. It lets callers sign with any type that implements

/// [`psbt::GetKey`], such as [`bitcoin::bip32::Xpriv`], without having to manage their

/// own secp256k1 context.

///

/// # BIP32 derivation metadata

///

/// [`bitcoin::Psbt::sign`] uses the `bip32_derivation` fields (for legacy and segwit inputs)

/// or the `tap_key_origins` fields (for taproot inputs) in each PSBT input to locate

/// the correct child keys. PSBTs received from external coordinator tools or

/// hardware wallet flows typically carry this metadata already.

///

/// # Returns

///

/// On success, a [`psbt::SigningKeysMap`] mapping each signed input index to the keys

/// that were used. On failure, a tuple of the partial success map and a

/// [`psbt::SigningErrors`] map of per-input errors.

///

/// [`secp256k1`]: bitcoin::secp256k1

/// Sign a PSBT using an external key provider via [`bitcoin::Psbt::sign`].

///

/// This is a thin wrapper around [`bitcoin::Psbt::sign`] that supplies the wallet's

/// internal [`secp256k1`] context. It lets callers sign with any type that implements

/// [`psbt::GetKey`], such as [`bitcoin::bip32::Xpriv`], without having to manage their

/// own secp256k1 context. Use it for software signing.

///

/// # BIP32 derivation metadata

///

/// [`bitcoin::Psbt::sign`] uses the `bip32_derivation` fields (for legacy and segwit inputs)

/// or the `tap_key_origins` fields (for taproot inputs) in each PSBT input to locate

/// the correct child keys. PSBTs constructed from within the wallet, or PSBTs received from

/// external coordinator tools or hardware wallet flows, typically carry this metadata already.

///

/// # Returns

///

/// On success, a [`psbt::SigningKeysMap`] mapping each signed input index to the keys

/// that were used. On failure, a tuple of the partial success map and a

/// [`psbt::SigningErrors`] map of per-input errors.

///

/// [`secp256k1`]: bitcoin::secp256k1

/// [`TransactionSigner`]: crate::signer::TransactionSigner

I'm not sure if we should refer to TransactionSigner, as it'll be later removed with signers module.

The hardware signers should handle the PSBT and signing keys on their own, considering that we had archived https://github.com/bitcoindevkit/rust-hwi and it's considered unmaintained.

I'm not sure if we should refer to TransactionSigner, as it'll be later removed with signers module.

Anyway, we can add it now and remove it when removing the module.

Yep thanks, was thinking about it too.
From my understanding we added sign_with_signers as a transition step to keep supporting the bdk signer for some more time while moving to psbt.sign.
Which means yeah agree we shouldn't refer to it.

oleonardolima

cACK 0884a8d

I suggest squashing the doc commit into the first one, and also updating the PR description to reflect the current state.

oleonardolima · 2026-06-18T21:15:21Z

nit: I think it's a good idea to also add the helper method get_keymap here, you can cherry-pick it from #334.

As you add the helper method here, you can add an example of how to use in the documentation of sign_psbt.

NOTE: that you'd need to use the KeyMapWrapper, as we're still in miniscript 12.x

EliteCoder18 · 2026-06-19T21:25:17Z

Happy to add get_keymap, but since KeyMapWrapper is a temporary shim until rust-miniscript#872 lands, I'd rather not bake it into the docs here and then need to update it again in 4.0. planning to add get_keymap as a proper follow-up once the clean descriptor-aware path is available. Does that work for you?

nit: I think it's a good idea to also add the helper method get_keymap here, you can cherry-pick it from #334.

As you add the helper method here, you can add an example of how to use in the documentation of sign_psbt.

NOTE: that you'd need to use the KeyMapWrapper, as we're still in miniscript 12.x

oleonardolima · 2026-06-20T15:30:48Z

Happy to add get_keymap, but since KeyMapWrapper is a temporary shim until rust-miniscript#872 lands, I'd rather not bake it into the docs here and then need to update it again in 4.0. planning to add get_keymap as a proper follow-up once the clean descriptor-aware path is available. Does that work for you?

This PR is aimed to 3.x.x release, so it'll require the KeyMapWrapper anyway, either here or in the user side. It's best if we have all the ergonomic and convenience methods to guide the users onto how to use the new API, as per #340

EliteCoder18 · 2026-06-20T21:21:59Z

Thanks for the suggestion. I've added get_keymap() along with a test, and updated the sign_psbt docs with an example showing the get_keymap → KeyMapWrapper flow.

I left out a KeyMapWrapper re-export for now to keep the public API surface minimal, since it can already be imported directly from miniscript.

This PR is aimed to 3.x.x release, so it'll require the KeyMapWrapper anyway, either here or in the user side. It's best if we have all the ergonomic and convenience methods to guide the users onto how to use the new API, as per #340

Add a thin wrapper method `sign_psbt` that delegates to `bitcoin::Psbt::sign`, passing the wallet's internal secp256k1 context. This allows users to sign PSBTs with their own `GetKey` implementor (e.g., `Xpriv` or hardware wallet adapters) without managing a separate secp256k1 context. docs(wallet): document Wallet::sign_psbt usage and requirement

Add tests covering signing with a matching and non-matching `Xpriv`. - `test_sign_psbt_with_xpriv`: verifies that a matching key signs at least one input - `test_sign_psbt_with_wrong_key_signs_nothing`: verifies that an unrelated key returns Ok but produces no signatures

oleonardolima

ACK 40a8cac

oleonardolima · 2026-06-23T14:14:40Z

+            "expected no partial signatures when signing with an unrelated key"
+        );
+    }
+}


nit: you could also add a test that uses the get_keymap and sign with Wallet keys here.

oleonardolima · 2026-06-23T14:15:33Z

cc'in @thunderbiscuit as you're interested in the new signing flow.

noahjoeris · 2026-06-24T13:29:42Z

Why do we want get_keymap if we plan to remove wallet-owned signers?
This is adding functionality that is already planned to be deprecated soon.

notmandatory · 2026-06-24T17:01:44Z

Why do we want get_keymap if we plan to remove wallet-owned signers? This is adding functionality that is already planned to be deprecated soon.

This is a good point. If this is something we can do without and will have to later remove then we shouldn't do it at all. Based on the docs in bitcoindevkit/book-of-bdk#155 it looks like we don't need this helper.

oleonardolima · 2026-06-24T19:41:42Z

Why do we want get_keymap if we plan to remove wallet-owned signers? This is adding functionality that is already planned to be deprecated soon.

Yes, it'll be removed in the next breaking change when removing the signers, though it's a good helper (not a MUST) method for current users to use the Wallet::sign_psbt<K> at current state, otherwise they would need to handle that themselves.

oleonardolima · 2026-06-24T19:45:37Z

Why do we want get_keymap if we plan to remove wallet-owned signers? This is adding functionality that is already planned to be deprecated soon.

This is a good point. If this is something we can do without and will have to later remove then we shouldn't do it at all. Based on the docs in bitcoindevkit/book-of-bdk#155 it looks like we don't need this helper.

Won't the SignersContainers in the example there also be removed in the future ?

EliteCoder18 requested review from ValuedMammal and oleonardolima as code owners April 10, 2026 20:15

github-project-automation Bot added this to BDK Wallet Apr 10, 2026

ValuedMammal added this to the Wallet 3.1.0 milestone Apr 10, 2026

ValuedMammal added the new feature New feature or request label Apr 10, 2026

ValuedMammal assigned EliteCoder18 Apr 10, 2026

ValuedMammal moved this to In Progress in BDK Wallet Apr 10, 2026

EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch from 6694fbb to ce3d189 Compare April 11, 2026 08:34

ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Apr 14, 2026

ValuedMammal reviewed Apr 23, 2026

View reviewed changes

oleonardolima reviewed Apr 27, 2026

View reviewed changes

EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch from ce3d189 to 0884a8d Compare April 30, 2026 18:52

notmandatory removed this from the Wallet 3.1.0 milestone Jun 2, 2026

noahjoeris reviewed Jun 18, 2026

View reviewed changes

oleonardolima added this to the Wallet 3.2.0 milestone Jun 18, 2026

oleonardolima reviewed Jun 18, 2026

View reviewed changes

EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch from 0884a8d to 0b62d13 Compare June 19, 2026 21:33

EliteCoder18 added 2 commits June 21, 2026 02:58

EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch 3 times, most recently from bed270d to 40a8cac Compare June 20, 2026 22:29

oleonardolima approved these changes Jun 23, 2026

View reviewed changes

oleonardolima requested a review from notmandatory June 23, 2026 14:15

noahjoeris mentioned this pull request Jun 23, 2026

Deprecate wallet-owned signing APIs #505

Draft

2 tasks

feat(wallet): add get_keymap() helper, sign_psbt doc example and test

43702e7

EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch from 40a8cac to 43702e7 Compare June 23, 2026 19:49

noahjoeris mentioned this pull request Jun 24, 2026

Update starter example to use new signing workflow bitcoindevkit/book-of-bdk#155

Merged

		/// [`bitcoin::Psbt::sign`] uses the `bip32_derivation` fields in each PSBT input to
		/// locate the correct child keys. PSBTs received from external coordinator tools or

Uh oh!

Conversation

EliteCoder18 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

Uh oh!

codecov Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ValuedMammal commented Apr 10, 2026

Uh oh!

EliteCoder18 commented Apr 11, 2026

Uh oh!

ValuedMammal left a comment

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

EliteCoder18 Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

noahjoeris 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!

noahjoeris Jun 18, 2026 • 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!

oleonardolima Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

noahjoeris Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

oleonardolima commented Jun 18, 2026

Uh oh!

EliteCoder18 commented Jun 19, 2026

Uh oh!

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

Uh oh!

Uh oh!

EliteCoder18 commented Jun 20, 2026

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

oleonardolima Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima commented Jun 23, 2026

Uh oh!

noahjoeris commented Jun 24, 2026

Uh oh!

notmandatory commented Jun 24, 2026

Uh oh!

oleonardolima commented Jun 24, 2026

Uh oh!

oleonardolima commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

EliteCoder18 commented Apr 10, 2026 •

edited

Loading

codecov Bot commented Apr 10, 2026 •

edited

Loading

noahjoeris left a comment •

edited

Loading

noahjoeris Jun 18, 2026 •

edited

Loading

oleonardolima commented Jun 20, 2026 •

edited

Loading