wallet: support bip388 policy with external signer

Sjors commented at 3:31 pm on July 18, 2025: member

External signers such as Ledger, BitBox02 and Jade, when used with multisig, use BIP388 to display (and constrain) descriptor policies to their users.

At least in the case of Ledger (haven’t tested the others) this requires us to hold on to an hmac, which proves to the device that the user previously reviewed and approved it.

This PR adds a new RPC call registerpolicy which converts our descriptor(s) into a BIP388 policy (partially implemented), calls a new HWI method register (implemented) and stores the resulting HMAC (implemented).

When signing a transaction using HWI’s signtx it passes the HMAC along with the PSBT (TODO).

For convenience the HMAC can be retrieved via getwalletinfo (implemented). This is useful for scenario’s where we can’t (yet) complete a transaction via HWI calls.

Note that the HMAC itself is not specified in BIP388 and may be different for different hardware vendors. So we’ll store and echo any hex string the device gives us.

An alternative approach to directly supporting BIP388 policies would be to pass the involved descriptors to HWI and have it figure out how to derive a policy.

Testing:

make a multisig wallet with a Ledger or other supported device
- try https://github.com/Sjors/bitcoin/pull/91 if you’re feeling adventurous, it includes this PR
install https://github.com/bitcoin-core/HWI/pull/791 HWI branch
call bitcoin rpc registerpolicy
call bitcoin rpc getwalletinfo to see the hmac

TODO:

derive policy using Descriptor class instead regex madness, implement all constraints
pass hmac along when signing

Depends on:

HWI register command: https://github.com/bitcoin-core/HWI/pull/791
HWI optional --hmac argument for signtx (if set, bypass the current auto-registration workaround)

Potential followups:

pass hmac along when displaying an address. This provides some extra assurance to the user. See https://github.com/bitcoin-core/HWI/pull/647

DrahtBot added the label Wallet on Jul 18, 2025

DrahtBot commented at 3:31 pm on July 18, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33008.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33034 (wallet: Store transactions in a separate sqlite table by achow101)
#32958 (wallet/refactor: change PSBTError to PSBTResult and remove std::optionalcommon::PSBTResult and return common::PSBTResult by kevkevinpal)
#32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
#32876 (refactor: use options struct for signing and PSBT operations by Sjors)
#32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)
#32857 (wallet: allow skipping script paths by Sjors)
#32784 (wallet: derivehdkey RPC to get xpub at arbitrary path by Sjors)
#32652 (wallet: add codex32 argument to addhdkey by roconnor-blockstream)
#32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
#29136 (wallet: addhdkey RPC to add just keys to wallets via new unused(KEY) descriptor by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

In wallet.cpp TODO comment: “apply furth BIP388 restrictions” → “apply further BIP388 restrictions” [‘furth’ is a typo for ‘further’]
In wallet.cpp comment: “Replace keys in descriptor swith @1, etc” → “Replace keys in descriptor with @1, etc” [‘swith’ should be ‘with’]
In external_signer_scriptpubkeyman.cpp error message: “Signer return invalid hmac: %s” → “Signer returned invalid hmac: %s” [past tense ‘returned’ needed]
In wallet.cpp error message: “Could not find ExternalSignerScriptPubKeyMananager” → “Could not find ExternalSignerScriptPubKeyManager” [extra ‘an’ typo]
In wallet.h doc comment: “manangers” → “managers” [spelling typo]

drahtbot_id_4_m

DrahtBot added the label CI failed on Jul 18, 2025

DrahtBot commented at 4:31 pm on July 18, 2025: contributor

🚧 At least one of the CI tasks failed. Task previous releases, depends DEBUG: https://github.com/bitcoin/bitcoin/runs/46270694515 LLM reason (✨ experimental): The CI failure is caused by a compilation error due to a redundant declaration of ‘RPCHelpMan wallet::registerpolicy()’ which is treated as an error because all warnings are enabled as errors.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

Sjors force-pushed on Jul 21, 2025

DrahtBot removed the label CI failed on Jul 21, 2025

Sjors force-pushed on Jul 24, 2025

wallet: add bip388_hmac record bea4cebfbd

wallet: add RegisterPolicy to external signer ff9edd55e5

wallet: add RegisterPolicy to signer SPKM 638c643195

Sjors force-pushed on Aug 5, 2025

refactor: avoid std::regex in ReplaceAll 644859e3de

Sjors force-pushed on Aug 5, 2025

DrahtBot added the label CI failed on Aug 5, 2025

DrahtBot commented at 9:20 pm on August 5, 2025: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/runs/47448999684 LLM reason (✨ experimental): The CI failure is due to an error flagged by clang-tidy in wallet.cpp at line 2621.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

Sjors force-pushed on Aug 6, 2025

DrahtBot removed the label CI failed on Aug 6, 2025

Sjors force-pushed on Aug 6, 2025

wallet: add registerPolicy() 32c6d33b67

rpc: add registerpolicy f2c78a5cc7

Sjors force-pushed on Aug 6, 2025

pythcoiner commented at 5:12 pm on August 7, 2025: none

At least in the case of Ledger (haven’t tested the others) this requires us to hold on to an hmac, which proves to the device that the user previously reviewed and approved it.

In the range of devices supporting miniscript, only Ledger requires the software to store the Proof Of Registration, other devices stores it internally(SpecterDiy, BitBox, Coldcard, Jade) or not at all (Krux).

DrahtBot added the label Needs rebase on Aug 12, 2025

DrahtBot commented at 8:10 pm on August 12, 2025: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

wallet: support bip388 policy with external signer #33008

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)