wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key #36154

pull jeanpablojp wants to merge 2 commits into bitcoin:master from jeanpablojp:psbt-global-xpubs changing 13 files +336 −36
  1. jeanpablojp commented at 10:53 PM on September 2, 2026: contributor

    Closes #27583.

    PSBT_GLOBAL_XPUB already serializes, deserializes and merges when PSBTs are combined. Nothing writes it, so a PSBT produced by the wallet never carries it.

    The original motivation was the Ledger, and BIP 388 solved that for some devices, with Sjors working on the registration path. The field is still needed whenever the signer has no registered policy to work from. HWI's BitBox02 driver builds the multisig script config out of the PSBT's extended keys and refuses to sign without them, and HWI's own test harness marks that device as needing them, include_xpubs = True in test/test_bitbox02.py against False for the Trezor. Of the seven device drivers in HWI only those two read the field at all; the Ledger, Coldcard and Jade all go through a registration of their own. Today Specter fills the field itself after getting the PSBT from Core, and only when the wallet has more than one key.

    <details><summary>Before and after with the BitBox02 and Trezor simulators</summary>

    A 2-of-3 wsh(sortedmulti(...)) on regtest, with the simulator as one co-signer and two keys from Core wallets. The PSBT comes from walletcreatefundedpsbt on this branch; the "before" file is that same PSBT with its three PSBT_GLOBAL_XPUB records removed, so nothing else differs. HWI 3.2.0, BitBox02 simulator at firmware/v9.24.0, the version HWI pins.

    without PSBT_GLOBAL_XPUB: REFUSED -> BadArgumentError: This BitBox02 is not one of the cosigners
    with PSBT_GLOBAL_XPUB   : SIGNED, 1 partial signature(s)
    

    The refusal comes from _multisig_scriptconfig, which looks for the device's own xpub among the PSBT's and raises when the map is empty. Registering the account needs a name passed in, since HWI sends an empty one and the device then asks for it interactively, which a headless simulator cannot answer.

    The Trezor T simulator signs either way, but it is handed different data. parse_multisig in its driver fills each co-signer node from the PSBT's global scope, and with nothing there it sends placeholders:

    without PSBT_GLOBAL_XPUB
      input : global xpubs=0  zeroed chain codes=3/3  depths=[0, 0, 0]  paths=[[], [], []]
      change: global xpubs=0  zeroed chain codes=3/3  depths=[0, 0, 0]  paths=[[], [], []]
      result: SIGNED, 1 partial signature(s)
    
    with PSBT_GLOBAL_XPUB
      input : global xpubs=3  zeroed chain codes=0/3  depths=[4, 4, 4]  paths=[[0, 0], [0, 0], [0, 0]]
      change: global xpubs=3  zeroed chain codes=0/3  depths=[4, 4, 4]  paths=[[1, 0], [1, 0], [1, 0]]
      result: SIGNED, 1 partial signature(s)
    

    On this firmware the signature, the confirmation prompts and the screens are identical either way. What the field changes is that the device is told the truth instead of a placeholder.

    </details>

    Descriptor gains an accessor for its extended keys, each with the origin taken at the deepest hardened step, and DescriptorScriptPubKeyMan::FillPSBT writes them when the descriptor contributed to the PSBT. That covers walletcreatefundedpsbt, walletprocesspsbt, send, sendall, psbtbumpfee and the GUI send dialog, which all go through CWallet::FillPSBT.

    The field is only written for descriptors with two or more extended keys. With one, the signer derives its own key from its seed, so the entry would add nothing and only cost space. The field is not small. On a 2-of-3 it adds 291 bytes, taking the PSBT from 933 to 1224. bip32derivs is respected. That same PSBT already carries the full witness_script and bip32_derivs, so what is added is the chain code of each account.

    There are unit tests for the accessor, and on the wallet side a functional test with a wallet whose internal descriptor uses different keys from its external one, the only arrangement that can show it publishing on its own.

    descriptorprocesspsbt is left out, since it signs from the descriptors it is given without going through the wallet, and I intend to cover it as a follow-up. musig() contributes nothing, since derivation is applied to the aggregate key.

    The decodepsbt output changes, and a release note is included.

  2. refactor: extract the last hardened xpub lookup in BIP32PubkeyProvider
    ToNormalizedString walks the key path backwards to the last hardened
    step, and then fetches the extended public key sitting at that step,
    from the descriptor cache when it is cached and by deriving it
    otherwise. Both halves are useful on their own, so pull them out as
    LastHardenedIndex() and GetLastHardenedExtPubKey().
    
    No behaviour change.
    42d5635587
  3. DrahtBot added the label Wallet on Sep 2, 2026
  4. DrahtBot commented at 10:53 PM on September 2, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. jeanpablojp force-pushed on Sep 2, 2026
  6. DrahtBot added the label CI failed on Sep 2, 2026
  7. DrahtBot commented at 11:14 PM on September 2, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task NetBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/33692512279/job/100454171219</sub> <sub>LLM reason (✨ experimental): CI failed because the C++ build was stopped by a Clang -Wthread-safety-analysis error treated as -Werror in src/wallet/scriptpubkeyman.cpp (calling AddGlobalXpubs violates capability !cs_desc_man).</sub>

    <details><summary>Hints</summary>

    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.

    </details>

  8. jeanpablojp marked this as a draft on Sep 3, 2026
  9. wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key
    BIP 174 lets a PSBT carry the extended public keys the inputs and
    outputs derive from, so that a signer holding no wallet state can
    rebuild the script from the PSBT alone. Hardware signers read the
    field, but the wallet never writes it, so a PSBT it produces never
    carries it.
    
    Give Descriptor an accessor for the extended public keys it is built
    from, each with the origin of the key itself, taken at the deepest
    hardened step so that the unhardened children used in the transaction
    can be derived from it, and have DescriptorScriptPubKeyMan::FillPSBT
    write them when the descriptor contributed to the PSBT. That covers
    every RPC that builds a PSBT from the wallet, walletcreatefundedpsbt,
    walletprocesspsbt, send, sendall and psbtbumpfee, as well as the GUI
    send dialog, since all of them go through CWallet::FillPSBT.
    
    Only descriptors with more than one key publish: for a single key the
    field says nothing the per-input derivation paths do not already say,
    and every PSBT the wallet produces would grow for nothing. bip32derivs
    is respected, so the field can be turned off with the paths.
    92bf732c4e
  10. jeanpablojp force-pushed on Sep 3, 2026
  11. jeanpablojp marked this as ready for review on Sep 3, 2026
  12. DrahtBot removed the label CI failed on Sep 3, 2026
  13. Sjors commented at 6:34 AM on September 3, 2026: member

    @jeanpablojp in the RP description, can you point to the source code in Specter where it adds the global xpub? This might be useful for those who want to verify that it's still needed. Similarly, it's useful to provide a before and after example flow with HWI (and e.g. a Trezor / simulator).

    (It's also better to not to tag people in the PR description, because IIUC that text ends up in the merge commit, which then keeps triggering notifications as other projects backport it.)

  14. jeanpablojp commented at 9:03 PM on September 3, 2026: contributor

    Both are in the description now.

    The Specter link points at Wallet.fill_psbt, pinned to a commit so the line numbers don't drift.

    On the flow: I ran the Trezor T simulator first, and it signs the same PSBT with or without the field, same signature, same prompts, same screens. What differs is only the data HWI hands it, since without the field parse_multisig fills the co-signer nodes with zeroed chain codes and no derivation paths. The device that does show a difference is the BitBox02: it refuses with This BitBox02 is not one of the cosigners and signs once the field is there. Both runs are in the collapsed block.

    I had written that the Trezor needed the field. That was wrong, and the description is corrected.

    Also dropped the tag, thanks for the note about the merge commit.

  15. Sjors commented at 7:08 AM on September 4, 2026: member

    Thanks. It's useful to know that Trezor doesn't seem to need this, but BitBox02 and perhaps other devices do.

    I suspect that once we support the new registerdescriptor command in HWI, and pass that registration back to HWI when signing, none of the devices will need the global xpub. That's because HWI can construct everything it needs from the descriptor, which itself contains the xpubs.

    But providing global xpubs may still be useful for flows without HWI.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-04 09:50 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me