Skip to content

feat(wallet): add Wallet::sign_psbt<K> wrapping bitcoin::Psbt::sign#438

Open
EliteCoder18 wants to merge 3 commits intobitcoindevkit:masterfrom
EliteCoder18:feat/sign-psbt-wrapper
Open

feat(wallet): add Wallet::sign_psbt<K> wrapping bitcoin::Psbt::sign#438
EliteCoder18 wants to merge 3 commits intobitcoindevkit:masterfrom
EliteCoder18:feat/sign-psbt-wrapper

Conversation

@EliteCoder18
Copy link
Copy Markdown

@EliteCoder18 EliteCoder18 commented Apr 10, 2026

Description

Related to #331

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

  • BIP32 derivation prerequisite: bitcoin::Psbt::sign reads bip32_derivation fields to locate child keys. A freshly built PSBT lacks these. Callers must first run wallet.sign(&mut psbt, SignOptions { try_finalize: false, ..Default::default() }) to populate them. Documented in rustdoc.
  • 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.
  • Descriptor-aware variant deferred: The KeyMapWrapper variant (rust-miniscript#867) is postponed to 4.0 per maintainer direction.

Changelog notice

  • Added Wallet::sign_psbt<K> wrapper around bitcoin::Psbt::sign using the wallet's internal secp256k1 context.

Checklists

All Submissions:

New Features:

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

@ValuedMammal ValuedMammal added this to the Wallet 3.1.0 milestone Apr 10, 2026
@ValuedMammal ValuedMammal added the new feature New feature or request label Apr 10, 2026
@ValuedMammal ValuedMammal moved this to In Progress in BDK Wallet Apr 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.07%. Comparing base (fb7681a) to head (ce3d189).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
+ Coverage   80.04%   80.07%   +0.03%     
==========================================
  Files          24       24              
  Lines        5336     5345       +9     
  Branches      242      242              
==========================================
+ Hits         4271     4280       +9     
  Misses        987      987              
  Partials       78       78
Flag Coverage Δ
rust 80.07% <100.00%> (+0.03%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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
Copy link
Copy Markdown
Collaborator

ValuedMammal commented Apr 10, 2026

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
feat(wallet): add Wallet::sign_psbt<K> wrapper for Psbt::sign
10ec038 
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.
@EliteCoder18
test(wallet): add tests for Wallet::sign_psbt
808b1f3 
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
@EliteCoder18
docs(wallet): document Wallet::sign_psbt usage and requirement
ce3d189
@EliteCoder18 EliteCoder18 force-pushed the feat/sign-psbt-wrapper branch from 6694fbb to ce3d189 Compare April 11, 2026 08:34
@EliteCoder18
Copy link
Copy Markdown
Author

EliteCoder18 commented Apr 11, 2026

@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 ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ValuedMammal ValuedMammal Awaiting requested review from ValuedMammal ValuedMammal is a code owner

@oleonardolima oleonardolima Awaiting requested review from oleonardolima oleonardolima is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@EliteCoder18 EliteCoder18

Labels

new feature New feature or request

Projects

BDK Wallet
Status: Needs Review

Milestone

Wallet 3.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@EliteCoder18 @ValuedMammal