psbt: preserve sighash type when merging inputs #36076

thomasbuilds commented at 10:33 AM on August 25, 2026: contributor

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.

DrahtBot added the label PSBT on Aug 25, 2026

DrahtBot commented at 10:33 AM on August 25, 2026: contributor

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

Code Coverage & Benchmarks

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

Reviews

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

Type	Reviewers
ACK	winterrdog, vicjuma, rkrux, achow101
Stale ACK	jpk68, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36122 (BIP460: CISA for Taproot key path spends by fjahr)

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.

jpk68 commented at 2:33 PM on August 25, 2026: contributor

ACK 15803d84ebb6871d507dd93065f3572a7291271e

winterrdog commented at 11:02 AM on August 26, 2026: contributor

Concept ACK

sighash_type specifies which parts of the transaction a signature for the input commits to, so it makes sense to retain it when merging if one PSBT has it and the other does not

in src/psbt.cpp:467 in 15803d84eb outdated

 463 | @@ -464,6 +464,7 @@ bool PSBTInput::Merge(const PSBTInput& input)
 464 |      for (const auto& [agg_key_lh, psigs] : input.m_musig2_partial_sigs) {
 465 |          m_musig2_partial_sigs[agg_key_lh].insert(psigs.begin(), psigs.end());
 466 |      }
 467 | +    if (sighash_type == std::nullopt && input.sighash_type != std::nullopt) sighash_type = input.sighash_type;

vicjuma commented at 12:31 PM on August 26, 2026:

I think it correctly complements the sighash_type check in line 701 especially after the sighash_type change from int to std::optional<int> type in #31622

in test/functional/rpc_psbt.py:577 in 15803d84eb outdated

 572 | +        assert_equal(node.decodepsbt(signed)["inputs"][0].get("sighash"), "ALL|ANYONECANPAY")
 573 | +        updated = wallet.walletprocesspsbt(psbt=psbt, sign=False)["psbt"]
 574 | +        assert "sighash" not in node.decodepsbt(updated)["inputs"][0]
 575 | +
 576 | +        finalized = []
 577 | +        for psbts in [[signed, updated], [updated, signed]]:

vicjuma commented at 12:32 PM on August 26, 2026:

The above change works perfectly well independent of the order.

for psbts in [[updated, signed], [signed, updated]]:
    combined = node.combinepsbt(psbts)
    self.log.info(f"combined sighash: {node.decodepsbt(combined)['inputs'][0].get('sighash')}")

Before: 2026-08-26T12:24:19.694909Z TestFramework (INFO): combined sighash: None After: 2026-08-26T12:29:47.516225Z TestFramework (INFO): combined sighash: ALL|ANYONECANPAY

vicjuma commented at 12:34 PM on August 26, 2026: contributor

ACK 15803d84ebb6871d507dd93065f3572a7291271e

DrahtBot requested review from winterrdog on Aug 26, 2026

winterrdog commented at 7:18 PM on August 26, 2026: contributor

tACK 15803d84ebb6871d507dd93065f3572a7291271e

successfully built and tested on this toolchain: FreeBSD 15.0/clang++-19/x86_64

i commented out the new sighash_type preservation line and ran the functional test, which failed with None == ALL|ANYONECANPAY, confirming that the field is not retained when it is absent from the first PSBT but present in the second

<details> <summary>diff to comment the new preservation line </summary>

diff --git a/src/psbt.cpp b/src/psbt.cpp
index 18e2d0a7f8..d4d6c31418 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -464,7 +464,7 @@ bool PSBTInput::Merge(const PSBTInput& input)
     for (const auto& [agg_key_lh, psigs] : input.m_musig2_partial_sigs) {
         m_musig2_partial_sigs[agg_key_lh].insert(psigs.begin(), psigs.end());
     }
-    if (sighash_type == std::nullopt && input.sighash_type != std::nullopt) sighash_type = input.sighash_type;
+    // if (sighash_type == std::nullopt && input.sighash_type != std::nullopt) sighash_type = input.sighash_type;
     if (sequence == std::nullopt && input.sequence != std::nullopt) sequence = input.sequence;
     if (time_locktime == std::nullopt && input.time_locktime != std::nullopt) time_locktime = input.time_locktime;
     if (height_locktime == std::nullopt && input.height_locktime != std::nullopt) height_locktime = input.height_locktime;

</details>

<details> <summary>test output log i got after i applied the diff </summary>

