#36114 wallet: harden external signer psbt processing, revamp mock
https://github.com/bitcoin/bitcoin/pull/36114 · · +276/-66 in 5 files, 4 commits · labels: Wallet · draft
Goal
- Prevent buggy or compromised hardware wallets from tampering with outputs or signing with unsafe sighash types
- Improve test coverage for complex external signer workflows including multisig and misbehaving signers
This PR hardens external signer processing by merging the PSBT returned from the signer instead of replacing the node's PSBT, and by rejecting responses that declare or contain signatures using unsafe sighash types such as SIGHASH_NONE or SIGHASH_SINGLE. It also replaces the static external signer functional test mock with an offline descriptor wallet node capable of producing real signatures and simulating varied signer behaviors.
Problem: Previously, the wallet replaced its PSBT wholesale with the external signer's response, leaving users vulnerable to a buggy or compromised hardware wallet modifying outputs or using sighash types that allow third-party output tampering. In addition, the test mock could only echo pre-baked PSBTs, preventing tests for misbehaving signers or multi-party signing.
Category: Wallet (#17 of 84)
P2 · fund safety
- P2 because it prevents external signers from silently dropping or altering outputs and using unsafe sighash
- Protects hardware wallet users from potential fund loss caused by malicious or buggy signer firmware
- Significantly expands functional test capabilities for advanced multi-party signing setups
P2 because it closes a fund-safety gap where external signers could tamper with transaction outputs or sign with SIGHASH_NONE without wallet detection. As Sjors notes, merging compensates for and thwarts signers dropping outputs, reducing values, or changing scripts, while the revamped mock provides leverage for testing complex wallet signing flows.
Membership: Touches src/external_signer.cpp and external signer functional tests in test/functional/wallet_signer.py.
Factors: security/stability 2, bug 1, performance 0, user value 2, leverage 2
Reviewability: Paused: Waiting on author
- Waiting on author to address a demonstrated bypass where finalized responses evade the sighash check
jeanpablojp demonstrated that a finalized PSBT response bypasses the unsafe sighash check, and this material finding has been unanswered for 9 days.
Author status: silent since 2026-09-08 after force-pushing a rebase
Open concerns:
- jeanpablojp noted that finalized PSBT responses bypass FindUnsafeSighashType because BIP 174 clears the input fields where sighash types and partial signatures are checked, allowing SIGHASH_NONE transactions to pass through
- jeanpablojp raised nits about v0 PSBT failure error messages and splitting the ECDSA and taproot sighash test cases
Agreement: Positive
- Concept approval with detailed testing of edge cases (jeanpablojp)
- Unaddressed finding: finalized responses bypass the unsafe sighash check, no author reply (jeanpablojp)
Concept ACK from jeanpablojp with an open question about covering finalized responses; author has not replied.
The direction is supported by jeanpablojp who actively tested edge cases with the new mock, though an author response is needed regarding finalized PSBT handling.
- jeanpablojp provided a Concept ACK and detailed test findings showing SIGHASH_NONE passing when the mock returns a finalized PSBT
Review verdicts (DrahtBot): 0
- Concept ACK: jeanpablojp
Dependencies
Enables:
- absorbs and supersedes #35358
- enables functional testing for multisig and MuSig2 external signer setups
Files
271 lines under test/bench/ci.
- test/functional/wallet_signer.py +108/-55
- test/functional/mocks/signer.py +98/-10
- src/external_signer.cpp +40/-1
Uncertainties
- Whether the author intends to support or reject finalized responses from external signers
Card
This PR hardens external signer PSBT handling in the wallet and revamps the functional test mock signer. By merging the signer's returned PSBT rather than replacing the wallet's PSBT wholesale, and by rejecting unsafe sighash types like SIGHASH_NONE, it protects users from malicious or malfunctioning hardware signers tampering with outputs or fees. The mock signer is upgraded to use an independent offline descriptor wallet, enabling robust tests for misbehaving signers and multisig workflows. The change has Concept ACK from jeanpablojp, who noted that finalized PSBT responses currently bypass the sighash check, with an author reply pending.