psbt: verify final scripts in FinalizePSBT #36343

finalizepsbt can return complete: true and a hex for a PSBT whose PSBT_IN_FINAL_SCRIPTWITNESS (or final scriptSig) is invalid.

FinalizePSBT counts an input as complete when SignPSBTInput succeeds. If the input already has final fields, PSBTInput::FillSignatureData marks the sigdata complete and ProduceSignature returns true early (sign.cpp:752), before the script is verified. So the existing final fields are trusted as-is.

To reproduce, take a signed P2WPKH v0 PSBT that has a non_witness_utxo and flip one bit of the signature inside the final witness:

• finalizepsbt returns complete: true and a hex
• analyzepsbt says the input is not final, next: finalizer
• testmempoolaccept on the hex fails with mempool-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)

Inputs that only have a witness_utxo already come out incomplete because of the require_witness_sig check, so this affects inputs with a non_witness_utxo.

#30357 fixed the same problem in FillPSBT by checking PSBTInputSignedAndVerified, and #33014 did it for descriptorprocesspsbt. This does the same in FinalizePSBT. Taproot verification needs every spent output. If one is missing, that input already fails in SignPSBTInput with MISSING_INPUTS and makes the PSBT incomplete, so the extra check doesn't change the result in that case.

The new test in rpc_psbt.py builds that PSBT and checks that finalizepsbt returns complete: false with no hex. It fails on master and passes with the fix. wallet_musig.py and the psbt_wallet_tests / script_tests unit tests still pass.

One behaviour change: a final script that is valid by consensus but non-standard now gives complete: false. FillPSBT and descriptorprocesspsbt already behave that way, since they check with the same standard flags.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Tested ACK 417a548cf23460d87d20ea5d3e1c9c7bcc5af03f

The new test fails with the one-line fix reverted and passes with it.

I also flipped a signature bit in the final fields of pkh, sh-wpkh, wpkh and taproot key-path inputs, and of the first input of a three-input PSBT whose other inputs were only partially signed. Without the fix, finalizepsbt returned complete=true and a hex whenever the input also had a non_witness_utxo, and testmempoolaccept and generateblock (submit=false) both rejected that hex. With the fix, all of them are complete=false with no hex. Valid, standard signatures give the same result as before.

As the description says, a final witness that is consensus-valid but non-standard is now complete=false too. I checked with a high-S signature: before, it was extracted when the input had a non_witness_utxo, and now it is not. This matches walletprocesspsbt, so I think it is right, but it may deserve a release note.

Thanks for the thorough testing. Agreed on the release note, added doc/release-notes-36343.md in a separate commit (7148c65121) covering the non-standard case. The code commit is unchanged.

reACK 7148c65121cbe7225acc9ab789146c3632f9de97

Only the release note is new since my Tested ACK, and it matches the code.