Traceback (most recent call last):
  File "/home/winterrdog/Documents/btc/btc-core/my-btc-fork/test/functional/test_framework/test_framework.py", line 145, in main
    self.run_test()
  File "/home/winterrdog/Documents/btc/btc-core/my-btc-fork/build/test/functional/rpc_psbt.py", line 1650, in run_test
    self.test_combinepsbt_sighash_type()
  File "/home/winterrdog/Documents/btc/btc-core/my-btc-fork/build/test/functional/rpc_psbt.py", line 579, in test_combinepsbt_sighash_type
    assert_equal(node.decodepsbt(combined)["inputs"][0].get("sighash"), "ALL|ANYONECANPAY")
  File "/home/winterrdog/Documents/btc/btc-core/my-btc-fork/test/functional/test_framework/util.py", line 94, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(None == ALL|ANYONECANPAY)

</details>

after a git restore to remove the local change, all tests passed successfully!

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

Concept ACK

Sjors commented at 8:21 AM on September 4, 2026: member

ACK 15803d84ebb6871d507dd93065f3572a7291271e

It seems this was overlooked as early as the introduction of PSBT in #13557 (e9d86a43ad8b1ab83b324e9a7a64c43a61337501). That wasn't a problem because the field was ignored in favor of an RPC argument, until #31622.

I proposed a test vector in https://github.com/bitcoin/bips/pull/2275, and specified combine tests must pass in both orders (since existing test vectors don't conflict).

diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json
index 06c6f9e849..583c34e1a5 100644
--- a/test/functional/data/rpc_psbt.json
+++ b/test/functional/data/rpc_psbt.json
@@ -289,4 +289,11 @@
             ],
             "result" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgf0cwRAIgdAGK1BgAl7hzMjwAFXILNoTMgSOJEEjn282bVa1nnJkCIHPTabdA4+tT3O+jOCPIBwUUylWn3ZVE8VfBZ5EyYRGMASICAtq2H/SaFNtqfQKwzR+7ePxLGDErW05U2uTbovv+9TbXRzBEAiBjGpif5zipKtAZhgIzEsGSFP4oArOeXLwaw2eIBsaSwwIgOdtsOHvSZ3Ft/bPU2NpQuOhdITMmunx9qqTAzkHrkiMBAQMEAQAAAAEER1IhApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/IQLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU211KuIgYClYO/Oa4KYJdHrRma3dY0+mEIVZ1sXNObTCGD8auW4H8Q2QxqTwAAAIAAAACAAAAAgCIGAtq2H/SaFNtqfQKwzR+7ePxLGDErW05U2uTbovv+9TbXENkMak8AAACAAAAAgAEAAIAAAQEgAMLrCwAAAAAXqRS39fr0Dj1ApaRZsds1NfK3L6kh6IciAgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3EcwRAIgYut6VWEHp8c/RaxKtaHd329wdfsSdZaafzg+//eEvLICIAwF27dHDb8vCFV901bHMlwe0wkT6ZbNOECUXbEiKNpfASICAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zRzBEAiBl9FulmYtZon/+GnvtAWrx8fkNVLOqj3RQql9WolEDvQIgf3JHA60e25ZoCyhLVtT/y4j3+3Weq74IqjDym4UTg9IBAQMEAQAAAAEEIgAgjCNTFzdDtZXftKB7crqOQuN5fadOh/59nXSX47ICiQMBBUdSIQMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3CECOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNSriIGAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zENkMak8AAACAAAAAgAMAAIAiBgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3BDZDGpPAAAAgAAAAIACAACAACICA6mkw39ZltOqJdusa1cK8GUDlEkpQkYLNUdT7Z7spYdxENkMak8AAACAAAAAgAQAAIAAIgICf2OZdX0u/1WhNq0CxoSxg4tlVuXxtrNCgqlLa1AFEJYQ2QxqTwAAAIAAAACABQAAgAA="
