Merge bitcoin/bitcoin#36076: psbt: preserve sighash type when merging inputs

ea785a31f7 psbt: preserve sighash type when merging inputs (Thomas)

Pull request description:

  `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.

ACKs for top commit:
  achow101:
    ACK ea785a31f7
  winterrdog:
    Re-ACK ea785a31f7
  vicjuma:
    ACK ea785a31f7
  rkrux:
    lgtm ACK ea785a31f7

Tree-SHA512: 3487368509926c3dc0218dfab2e08273676504ad5ed4635e12e56c0484bda4cd94f4ba6f2df26ee4a902ce9a274546f727f8eca4a62eb0add3a700b2141eb272

src/psbt.cpp

@@ -460,6 +460,7 @@ void 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 (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;

test/functional/rpc_psbt.py

@@ -565,6 +565,34 @@ class PSBTTest(BitcoinTestFramework):
         psbt.i[0].map[PSBT_IN_SIGHASH_TYPE] = (0x101).to_bytes(4, "little")
         assert_equal(node.decodepsbt(psbt.to_base64())["inputs"][0]["sighash"], "")
 
+    def test_combinepsbt_sighash_type(self):
+        self.log.info("Test that combining PSBTs preserves the sighash type field regardless of order")
+        node = self.nodes[0]
+        node.createwallet("combine_sighash")
+        wallet = node.get_wallet_rpc("combine_sighash")
+        def_wallet = node.get_wallet_rpc(self.default_wallet_name)
+
+        def_wallet.send([{wallet.getnewaddress(address_type="bech32"): 1}])
+        self.generate(node, 1)
+        psbt = wallet.walletcreatefundedpsbt(wallet.listunspent(), [{def_wallet.getnewaddress(): 0.5}])["psbt"]
+
+        signed = wallet.walletprocesspsbt(psbt=psbt, sighashtype="ALL|ANYONECANPAY", finalize=False)["psbt"]
+        assert_equal(node.decodepsbt(signed)["inputs"][0].get("sighash"), "ALL|ANYONECANPAY")
+        updated = wallet.walletprocesspsbt(psbt=psbt, sign=False)["psbt"]
+        assert "sighash" not in node.decodepsbt(updated)["inputs"][0]
+
+        finalized = []
+        for psbts in [[signed, updated], [updated, signed]]:
+            combined = node.combinepsbt(psbts)
+            assert_equal(node.decodepsbt(combined)["inputs"][0].get("sighash"), "ALL|ANYONECANPAY")
+            fin_res = node.finalizepsbt(combined)
+            assert_equal(fin_res["complete"], True)
+            assert_equal(node.testmempoolaccept([fin_res["hex"]])[0]["allowed"], True)
+            finalized.append(fin_res["hex"])
+        assert_equal(finalized[0], finalized[1])
+
+        wallet.unloadwallet()
+
     def assert_change_type(self, psbtx, expected_type):
         """Assert that the given PSBT has a change output with the given type."""
 
@@ -1628,6 +1656,7 @@ class PSBTTest(BitcoinTestFramework):
             self.test_sighash_mismatch()
         self.test_sighash_adding()
         self.test_decodepsbt_long_sighash_type()
+        self.test_combinepsbt_sighash_type()
         self.test_psbt_named_parameter_handling()
         self.test_psbt_roundtrip()
         self.test_psbt_version()