wallet: fill PSBT_GLOBAL

jeanpablojp commented at 10:53 PM on September 2, 2026: contributor

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. Today Specter fills the field itself after getting the PSBT from Core.

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

A new Descriptor accessor returns the extended keys a descriptor is built from, and DescriptorScriptPubKeyMan::FillPSBT writes those of every descriptor with an input or an output in the PSBT. That covers walletcreatefundedpsbt, walletprocesspsbt, send, sendall, psbtbumpfee and the GUI send dialog, which all go through CWallet::FillPSBT.

Only the keys BIP 174 allows are added, the ones that derive, through unhardened steps alone, a key their descriptor produced, listed in the PSBT at the origin the descriptor gave it. So a key expression ending in /*h adds nothing.

Whoever holds a PSBT from the wallet can now derive the addresses of the descriptors it uses. Setting bip32derivs to false in walletcreatefundedpsbt or walletprocesspsbt leaves the field out, along with the derivation paths.

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.

DrahtBot added the label Wallet on Sep 2, 2026

DrahtBot commented at 10:53 PM on September 2, 2026: contributor

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

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36154
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36154.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36154#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	vicjuma
Stale ACK	fametrano

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36143 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36143.svg"> (descriptor: add CreateMultisigDescriptor() by rxbryan)
#36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
#36013 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36013.svg"> (test: Descriptor roundtrip and raw()/ addr() coverage by pablomartin4btc)
#36005 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36005.svg"> (rpc, descriptor: warn when multipath expansion clones single-path key expressions by pythcoiner)
#35918 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35918.svg"> (descriptor: Expose key expression parsing by pseudoramdom)
#35444 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35444.svg"> (wallet: make descriptor SPKM mutex non-recursive by w0xlt)

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 places where comparison-specific test macros should replace generic comparisons:

test/functional/wallet_multisig_descriptor_psbt.py: assert len(claims) == len(forged) → Prefer assert_equal(len(claims), len(forged)).
test/functional/wallet_multisig_descriptor_psbt.py: Both assert len(got) == 1 and got[0] in either, got checks → Prefer assert_equal(len(got), 1) for the length comparison.

2026-11-02 18:49:57

jeanpablojp force-pushed on Sep 2, 2026

DrahtBot added the label CI failed on Sep 2, 2026

DrahtBot commented at 11:14 PM on September 2, 2026: contributor

🚧 At least one of the CI tasks failed. Task NetBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/33692512279/job/100454171219 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).

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

jeanpablojp marked this as a draft on Sep 3, 2026

jeanpablojp force-pushed on Sep 3, 2026

jeanpablojp marked this as ready for review on Sep 3, 2026

DrahtBot removed the label CI failed on Sep 3, 2026

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

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.

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.

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.

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.

jeanpablojp commented at 10:15 PM on October 30, 2026:

Done, it returns the map and takes only the cache.

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.

jeanpablojp commented at 10:15 PM on October 30, 2026:

Done, it now returns the root and its fingerprint, /*h included.

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.

jeanpablojp commented at 10:15 PM on October 30, 2026:

Done, it asks the cache and falls back to the root.

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.

jeanpablojp commented at 10:15 PM on October 30, 2026:

Right, done. It now merges whatever its participants return, and wallet_musig.py checks that every participant's xpub ends up in global_xpubs with the origin that leads to it.

DrahtBot added the label Needs rebase on Sep 9, 2026

fametrano commented at 11:03 AM on October 2, 2026: contributor

@jeanpablojp, in case it saves you time: https://github.com/fametrano/bitcoin/tree/psbt-global-xpub-achow has achow101's points and the NetBSD thread-safety fix, rebased on master.

Changes: the multisig-only restriction is gone. Descriptor::GetExtPubKeysWithOrigins returns the map, takes no SigningProvider, and falls back to the root key when the cache lacks the last hardened one. musig() gives the participants' keys.

There are two commits. The wallet one is yours, with me as co-author. Take any of it or none. To use it as it is: git fetch https://github.com/fametrano/bitcoin psbt-global-xpub-achow && git reset --hard FETCH_HEAD, then force-push. A shorter description you could reword is below.

Written with LLM, a computer and the Internet; the mistakes, as usual, are all mine.

<details><summary>Suggested description</summary>

Closes #27583.

BIP 174 lets a PSBT carry, in PSBT_GLOBAL_XPUB, the extended public keys its inputs and outputs derive from. The wallet never writes the field. Some signers need them to rebuild the scripts.

FillPSBT now adds the extended keys of every descriptor that an input or output uses, unless bip32derivs is false. For each key expression this is the key at its last hardened step when the descriptor cache holds it, and the root key otherwise. For musig() these are the participants' keys. descriptorprocesspsbt does not go through the wallet and is left for a follow-up.

So, by default, a PSBT from the wallet carries the account xpubs of the descriptors it uses, and whoever gets it can derive the wallet's addresses. With hardened ranged derivation (/*h) the cache holds no such key, and the root key is published if its origin is known. Keys whose origin is unknown are left out.

</details>

jeanpablojp renamed this:
~~wallet: fill PSBT_GLOBAL_XPUB for descriptors with more than one key~~
wallet: fill PSBT_GLOBAL_XPUB
on Oct 30, 2026

jeanpablojp force-pushed on Oct 30, 2026

jeanpablojp commented at 10:23 PM on October 30, 2026: contributor

@achow101 thanks, shortened the description and dropped the multisig restriction. @fametrano thanks for the branch. The single-key test over the four address types follows yours. Unlike your branch, keys that derive none of the PSBT's keys stay out, like the root of a /*h expression, which in a migrated legacy wallet is the master key.

DrahtBot removed the label Needs rebase on Oct 30, 2026

fametrano commented at 6:20 PM on November 1, 2026: contributor

Concept ACK

One issue should be fixed before merge: a watch-only wallet holding one xpub under two origins now gets a PSBT that decodepsbt refuses. With wpkh([A/84h/1h/0h]xpub/0/*) and wpkh([00000000/84h/1h/0h]xpub/1/*), walletcreatefundedpsbt and send, which has no bip32derivs to turn it off, both report success and return it. No funds are at risk, but the error only shows when the next tool reads the file. This is the first of Ralph's two Minors; both reproduce at 437936e5 without an edited file.

The suggestion in AddGlobalXpubs keeps the origin already present, as MergeGlobalXPubs does since #35665. With all the suggestions, both reproductions exit 0, and the affected unit tests and eight functional tests pass; each regression test fails without its fix. Scripts and output: https://gist.github.com/fametrano/1cb22c31d89e06cd722dadaec3dc7230/b7f27effa3b47cd36468d9ae24407e71cf9970ee

Not blocking: the root exclusion falls to two RPCs. A sender calls descriptorprocesspsbt with wpkh([root origin]key) on a stranger's key, then a /*h wallet that the PSBT pays adds its root in walletprocesspsbt. That root derives no key of the wallet's descriptor, so no address is exposed, but the description says /*h adds nothing. The other two suggestions derive the key instead of comparing its path, and test it.

in src/wallet/scriptpubkeyman.cpp:1539 in 437936e575

1534 | +    // step, the descriptor hands out its root instead, and a root with hardened steps still
1535 | +    // below it derives none of them; a key written without its origin has a path too short
1536 | +    // for its depth. Neither can go in.
1537 | +    std::erase_if(xpubs, [&](const auto& entry) {
1538 | +        const auto& [origin, keys]{entry};
1539 | +        return !DerivesKeyInPSBT(psbtx, origin) ||

fametrano commented at 6:20 PM on November 1, 2026:

Not blocking: a path in the PSBT is only a claim, so this derives the key and compares it. It keeps the hardened check, since CPubKey::Derive asserts on a hardened step, and the regression test is the suggestion in the functional test. Only entries at this origin cost a derivation, and a PSBT's real key stops the search; a hostile 2 MB file of 510,000 steps took 6 to 7 s in walletprocesspsbt here, against 0.06 s on the head.

//! Whether one of these keys, at this origin, derives a key the PSBT gives a derivation
//! path for, through unhardened steps alone. The key is derived rather than its path
//! compared, since a path in the PSBT says whatever its sender wrote.
static bool DerivesKeyInPSBT(const PartiallySignedTransaction& psbtx, const KeyOriginInfo& origin, const std::set<CExtPubKey>& keys)
{
    const auto derives{[&](const XOnlyPubKey& pubkey, const KeyOriginInfo& info) {
        if (info.fingerprint != origin.fingerprint || info.path.size() < origin.path.size()) return false;
        const auto rest{info.path.begin() + origin.path.size()};
        if (!std::equal(info.path.begin(), rest, origin.path.begin()) ||
            std::any_of(rest, info.path.end(), [](uint32_t step) { return (step & BIP32_HARDENED_FLAG) != 0; })) return false;
        return std::ranges::any_of(keys, [&](CExtPubKey key) {
            for (auto step{rest}; step != info.path.end(); ++step) {
                CExtPubKey child;
                if (!key.Derive(child, *step)) return false;
                key = child;
            }
            return XOnlyPubKey{key.pubkey} == pubkey;
        });
    }};
    const auto lists_derived_key{[&](const auto& in_or_out) {
        return std::ranges::any_of(in_or_out.hd_keypaths, [&](const auto& entry) { return derives(XOnlyPubKey{entry.first}, entry.second); }) ||
               std::ranges::any_of(in_or_out.m_tap_bip32_paths, [&](const auto& entry) { return derives(entry.first, entry.second.second); });
    }};
    return std::ranges::any_of(psbtx.inputs, lists_derived_key) || std::ranges::any_of(psbtx.outputs, lists_derived_key);
}

void DescriptorScriptPubKeyMan::AddGlobalXpubs(PartiallySignedTransaction& psbtx) const
{
    LOCK(cs_desc_man);
    std::map<KeyOriginInfo, std::set<CExtPubKey>> xpubs{
        m_wallet_descriptor.descriptor->GetExtPubKeysWithOrigins(&m_wallet_descriptor.cache)};

    // BIP 174 wants the keys that the transaction's keys derive from, each with as many
    // steps in its path as its depth. Where the cache lacks the key at the last hardened
    // step, the descriptor hands out its root instead, and a root with hardened steps still
    // below it derives none of them; a key written without its origin has a path too short
    // for its depth. Neither can go in.
    std::erase_if(xpubs, [&](const auto& entry) {
        const auto& [origin, keys]{entry};
        return !DerivesKeyInPSBT(psbtx, origin, keys) ||

jeanpablojp commented at 7:00 PM on November 2, 2026:

Taken, thanks, with non-taproot entries compared as full keys rather than x-only.

in src/wallet/scriptpubkeyman.cpp:1545 in 437936e575

1540 | +               std::ranges::any_of(keys, [&](const CExtPubKey& key) { return origin.path.size() != size_t{key.nDepth}; });
1541 | +    });
1542 | +
1543 | +    for (const auto& [origin, keys] : xpubs) {
1544 | +        psbtx.m_xpubs[origin].insert(keys.begin(), keys.end());
1545 | +    }

fametrano commented at 6:20 PM on November 1, 2026:

walletcreatefundedpsbt and send both returned a PSBT decodepsbt refuses, in a wallet holding one xpub under two origins. MergeGlobalXPubs already keeps the first origin it sees (#35665), so this uses it. With this they decode, with one record, and the PR's tests still pass. In that wallet the origin that stays follows the pointer order of GetAllScriptPubKeyMans, so it can change across a restart.

    PartiallySignedTransaction candidates{CMutableTransaction{}};
    candidates.m_xpubs = std::move(xpubs);
    psbtx.MergeGlobalXPubs(candidates);

jeanpablojp commented at 7:00 PM on November 2, 2026:

Taken, thanks. MergeGlobalXPubs now takes the map, so the wallet calls it without the throwaway PSBT.

in test/functional/wallet_multisig_descriptor_psbt.py:134 in 437936e575 outdated

 129 | +        result = wallet.importdescriptors([
 130 | +            {"desc": descriptor(0), "active": True, "timestamp": "now"},
 131 | +            {"desc": descriptor(1), "active": True, "internal": True, "timestamp": "now"},
 132 | +        ])
 133 | +        assert all(r["success"] for r in result)
 134 | +        self._check_publishes_nothing(wallet, coordinator)

fametrano commented at 6:20 PM on November 1, 2026:

Not blocking: no edited bytes are needed, since two RPCs give the stranger's key the root's origin. This fails on the head because the root gets in, and passes with the derivation suggestion.

        self._check_publishes_nothing(wallet, coordinator)

        # A path in a PSBT is only a claim: this one gives a stranger's key the root's origin.
        stranger = coordinator.getaddressinfo(coordinator.getnewaddress(address_type="bech32"))["pubkey"]
        forged_desc = descsum_create(f"wpkh({keys[0]['origin']}{stranger})")
        psbt = coordinator.walletcreatefundedpsbt(
            inputs=[], outputs={wallet.getnewaddress(): 0.1, self.node.deriveaddresses(forged_desc)[0]: 0.1},
            feeRate=0.00010, bip32derivs=False)["psbt"]
        forged = self.node.descriptorprocesspsbt(psbt, [forged_desc], bip32derivs=True, finalize=False)["psbt"]
        processed = wallet.walletprocesspsbt(forged, sign=False)["psbt"]
        assert_equal(wallet.decodepsbt(processed)["global_xpubs"], [])

jeanpablojp commented at 7:00 PM on November 2, 2026:

Added, with a second forged output through tr(), since the taproot comparison had no test otherwise, and a check that both claims reach the PSBT.

in test/functional/wallet_multisig_descriptor_psbt.py:164 in 437936e575 outdated

 159 | +                                                   outputs={coordinator.getnewaddress(): 0.5},
 160 | +                                                   add_inputs=False, change_type=address_type, feeRate=0.00010)
 161 | +            assert funded["changepos"] != -1, "the case needs a change output"
 162 | +            # The receiving and change descriptors of an address type share their account key.
 163 | +            parent_desc = wallet.getaddressinfo(address)["parent_desc"]
 164 | +            self._check_global_xpubs(funded["psbt"], [parent_desc[parent_desc.index("["):]], wallet)

fametrano commented at 6:20 PM on November 1, 2026:

This fails on the head with Duplicate Key and passes with the suggestion in AddGlobalXpubs.

            self._check_global_xpubs(funded["psbt"], [parent_desc[parent_desc.index("["):]], wallet)

        # One xpub under two origins is one record: a repeated record makes the PSBT undecodable.
        account = wallet.derivehdkey("m/84h/1h/0h")
        two_origins = self.node.get_wallet_rpc(self.node.createwallet(
            wallet_name="two_origins", blank=True, disable_private_keys=True)["name"])
        result = two_origins.importdescriptors([
            {"desc": descsum_create(f"wpkh({account['origin']}{account['xpub']}/0/*)"),
             "active": True, "timestamp": "now"},
            {"desc": descsum_create(f"wpkh([00000000/84h/1h/0h]{account['xpub']}/1/*)"),
             "active": True, "internal": True, "timestamp": "now"},
        ])
        assert all(r["success"] for r in result)
        coordinator.sendtoaddress(two_origins.getnewaddress(), 1)
        self.generate(self.node, 1)
        funded = two_origins.walletcreatefundedpsbt(inputs=[], outputs={coordinator.getnewaddress(): 0.5}, feeRate=0.00010)
        assert_equal(len(two_origins.decodepsbt(funded["psbt"])["global_xpubs"]), 1)

jeanpablojp commented at 7:00 PM on November 2, 2026:

Added, with two more cases, the same xpub twice in one descriptor and a PSBT whose sender already lists the wallet's xpub under another origin. The second also made walletprocesspsbt return a PSBT that does not decode.

jeanpablojp force-pushed on Nov 2, 2026

fametrano commented at 8:00 AM on November 4, 2026: contributor

tACK 1cc67cf916

The four suggestions are in, and the new tests pin them. With each fix reverted on this head, wallet_multisig_descriptor_psbt.py fails: in test_one_record_per_xpub when the xpubs bypass MergeGlobalXPubs, and at the forged claims when DerivesKeyInPSBT stops comparing the derived key, even in the taproot map alone. Unmodified, the descriptor, PSBT, wallet and key_io unit suites and eight wallet and PSBT functional tests pass.

Not blocking: Ralph's first Minor here, the cost of deriving along unrelated paths, comes from my derivation suggestion. Beyond the 2 MB file I timed then, a 17 MB PSBT whose output lists 16,000 stranger keys at the root's origin, each 255 unhardened steps deep, kept walletprocesspsbt busy 73 to 96 s on a loaded 10-core machine. Ralph's fix would bound it, and I would take it here.

Commands and logs: https://gist.github.com/fametrano/1cb22c31d89e06cd722dadaec3dc7230/29b33df1cda87b91430efbb654e68b901f5f73ef

DrahtBot added the label Needs rebase on Nov 4, 2026

descriptor: add GetExtPubKeysWithOrigins

Give Descriptor a way to hand out the extended public keys it is built
from, each keyed by its origin.

Each BIP 32 key expression gives the key at its last hardened step when
the descriptor cache holds that key, and its root key otherwise. The key
at the last hardened step is the one the expression's unhardened
children derive from. A key origin written in the descriptor takes the
place of the root's fingerprint and is prepended to the path. A musig()
expression hands out its participants' keys, and a plain public key has
none to give.

The keys carry the network's version bytes, since BIP 174 serializes
those bytes as part of each PSBT_GLOBAL_XPUB key and a key parsed from a
descriptor lacks them. The wallet starts using this in the next commit.

31b2f8cb00

wallet: fill PSBT_GLOBAL_XPUB

BIP 174 lets a PSBT carry the extended public keys that the keys in its
inputs and outputs derive from, so that a signer holding no wallet
state can rebuild the scripts from the PSBT alone. The PSBT code
already serializes, deserializes and merges PSBT_GLOBAL_XPUB, but the
wallet never writes it.

Have DescriptorScriptPubKeyMan::FillPSBT add the extended keys of every
descriptor that contributed an input or an output, unless bip32derivs
is false. That covers walletcreatefundedpsbt, walletprocesspsbt, send,
sendall, psbtbumpfee and the GUI, which all go through
CWallet::FillPSBT.

Only keys that BIP 174 allows go in. A key has to derive, through
unhardened steps alone, one of the keys the descriptor produced that the
PSBT gives a derivation path for, and its path has to have as many steps
as its depth. The derivation is checked by deriving the key and
comparing it with the listed one, since keys can share an origin. The
first rule leaves out the root key a descriptor falls back to when its
cache lacks the key at the last hardened step, which is always the case
with /*h derivation. The second leaves out a non-master key written in
the descriptor without its origin.

A path in a PSBT is only what its sender claims, so a listed key counts
only at the origin the descriptor gave it, and nothing is derived along
any other path, however many the PSBT lists.

A key the PSBT already carries keeps the origin it has there, since
BIP 174 allows one record per key. MergeGlobalXPubs already does this
when PSBTs are combined, and now takes the xpubs to merge rather than
a whole PSBT, so that the wallet adds its own through it.

A PSBT from the wallet now carries the extended keys of the
descriptors it uses, and whoever holds it can derive their addresses.
Setting bip32derivs to false in walletcreatefundedpsbt or
walletprocesspsbt leaves them out, along with the derivation paths.

The BIP 174 updater test vector now carries the extended keys of its
descriptors.

118c6d1f73

jeanpablojp force-pushed on Nov 4, 2026

DrahtBot removed the label Needs rebase on Nov 4, 2026

in src/wallet/scriptpubkeyman.cpp:1532 in 118c6d1f73

1527 | +        }
1528 | +        return child.pubkey == pubkey;
1529 | +    });
1530 | +}
1531 | +
1532 | +void DescriptorScriptPubKeyMan::AddGlobalXpubs(PartiallySignedTransaction& psbtx) const

vicjuma commented at 7:10 AM on November 7, 2026:

Global xpubs populated successfully after this change

master branch

psbt-global-xpubs branch

in src/rpc/rawtransaction.cpp:1951 in 118c6d1f73

1947 | @@ -1948,7 +1948,7 @@ static RPCMethod joinpsbts()
1948 |          for (const PSBTOutput& output : psbt.outputs) {
1949 |              merged_psbt.AddOutput(output);
1950 |          }
1951 | -        merged_psbt.MergeGlobalXPubs(psbt);
1952 | +        merged_psbt.MergeGlobalXPubs(psbt.m_xpubs);

vicjuma commented at 8:51 AM on November 7, 2026:

Good call 👍

in src/script/descriptor.cpp:272 in 118c6d1f73

 265 | @@ -266,6 +266,11 @@ struct PubkeyProvider
 266 |      /** Return the extended public key for this PubkeyProvider, if it has one. */
 267 |      virtual std::optional<CExtPubKey> GetRootExtPubKey() const = 0;
 268 |  
 269 | +    /** The extended public keys this provider is built from, each keyed by the origin
 270 | +     *  that leads to it: the key at the deepest hardened step when the cache holds it,
 271 | +     *  and the root otherwise. */
 272 | +    virtual std::map<KeyOriginInfo, std::set<CExtPubKey>> GetExtPubKeysWithOrigins(const DescriptorCache* cache) const = 0;

