#36343 psbt: verify final scripts in FinalizePSBT
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36343 psbt: verify final scripts in FinalizePSBT | Ready | 1 | Strong | 13 + 28 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
Check PSBTInputSignedAndVerified for each input in FinalizePSBT before marking it complete. This prevents FinalizePSBT from unconditionally trusting pre-existing final scriptSig or scriptWitness fields without verifying them against standard execution flags.
Problem: When a PSBT input has a non-witness UTXO and already contains final script fields, SignPSBTInput accepts them and ProduceSignature returns early before script verification. As a result, finalizepsbt can report complete: true and return a raw transaction hex whose script fails mempool verification, contradicting analyzepsbt which reports the input as not final.
Evidence of importance:
- PR description demonstrates reproduction: flipping a bit in the final witness of a P2WPKH PSBT causes finalizepsbt to output complete: true with a hex that testmempoolaccept rejects with 'mempool-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)', while analyzepsbt marks the input not final
- Author cites prior merged fixes applying the same check to other PSBT entry points: #30357 for FillPSBT and #33014 for descriptorprocesspsbt
Scope: Small logic change in FinalizePSBT with an added functional test and release note.
Risk of a mistake: medium (rpc_api). A flaw in final script verification could mistakenly reject valid PSBTs or block transaction finalization in RPC workflows.
Changed since the previous assessment: Added release notes document doc/release-notes-36343.md.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- author: response to Bicaru20's suggestion on analyzepsbt next role 2026-10-30
Agreement
Strong
substantive support, no open objection (fametrano, fametrano)
Thread notes: Added Bicaru20's Concept ACK and suggestion regarding updating analyzepsbt to report updater when verification of final scripts fails.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fametrano (none) | suggestion, maintenance | Add a release note describing the behavior change where consensus-valid but non-standard scripts now return complete=false. | accepted · fix pushed · reviewer confirmed the added release note matches the code | answered | yes | 2026-10-02: 'it may deserve a release note' Status from: 2026-10-03: fametrano: 'Only the release note is new since my Tested ACK, and it matches the code.' |
| Bicaru20 (contributor) | suggestion, interface | Update AnalyzePSBT or SignPSBTInput so that analyzepsbt returns next: updater instead of finalizer when final scriptSig or scriptWitness fields fail verification | open | answered | no | 2026-10-30: 'Since the final script_sig or final script_witness failed verification and in the same test we see that when trying to finalize the psbt it fails, I belvie this field on `analyzepsbt` can be missleading and should return `updater`.' |
Support:
- fametrano (none) (Tested ACK, 2026-10-02) [both]: verified the bugfix on pkh, sh-wpkh, wpkh, and taproot inputs; verified non-standard inputs match walletprocesspsbt behavior
- fametrano (none) (ACK, 2026-10-03) [read: doc/release-notes.md]: Only the release note is new since my Tested ACK, and it matches the code.
- l0rinc (member) (Concept ACK, 2026-10-28): (no reason given) [not substantive]
- Bicaru20 (contributor) (Concept ACK, 2026-10-30) [read: src/node/psbt.cpp, test/functional/rpc_psbt.py]: (no reason given) [not substantive]
Participants:
- fametrano (none, support, 2 statements 2026-10-02 to 2026-10-03): Tested ACK across multiple input types; reACKed after confirming release note was added
- l0rinc (member, support, 1 statements 2026-10-28 to 2026-10-28): Concept ACK
- Bicaru20 (contributor, support, 1 statements 2026-10-30 to 2026-10-30): Concept ACK; suggested updating analyzepsbt or SignPSBTInput so next returns updater instead of finalizer when final fields fail verification
Files
28 lines under test/bench/ci.
- test/functional/rpc_psbt.py +28/-0
- doc/release-notes-36343.md +10/-0
- src/psbt.cpp +3/-1
Card
The short text the ranking pass compares across PRs.
FinalizePSBT now verifies final scriptSig and scriptWitness data using PSBTInputSignedAndVerified before marking an input complete. Previously, inputs with a non_witness_utxo and existing final fields were trusted without script checks, causing finalizepsbt to return complete: true along with raw transaction hex that fails mempool verification. This aligns FinalizePSBT with fixes previously made to FillPSBT (#30357) and descriptorprocesspsbt (#33014). An intentional side effect is that inputs with consensus-valid but non-standard final scripts will now return complete: false.
Data
thread read through 2026-10-30 (5 statements), head 7148c65121
labels: PSBT
Categories
RPC / REST / ZMQ
P3 · bug fix (ranking pass: Moved up 7: finalizepsbt reporting complete with unusable hex misleads callers about a transaction's state.)
Prevents finalizepsbt from falsely returning 'complete: true' and emitting an invalid raw transaction hex when pre-existing final scripts fail verification. As the reproduction notes, an invalid witness could previously cause finalizepsbt to claim completion while testmempoolaccept rejected the transaction with a script verification failure.
Membership: Modifies FinalizePSBT behavior exposed by the finalizepsbt RPC without wallet code, which the wallet category explicitly routes to rpc.
Factors: security/stability 1, bug 1, performance 0, user value 1, leverage 0