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, and a signer that only has the file cannot rebuild the multisig from it.

    A signer with no registered policy to work from needs it: HWI's BitBox02 driver builds the multisig out of the PSBT's extended keys and refuses to sign without them. 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, signed through HWI 3.2.0. The "before" file is the same PSBT with its PSBT_GLOBAL_XPUB records removed, so nothing else differs.

    without the field with the field
    BitBox02 refuses, This BitBox02 is not one of the cosigners signs
    Trezor T signs, co-signer nodes sent with zeroed chain codes and no derivation paths signs, nodes sent with the real chain codes and paths

    What parse_multisig hands the Trezor for the change output:

    without: global xpubs=0  zeroed chain codes=3/3  depths=[0, 0, 0]  paths=[[], [], []]
    with   : global xpubs=3  zeroed chain codes=0/3  depths=[4, 4, 4]  paths=[[1, 0], [1, 0], [1, 0]]
    

    </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.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36167 ([RFC] Enable -Wunused by fanquake)
    • #36143 (descriptor: add CreateMultisigDescriptor() by rxbryan)
    • #36133 (wallet: store multipath descriptor by Sjors)
    • #36122 (BIP460: CISA for Taproot key path spends by fjahr)
    • #36013 (test: Descriptor roundtrip and raw()/ addr() coverage by pablomartin4btc)
    • #35445 (wallet, descriptor: Revert StringType::COMPAT for Miniscript expressions and drop the concept of a Descriptor ID that can be validated by achow101)
    • #35370 (rpc: add key-origin modes to PSBT processing RPCs by junbyjun1238)
    • #35041 (descriptor: speed-up Parse (xpub/xpriv) in ~30% by brunoerg)
    • #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)

    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.

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

  16. achow101 commented at 10:38 PM on September 7, 2026: member

    Please shorten your description. We don't need to know the exact details about every single device that needs to know about the global xpub.

    Why is this restricted to only multisigs? Just because that is the described use case in the original issue does not mean that it is the only use case. PSBTs and Descriptors are generic objects, we should avoid doing things that are specific to one type of descriptor or psbt.

  17. in src/script/descriptor.cpp:244 in 92bf732c4e
     236 | @@ -237,6 +237,12 @@ struct PubkeyProvider
     237 |      /** Return the extended public key for this PubkeyProvider, if it has one. */
     238 |      virtual std::optional<CExtPubKey> GetRootExtPubKey() const = 0;
     239 |  
     240 | +    /** Collect the extended public keys this provider contributes, each with the origin
     241 | +     *  of the key itself, in the form BIP 174's PSBT_GLOBAL_XPUB expects: the key at the
     242 | +     *  deepest hardened derivation step, so that the unhardened children used in the
     243 | +     *  transaction can be derived from it. */
     244 | +    virtual void GetExtPubKeysWithOrigins(const SigningProvider& arg, const DescriptorCache* cache, std::map<KeyOriginInfo, std::set<CExtPubKey>>& out) const = 0;
    


    achow101 commented at 10:40 PM on September 7, 2026:

    In 92bf732c4ee0e7096aa3f203ebcafcdf9366ff1a "wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key"

    This should return the map rather than taking it as an output parameter.

    Do not provide SigningProvider, either the last hardened key is already cached, or it does not exist. There is no need to take a SigningProvider here, otherwise the wallet wouldn't function.

  18. in src/script/descriptor.cpp:640 in 92bf732c4e
     632 | @@ -615,6 +633,28 @@ class BIP32PubkeyProvider final : public PubkeyProvider
     633 |      {
     634 |          return m_root_extkey;
     635 |      }
     636 | +    void GetExtPubKeysWithOrigins(const SigningProvider& arg, const DescriptorCache* cache, std::map<KeyOriginInfo, std::set<CExtPubKey>>& out) const override
     637 | +    {
     638 | +        // A hardened ranged key expression derives children the receiver cannot reach from
     639 | +        // any parent we could publish, so there is nothing useful to contribute.
     640 | +        if (m_derive == DeriveType::HARDENED_RANGED) return;
    


    achow101 commented at 10:43 PM on September 7, 2026:

    In 92bf732c4ee0e7096aa3f203ebcafcdf9366ff1a "wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key"

    We should not make assumptions about what the caller is going to use the xpubs for. This should return the root and it's fingerprint for any key where we cannot do the normalized key thing.

  19. in src/script/descriptor.cpp:644 in 92bf732c4e
     639 | +        // any parent we could publish, so there is nothing useful to contribute.
     640 | +        if (m_derive == DeriveType::HARDENED_RANGED) return;
     641 | +
     642 | +        KeyOriginInfo origin;
     643 | +        CExtPubKey xpub;
     644 | +        const int last_hardened{LastHardenedIndex()};
    


    achow101 commented at 10:46 PM on September 7, 2026:

    In 92bf732c4ee0e7096aa3f203ebcafcdf9366ff1a "wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key"

    It is not necessary to determine what the last hardened index is, just ask the cache for it, and if the cache doesn't have that key, return the root.

  20. in src/script/descriptor.cpp:856 in 92bf732c4e
     848 | @@ -809,6 +849,11 @@ class MuSigPubkeyProvider final : public PubkeyProvider
     849 |      {
     850 |          return std::nullopt;
     851 |      }
     852 | +    // Derivation here is applied to the aggregate key, not to the participants, so a
     853 | +    // participant's extended key does not derive the key that ends up in the script.
     854 | +    // BIP 174 asks for keys the receiver can derive the transaction's keys from, so
     855 | +    // there is nothing here that fits.
     856 | +    void GetExtPubKeysWithOrigins(const SigningProvider&, const DescriptorCache*, std::map<KeyOriginInfo, std::set<CExtPubKey>>&) const override {}
    


    achow101 commented at 10:48 PM on September 7, 2026:

    In 92bf732c4ee0e7096aa3f203ebcafcdf9366ff1a "wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key"

    This is incorrect. It should traverse the participants and get their xpubs.


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-09 07:56 UTC