#36154 wallet: fill PSBT_GLOBAL_XPUB
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36154 wallet: fill PSBT_GLOBAL_XPUB | Ready | 0 | Neutral | 160 + 251 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Populates the BIP 174 PSBT_GLOBAL_XPUB map when the wallet creates or updates PSBTs through CWallet::FillPSBT. Adds a Descriptor::GetExtPubKeysWithOrigins interface across descriptor and pubkey provider classes to retrieve extended public keys stamped with network version bytes. Only keys deriving an input or output in the PSBT through unhardened steps are included, and the field is omitted when bip32derivs is set to false.
Problem: Core's wallet never writes extended public keys into PSBT_GLOBAL_XPUB even though BIP 174 defines the field and Core's PSBT parser supports it. Stateless external signers without internal wallet state cannot verify multisig policies or change derivation paths from the PSBT alone, causing some hardware wallets to reject transactions and forcing coordinators to manually inject the fields.
Evidence of importance:
- PR description documents simulator tests where BitBox02 refuses to sign without the field ('This BitBox02 is not one of the cosigners') and Trezor T receives zeroed chain codes and empty derivation paths
- Links to Specter Desktop source code showing an explicit workaround to populate PSBT_GLOBAL_XPUB after receiving PSBTs from Core
- Closes issue #27583 ('psbt: set global_xpubs (at least for multisig descriptors)')
Scope: Adds GetExtPubKeysWithOrigins virtual method across Descriptor, PubkeyProvider, and all derived classes, updating DescriptorScriptPubKeyMan to query it during FillPSBT.
Risk of a mistake: medium (wallet, rpc_api, tests, docs). Incorrect global xpub derivation or metadata in PSBTs could cause hardware wallets to reject transactions or sign invalid witness data.
Changed since the previous assessment: Expanded from multisig-only descriptors to general descriptors including MusigPubkeyProvider, added DerivesKeyInPSBT filtering to only inject keys that derive keys present in the transaction, and updated RPC spend documentation for bip32derivs.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: achow101 to re-review after author addressed all comments 2026-10-30
Agreement
Neutral
objections settled, nobody has spoken for the PR
Thread notes: The author addressed all of achow101's objections and suggestions and pushed an updated branch.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| achow101 (member) | objection, approach | Restricts global xpubs to multisig descriptors instead of handling descriptors and PSBTs generically | fixed · fix pushed · dropped the multisig restriction across descriptors and PSBTs | changed | yes | 2026-09-07: 'PSBTs and Descriptors are generic objects, we should avoid doing things that are specific to one type of descriptor or psbt.' Status from: 2026-10-30: jeanpablojp: 'dropped the multisig restriction' |
| achow101 (member) | suggestion, interface | Return the map directly rather than taking it as an output parameter | fixed · fix pushed · returns the map directly | answered | yes | 2026-09-07: 'This should return the map rather than taking it as an output parameter.' Status from: 2026-10-30: jeanpablojp: 'it returns the map' |
| achow101 (member) | suggestion, interface | Do not pass SigningProvider since the last hardened key is either cached or does not exist | fixed · fix pushed · takes only the cache instead of SigningProvider | answered | yes | 2026-09-07: '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.' Status from: 2026-10-30: jeanpablojp: 'takes only the cache' |
| achow101 (member) | objection, approach | Makes assumptions about caller usage instead of returning the root and its fingerprint when normalized keys cannot be obtained | fixed · fix pushed · returns root and fingerprint for unnormalized keys | answered | yes | 2026-09-07: '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.' Status from: 2026-10-30: jeanpablojp: 'it now returns the root and its fingerprint, /*h included' |
| achow101 (member) | suggestion, approach | Query the cache directly and return the root if missing, rather than calculating the last hardened index | fixed · fix pushed · asks cache directly and falls back to root | answered | yes | 2026-09-07: '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.' Status from: 2026-10-30: jeanpablojp: 'it asks the cache and falls back to the root' |
| achow101 (member) | objection, correctness | MuSig descriptors omit participant extended public keys | fixed · fix pushed · merges participant xpubs for MuSig descriptors | changed | yes | 2026-09-07: 'This is incorrect. It should traverse the participants and get their xpubs.' Status from: 2026-10-30: jeanpablojp: 'It now merges whatever its participants return' |
Participants:
- Sjors (member, neutral, 2 statements 2026-09-03 to 2026-09-04): asked for description clarifications and noted global xpub utility for non-HWI flows
- achow101 (member, objection, 5 statements 2026-09-07 to 2026-09-07): objected to multisig-only restriction, MuSig omission, and interface design
- fametrano (none, neutral, 1 statements 2026-10-02 to 2026-10-02): offered a rebased branch addressing achow101's comments
Files
252 lines under test/bench/ci.
- test/functional/wallet_multisig_descriptor_psbt.py +125/-0
- src/test/descriptor_tests.cpp +119/-0
- src/script/descriptor.cpp +67/-0
- src/wallet/scriptpubkeyman.cpp +53/-0
- doc/release-notes-36154.md +11/-0
- src/script/descriptor.h +10/-0
- src/key_io.cpp +7/-0
- src/key_io.h +6/-0
- src/wallet/rpc/spend.cpp +2/-2
- src/wallet/scriptpubkeyman.h +4/-0
- test/functional/wallet_musig.py +4/-0
- src/wallet/test/psbt_wallet_tests.cpp +1/-1
- src/test/fuzz/descriptor_parse.cpp +1/-0
- src/wallet/test/walletload_tests.cpp +1/-0
Card
The short text the ranking pass compares across PRs.
Bitcoin Core now populates the BIP 174 PSBT_GLOBAL_XPUB field during wallet PSBT creation and filling. Without this field, stateless external signers such as the BitBox02 cannot reconstruct multisig scripts or verify change outputs from the PSBT alone and refuse to sign. The PR resolves issue 27583 and removes the need for external coordinators like Specter to manually inject global xpubs into PSBTs produced by Core. Key inclusion is filtered to keys that derive transaction inputs or outputs, and can be disabled via the bip32derivs option.
Data
thread read through 2026-10-30 (14 statements), head 437936e575
labels: Wallet
Categories
Wallet
P2 · bug fix (ranking pass: Moved up: stateless-signer refusals are a concrete interoperability failure, ranked with the other PSBT correctness fixes.)
Populating PSBT_GLOBAL_XPUB resolves confusing signing failures and interoperability issues with stateless hardware signers like BitBox02 and Trezor. The description cites hardware wallet rejections ('This BitBox02 is not one of the cosigners') and downstream workarounds in Specter Desktop, addressing long-standing issue #27583.
Membership: Tagged with Wallet label and modifies CWallet::FillPSBT, DescriptorScriptPubKeyMan, and descriptor interfaces to populate BIP 174 global xpubs.
Factors: security/stability 0, bug 2, performance 0, user value 2, leverage 0