wallet: harden external signer psbt processing, revamp mock #36114

pull Sjors wants to merge 5 commits into bitcoin:master from Sjors:2026/08/external-signer-mock changing 8 files +294 −70
  1. Sjors commented at 11:27 AM on August 28, 2026: member

    The current external signer functional test mock can only echo a prepared PSBT. This gets in the way of testing more complicated scenarios, i.e. misbehaving signers and (MuSig2) multisig.

    Commit test: have external signer mock use a wallet revamps the mock signer by handing it its own node and wallet. The (offline) mock wallet has the private keys, while the test uses the watch-only version.

    Commit external_signer: merge PSBT response instead of replacing lets the mock signer manipulate the PSBT it returns. It can drop outputs, reduce their value and change the script. Not all manipulations are malicious, e.g. a signer may simply not echo all PSBT fields, which is also covered. All this can be compensated for (thwarted) by merging the external signer PSBT instead of discarding the original.

    Commit external_signer: reject unsafe sighash types adds scenarios where devices use an unsafe sighash type.

    This PR absorbs the scenarios from #35358, but is better able to test them thanks to the new mock signer implementation.

    This PR does not intend to fully harden against a malicious external signer, and can't protect against a malicious HWI (equivalent) process running as the same user.

    Based on:

  2. psbt: preserve sighash type when merging inputs
    `PSBTInput::Merge` copies every optional input field from the other
    input when it is absent locally, except `PSBT_IN_SIGHASH_TYPE`. So
    `combinepsbt` silently drops the sighash type whenever the first PSBT
    does not carry it, making the result depend on the argument order.
    
    The field is what lets finalizers enforce the sighash type of existing
    signatures (BIP 174). When it is lost, `FinalizePSBT` falls back to the
    default type (`SIGHASH_ALL`, or `SIGHASH_DEFAULT` for taproot inputs),
    rejects signatures made with any other type as a sighash mismatch, and
    the PSBT can no longer be finalized. Combining a PSBT signed with
    `ALL|ANYONECANPAY` after a merely updated copy of the same PSBT
    reproduces this: `finalizepsbt` reports it as incomplete, while the
    reverse order finalizes and broadcasts fine.
    
    Merge the sighash type like the other optional fields, keeping the one
    already present, and test both combine orders.
    7c79fd2e01
  3. psbt: fix rendering for invalid long sighash type field
    The PSBT sighash type field is a 32 bit unsigned integer in BIP 174,
    signed in PSBTInput, and it is not validated when deserialized.
    
    decodepsbt incorrectly truncates this field before looking up its
    name. Fix that and add a test.
    9d774ffaa5
  4. test: have external signer mock use a wallet
    The external signer mock previously replayed a PSBT that the test
    prepared in advance. This makes it difficult to test more complicated
    scenarios like a misbehaving wallet and (MuSig2) multisig.
    
    Instead, give the mock its own descriptor wallet. The test provides a
    dedicated node for this wallet and keeps it offline, so the mock can't
    cheat by e.g. inspecting the UTXO set. The mock creates the wallet on
    first use and signs with walletprocesspsbt.
    
    wallet_signer.py now funds all four descriptor types and spends them
    in a single transaction, exercising every signing code path.
    3ec0242ffc
  5. external_signer: merge PSBT response instead of replacing
    Previously the PSBT returned by the external signer replaced the
    original wholesale, trusting the signer not to modify the transaction.
    Merge it instead. This rejects a response that describes a different
    transaction.
    
    Merging also supports signers that strip fields they don't need from
    their response. New tests cover both scenarios.
    
    Co-authored-by: brunoerg <brunoely.gc@gmail.com>
    f04a436787
  6. external_signer: reject unsafe sighash types
    A signature with SIGHASH_NONE or SIGHASH_SINGLE doesn't commit to all
    outputs, letting anyone alter them after signing. Reject a PSBT from
    an external signer that declares such a sighash type or contains
    signatures made with one.
    
    SIGHASH_ANYONECANPAY is still accepted: it only permits adding inputs,
    which does not affect us.
    
    The mock signer produces real signatures for these scenarios by
    letting its wallet sign with the requested sighash type, optionally
    hiding the declared sighash type field so that only the signatures
    themselves reveal it.
    
    Co-authored-by: brunoerg <brunoely.gc@gmail.com>
    79fb761ed8
  7. DrahtBot added the label Wallet on Aug 28, 2026
  8. DrahtBot commented at 11:27 AM on August 28, 2026: contributor

    <!--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/36114.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36113 (psbt: fix rendering for invalid long sighash type field by Sjors)
    • #36076 (psbt: preserve sighash type when merging inputs by thomasbuilds)
    • #35984 (sign: skip signing SIGHASH_SINGLE inputs with no corresponding output by furszy)
    • #35358 (external signer: verify PSBT is reliable after signing it by brunoerg)
    • #33112 (wallet: relax external_signer flag constraints 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-->

  9. Sjors commented at 11:30 AM on August 28, 2026: member

    cc @bigspider and other hardware wallet folks, any other hardware wallet shenanigans we should check against (perhaps in a followup)?

  10. dangervslash commented at 10:00 PM on August 28, 2026: none

    Approved flame

  11. dangervslash commented at 10:00 PM on August 28, 2026: none

    MarkUp


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-31 19:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me