+        },
+        {
+            "combine" : [
+                "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAAEBIADC6wsAAAAAF6kUt/X69A49QKWkWbHbNTXyty+pIeiHAQQiACCMI1MXN0O1ld+0oHtyuo5C43l9p06H/n2ddJfjsgKJAwEFR1IhAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcIQI63ZBPPW3PWd25BrDe4jUpt/+57VDl6GFRkmhgIh8Oc1KuIgYCOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnMQ2QxqTwAAAIAAAACAAwAAgCIGAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcENkMak8AAACAAAAAgAIAAIAAIgIDqaTDf1mW06ol26xrVwrwZQOUSSlCRgs1R1Ptnuylh3EQ2QxqTwAAAIAAAACABAAAgAAiAgJ/Y5l1fS7/VaE2rQLGhLGDi2VW5fG2s0KCqUtrUAUQlhDZDGpPAAAAgAAAAIAFAACAAA==",
+                "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgf0cwRAIgOL80NT9sZb1AQ+q36sL908FMx7YtrWwov6mYlSarN1oCIAl7zFcZ+ap+M1Smn2g5YxscWU+IyLsRnc2mjsMQFZzsgQEDBIEAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAAEBIADC6wsAAAAAF6kUt/X69A49QKWkWbHbNTXyty+pIeiHIgIDCJ3BDHrG21T5EymvYXMz2ziM6tDCMfcjN50bmQMLAtxHMEQCIAHLeP7QwBWvKVeF7d0Pz38hRdH1gBZIIOYctWj4/oQ/AiBbz5DTn8kiRHdCLpwvlqyo9WaRXO+k7NTlxSqZtYiSkYEBAwSBAAAAAQQiACCMI1MXN0O1ld+0oHtyuo5C43l9p06H/n2ddJfjsgKJAwEFR1IhAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcIQI63ZBPPW3PWd25BrDe4jUpt/+57VDl6GFRkmhgIh8Oc1KuIgYCOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnMQ2QxqTwAAAIAAAACAAwAAgCIGAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcENkMak8AAACAAAAAgAIAAIAAIgIDqaTDf1mW06ol26xrVwrwZQOUSSlCRgs1R1Ptnuylh3EQ2QxqTwAAAIAAAACABAAAgAAiAgJ/Y5l1fS7/VaE2rQLGhLGDi2VW5fG2s0KCqUtrUAUQlhDZDGpPAAAAgAAAAIAFAACAAA=="
+            ],
+            "result" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgf0cwRAIgOL80NT9sZb1AQ+q36sL908FMx7YtrWwov6mYlSarN1oCIAl7zFcZ+ap+M1Smn2g5YxscWU+IyLsRnc2mjsMQFZzsgQEDBIEAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAAEBIADC6wsAAAAAF6kUt/X69A49QKWkWbHbNTXyty+pIeiHIgIDCJ3BDHrG21T5EymvYXMz2ziM6tDCMfcjN50bmQMLAtxHMEQCIAHLeP7QwBWvKVeF7d0Pz38hRdH1gBZIIOYctWj4/oQ/AiBbz5DTn8kiRHdCLpwvlqyo9WaRXO+k7NTlxSqZtYiSkYEBAwSBAAAAAQQiACCMI1MXN0O1ld+0oHtyuo5C43l9p06H/n2ddJfjsgKJAwEFR1IhAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcIQI63ZBPPW3PWd25BrDe4jUpt/+57VDl6GFRkmhgIh8Oc1KuIgYCOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnMQ2QxqTwAAAIAAAACAAwAAgCIGAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcENkMak8AAACAAAAAgAIAAIAAIgIDqaTDf1mW06ol26xrVwrwZQOUSSlCRgs1R1Ptnuylh3EQ2QxqTwAAAIAAAACABAAAgAAiAgJ/Y5l1fS7/VaE2rQLGhLGDi2VW5fG2s0KCqUtrUAUQlhDZDGpPAAAAgAAAAIAFAACAAA=="
         }
     ],
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 6f17088a09..27c5624227 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -6,5 +6,5 @@
 """
 from decimal import Decimal
-from itertools import product
+from itertools import permutations, product
 from random import randbytes

@@ -1163,8 +1163,10 @@ class PSBTTest(BitcoinTestFramework):
             assert_equal(signed_tx, signer['result'])

-        # Combiner test
+        # Combiner test. None of the vectors contain conflicting fields, so
+        # the result must not depend on the order of the PSBTs (BIP 174).
         for combiner in combiners:
-            combined = self.nodes[2].combinepsbt(combiner['combine'])
-            assert_equal(combined, combiner['result'])
+            for psbts in permutations(combiner['combine']):
+                combined = self.nodes[2].combinepsbt(list(psbts))
+                assert_equal(combined, combiner['result'])

         # Empty combiner test

rkrux commented at 3:10 PM on September 7, 2026: contributor

Concept ACK 15803d84ebb6871d507dd93065f3572a7291271e

DrahtBot added the label Needs rebase on Sep 7, 2026

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.

ea785a31f7

thomasbuilds force-pushed on Sep 8, 2026

DrahtBot removed the label Needs rebase on Sep 8, 2026

winterrdog commented at 7:09 AM on September 8, 2026: contributor

Re-ACK ea785a31f783e67adc9b4f0c4a8e54acd2794904

DrahtBot requested review from Sjors on Sep 8, 2026

DrahtBot requested review from vicjuma on Sep 8, 2026

DrahtBot requested review from rkrux on Sep 8, 2026

vicjuma commented at 1:41 PM on September 8, 2026: contributor

ACK ea785a31f783e67adc9b4f0c4a8e54acd2794904

rkrux approved

rkrux commented at 1:50 PM on September 8, 2026: contributor

lgtm ACK ea785a31f783e67adc9b4f0c4a8e54acd2794904

Combining PSBTs should be independent of the order passed.

Edit: editing this comment to trigger Drahtbot to notice this a-c-k.

achow101 commented at 12:26 AM on September 9, 2026: member

ACK ea785a31f783e67adc9b4f0c4a8e54acd2794904

achow101 merged this on Sep 9, 2026

achow101 closed this on Sep 9, 2026