vicjuma commented at 9:17 AM on November 7, 2026:

Weak nit :-) the naming sounds like an accessor for a member variable, the Get part i.e

in src/script/descriptor.cpp:381 in 118c6d1f73

 374 | @@ -370,6 +375,17 @@ class OriginPubkeyProvider final : public PubkeyProvider
 375 |      {
 376 |          return m_provider->GetRootExtPubKey();
 377 |      }
 378 | +    std::map<KeyOriginInfo, std::set<CExtPubKey>> GetExtPubKeysWithOrigins(const DescriptorCache* cache) const override
 379 | +    {
 380 | +        std::map<KeyOriginInfo, std::set<CExtPubKey>> ret;
 381 | +        for (auto& [suborigin, xpubs] : m_provider->GetExtPubKeysWithOrigins(cache)) {

vicjuma commented at 9:29 AM on November 7, 2026:

Weak nit: Read-only operation on named variables warrants explicit const auto& as can be seen throughout the codebase

- for (auto& [suborigin, xpubs] : m_provider->GetExtPubKeysWithOrigins(cache)) {
+ for (const auto& [suborigin, xpubs] : m_provider->GetExtPubKeysWithOrigins(cache)) {

in src/script/descriptor.cpp:385 in 118c6d1f73

 380 | +        std::map<KeyOriginInfo, std::set<CExtPubKey>> ret;
 381 | +        for (auto& [suborigin, xpubs] : m_provider->GetExtPubKeysWithOrigins(cache)) {
 382 | +            KeyOriginInfo origin{suborigin};
 383 | +            origin.fingerprint = m_origin.fingerprint;
 384 | +            origin.path.insert(origin.path.begin(), m_origin.path.begin(), m_origin.path.end());
 385 | +            ret[origin].merge(xpubs);

vicjuma commented at 10:02 AM on November 7, 2026:

Weak nit: with the explicit const auto& comes

- ret[origin].merge(xpubs);
+ ret[origin].insert(xpubs.begin(), xpubs.end());

the only place I have seen .merge used in the codebase is src/script/miniscript.h:1544: key_set.merge(*sub); where the sets are mutated. The sets used here are not large enough to warrant optimization, and .insert() has no side effects unlike .merge()

in src/script/descriptor.cpp:896 in 118c6d1f73

 888 | @@ -849,6 +889,16 @@ class MuSigPubkeyProvider final : public PubkeyProvider
 889 |      {
 890 |          return std::nullopt;
 891 |      }
 892 | +    std::map<KeyOriginInfo, std::set<CExtPubKey>> GetExtPubKeysWithOrigins(const DescriptorCache* cache) const override
 893 | +    {
 894 | +        std::map<KeyOriginInfo, std::set<CExtPubKey>> ret;
 895 | +        for (const auto& prov : m_participants) {
 896 | +            for (auto& [origin, keys] : prov->GetExtPubKeysWithOrigins(cache)) {

vicjuma commented at 10:11 AM on November 7, 2026:

Same here, explicit const on auto& and .insert() vs mutable auto& and .merge()

in src/script/descriptor.cpp:1185 in 118c6d1f73

1176 | @@ -1127,6 +1177,23 @@ class DescriptorImpl : public Descriptor
1177 |          }
1178 |      }
1179 |  
1180 | +    // NOLINTNEXTLINE(misc-no-recursion)
1181 | +    std::map<KeyOriginInfo, std::set<CExtPubKey>> GetExtPubKeysWithOrigins(const DescriptorCache* cache) const override
1182 | +    {
1183 | +        std::map<KeyOriginInfo, std::set<CExtPubKey>> ret;
1184 | +        for (const auto& p : m_pubkey_args) {
1185 | +            for (auto& [origin, keys] : p->GetExtPubKeysWithOrigins(cache)) {

vicjuma commented at 10:12 AM on November 7, 2026:

Same

in src/script/descriptor.cpp:1190 in 118c6d1f73

1185 | +            for (auto& [origin, keys] : p->GetExtPubKeysWithOrigins(cache)) {
1186 | +                ret[origin].merge(keys);
1187 | +            }
1188 | +        }
1189 | +        for (const auto& sub : m_subdescriptor_args) {
1190 | +            for (auto& [origin, keys] : sub->GetExtPubKeysWithOrigins(cache)) {

vicjuma commented at 10:12 AM on November 7, 2026:

Same

vicjuma commented at 10:21 AM on November 7, 2026: contributor

Concept ACK, global xpubs are optional, so including them is harmless, and wallets or signers that require them can use them

wallet: fill PSBT_GLOBAL_XPUB #36154

External sites

Reviews

Conflicts

LLM Linter (✨ experimental)