wallet: Fix FillPSBT failing to sign owned inputs when UTXOs disagree #35747

nervana21 commented at 4:45 PM on July 19, 2026: contributor

tl;dr

Previously, the wallet could fail to sign an intended spend when an input's non_witness_utxo and witness_utxo disagreed. This PR fixes FillPSBT by routing its UTXO-selection logic through GetUTXO.

Problem

A PSBT input can carry both a non_witness_utxo and a witness_utxo. GetUTXO prefers a verified non_witness_utxo and does not fall back to witness_utxo when the non_witness_utxo is present but unusable.

Previously, FillPSBT selected the SigningProvider from the witness_utxo whenever it was set. When the two disagreed, FillPSBT loaded keys for the witness_utxo script while SignPSBTInput signed the non_witness_utxo spend. This could lead to an incomplete input even though the wallet owns the appropriate non_witness_utxo script.

Fix

Route FillPSBT through GetUTXO so it shares SignPSBTInput's prefer/reject rule.

Behavior change

FillPSBT now uses GetUTXO for script selection, so an input whose non_witness_utxo is present but unusable (out-of-range or txid-mismatched) returns MISSING_INPUTS instead of first selecting a script from witness_utxo.

SignPSBTInput rejects the same input with MISSING_INPUTS, and FillPSBT already propagates any non-INCOMPLETE error for the remaining ScriptPubKeyMans and inputs. The change is that the failure is detected at script selection rather than after a mistaken key lookup.

Well-formed PSBTs are unaffected, since witness_utxo and non_witness_utxo resolve to the same script. Txid handling is also tightened in the selection path. A txid-mismatched non_witness_utxo is now rejected by GetUTXO rather than used for script selection when no witness_utxo is present.

Testing

fillpsbt_signs_despite_conflicting_witness_utxo covers the regression. psbt2_getutxo pins the GetUTXO prefer/reject rules the fix relies on.

Follow-up

SignPSBTInput, PSBTInputSignedAndVerified, and decodepsbt fee totaling still reimplement GetUTXO's logic and risk the same divergence. Refactoring them is left for a follow-up.

DrahtBot added the label Wallet on Jul 19, 2026

DrahtBot commented at 4:45 PM on July 19, 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/35747
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/35747.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/35747#author-ralph

Reviews

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

Type	Reviewers
ACK	8144225309

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:

#36382 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36382.svg"> (rpc: flag invalid taproot leaf control blocks in decodepsbt by fametrano)
#36224 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36224.svg"> (test: Add test coverage for PartiallySignedTransaction::Merge() by nebula-21)
#36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
#35569 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35569.svg"> (Encapsulation for CTransaction by purpleKarrot)

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.

DrahtBot added the label CI failed on Jul 19, 2026

DrahtBot commented at 5:45 PM on July 19, 2026: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/29695490423/job/88215409202 LLM reason (✨ experimental): CI failed because clang-tidy reported a bugprone-argument-comment error (commented argument name mismatch in wallet/test/psbt_wallet_tests.cpp), treated as warnings-as-errors.

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

nervana21 force-pushed on Jul 20, 2026

DrahtBot removed the label CI failed on Jul 20, 2026

in src/wallet/scriptpubkeyman.cpp:1376 in 0a13744b73 outdated

1368 | @@ -1369,18 +1369,15 @@ std::optional<PSBTError> DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran
1369 |          }
1370 |  
1371 |          // Get the scriptPubKey to know which SigningProvider to use
1372 | -        CScript script;
1373 | -        if (!input.witness_utxo.IsNull()) {
1374 | -            script = input.witness_utxo.scriptPubKey;
1375 | -        } else if (input.non_witness_utxo) {
1376 | -            if (input.prev_out >= input.non_witness_utxo->vout.size()) {

vicjuma commented at 10:15 AM on August 3, 2026:

Why do I feel as if these checks are enough? Is there any motivation for someone intending to disagree non_witness_utxo and witness_utxo? Any risk involved? I have explored some possible paths and saw that at no particular point can this be internal.

nervana21 commented at 3:28 PM on August 3, 2026:

Hi vicjuma, thanks for your review!

Why do I feel as if these checks are enough?

For SignPSBTInput they are but FillPSBT wasn't using that same rule for key selection though, which is the gap this PR closes.

Is there any motivation for someone intending to disagree non_witness_utxo and witness_utxo?

IIUC, a malicious or otherwise buggy PSBT coordinator could add a faulty witness_utxo to an honest peer's already-existing non_witness_utxo. That would cause FillPSBT to choose the faulty witness_utxo for key selection instead of the desired non_witness_utxo, so signing could fail even though we owned the right input.

Any risk involved?

As I see it, the risk with the old FillPSBT behavior is just that signing fails or stays incomplete. We'd never sign the wrong utxo.

I have explored some possible paths and saw that at no particular point can this be internal.

That's my understanding as well. I think this only corrects the external/mutated PSBT case.

DrahtBot added the label Needs rebase on Aug 15, 2026

nervana21 force-pushed on Aug 15, 2026

DrahtBot removed the label Needs rebase on Aug 15, 2026

DrahtBot added the label Needs rebase on Aug 18, 2026

test, doc: Pin PSBTInput::GetUTXO prefer rules

Document and test prefer-non_witness, reject OOB/txid mismatch, and no
witness fallthrough when fields disagree, before migrating callers.

4261345dd8

wallet: Select FillPSBT SigningProvider via GetUTXO

Match SignPSBTInput by preferring a verified non_witness_utxo and
rejecting a bad non_witness_utxo instead of falling back to a
witness_utxo.

Add a regression test.

7aed7f8d17

nervana21 force-pushed on Sep 19, 2026

nervana21 commented at 12:18 AM on September 19, 2026: contributor

Resolved merge conflicts and rebased off master.

DrahtBot added the label CI failed on Sep 19, 2026

DrahtBot removed the label Needs rebase on Sep 19, 2026

DrahtBot removed the label CI failed on Sep 19, 2026

8144225309 commented at 3:52 AM on October 5, 2026: contributor

ACK 7aed7f8d176c7b7d4c147df45e194994c3bf0fea

Reproduced on master first: fillpsbt_signs_despite_conflicting_witness_utxo fails there and passes with the fix. Reverting just the scriptpubkeyman.cpp change brings the failure back. Full unit and functional suites pass.

Routing FillPSBT through GetUTXO matches SignPSBTInput's selection, so when the witness_utxo disagrees with the non_witness_utxo, the wallet no longer loads keys for the wrong script. Nice fix, thanks.