external signer: verify PSBT is reliable after signing it #35358

Seeking Concept ACK

A compromised or "rogue" external signer could return a PSBT with silently altered outputs or use unsafe sighash types that do not commit to all outputs (perhaps unlikely, but a buggy signer could also change it). This PR adds a post-signing validation step in External::SignTransaction that compares the signed PSBT returned by the external signer against the original one before accepting it. Without these checks, it is not super friendly (e.g. using RPC) to manually verify the same.

Note: the added tests were partially vibe-coded.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35358.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #33112 (wallet: relax external_signer flag constraints, add musig2 test (partial) by Sjors)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

• walletcreatefundedpsbt([], {dest: 0.5}, 0, {...}, True) in test/functional/wallet_signer.py

^{2026-05-22 13:11:37}

Should SIGHASH_ALL | SIGHASH_ANYONECANPAY really be considered safe here?

The PR protects outputs, and ANYONECANPAY|ALL does commit to all outputs. But ANYONECANPAY also means the signed input can be included in a larger transaction with additional inputs that the wallet did not authorize. If the expected sighash is SIGHASH_ALL (or SIGHASH_DEFAULT for taproot), a rogue signer silently downgrading to ANYONECANPAY|ALL would pass this check while producing a signature with strictly weaker commitment.

Is allowing ANYONECANPAY an intentional design choice here, or should the check require the returned sighash to match the PSBT's per-input sighash field when present, and fall back to the default policy otherwise?

Should SIGHASH_ALL | SIGHASH_ANYONECANPAY really be considered safe here?

Yes.

It is not a problem for us if someone else adds more inputs. We only care that the inputs we want spend and the outputs we want to send to are all in the transaction. If someone else adds more inputs, the inputs we want to spend are still there, and the outputs we want to create are still committed to. There is no effect to the wallet, hence it is safe.

Concept OK I guess

I think we generally assume an external signer to be more trusted/secure, so we don't really do checks on their output. An external signer can always leak private keys or steal the user's coins and there isn't much we can do about it.