From 1cb416397b7aa4addd714f9ab68ae0a861ff81c3 Mon Sep 17 00:00:00 2001 From: Shuvam Pandey Date: Thu, 20 Aug 2026 05:31:02 +0545 Subject: [PATCH] psbt: avoid duplicate taproot leaf script keys when merging m_tap_scripts maps a leaf script to a set of control blocks, but is serialized one record per control block, keyed by the control block. PSBTInput::Merge unions it by the map key, so two PSBTs that map the same control block to different leaf scripts merge into an input serializing the 0x15 key twice. Duplicate keys are invalid, so combinepsbt hands back a PSBT that can no longer be decoded. Present since #22558 (v24.0). Merge the records instead of the map entries, keeping the leaf script already there, as BIP 174 lets the Combiner pick arbitrarily when conflicts occur. The control blocks already present are collected once rather than searched for per incoming record, which would be quadratic in the size of the PSBTs. Control blocks under a leaf script that both PSBTs carry are now kept as well, where the map level union dropped them. The test covers conflicting and non-conflicting merges, including an incoming leaf script whose control blocks only partly conflict, so records that do not conflict are not dropped alongside those that do. --- src/psbt.cpp | 13 +++++++- test/functional/rpc_psbt.py | 64 ++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index 586fded2a29..c9f0311634e 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -14,6 +14,7 @@ #include #include +#include using common::PSBTError; @@ -436,7 +437,17 @@ bool PSBTInput::Merge(const PSBTInput& input) m_proprietary.insert(input.m_proprietary.begin(), input.m_proprietary.end()); unknown.insert(input.unknown.begin(), input.unknown.end()); m_tap_script_sigs.insert(input.m_tap_script_sigs.begin(), input.m_tap_script_sigs.end()); - m_tap_scripts.insert(input.m_tap_scripts.begin(), input.m_tap_scripts.end()); + // Merge by control block, the serialized key (BIP 371), to avoid duplicate keys. Keep the + // leaf script already present; BIP 174 lets the Combiner pick arbitrarily on conflict. + std::set> seen_control_blocks; + for (const auto& [_, control_blocks] : m_tap_scripts) { + seen_control_blocks.insert(control_blocks.begin(), control_blocks.end()); + } + for (const auto& [leaf, control_blocks] : input.m_tap_scripts) { + for (const auto& control_block : control_blocks) { + if (seen_control_blocks.insert(control_block).second) m_tap_scripts[leaf].insert(control_block); + } + } m_tap_bip32_paths.insert(input.m_tap_bip32_paths.begin(), input.m_tap_bip32_paths.end()); if (redeem_script.empty() && !input.redeem_script.empty()) redeem_script = input.redeem_script; diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index d8c60fe61a3..7d830a3cdc3 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -43,6 +43,7 @@ from test_framework.psbt import ( PSBT_IN_PROPRIETARY, PSBT_IN_TAP_BIP32_DERIVATION, PSBT_IN_TAP_INTERNAL_KEY, + PSBT_IN_TAP_LEAF_SCRIPT, PSBT_IN_WITNESS_UTXO, PSBT_IN_FINAL_SCRIPTWITNESS, PSBT_OUT_MUSIG2_PARTICIPANT_PUBKEYS, @@ -50,7 +51,7 @@ from test_framework.psbt import ( PSBT_OUT_TAP_TREE, PSBT_OUT_SCRIPT, ) -from test_framework.script import CScript, OP_TRUE, SIGHASH_ALL, SIGHASH_ANYONECANPAY, hash160 +from test_framework.script import CScript, LEAF_VERSION_TAPSCRIPT, OP_TRUE, SIGHASH_ALL, SIGHASH_ANYONECANPAY, hash160 from test_framework.script_util import MIN_STANDARD_TX_NONWITNESS_SIZE, output_key_to_p2tr_script from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -409,6 +410,66 @@ class PSBTTest(BitcoinTestFramework): decoded = self.nodes[0].decodepsbt(combined) assert_equal(decoded["global_xpubs"], [{"xpub": xpub, "master_fingerprint": "00000000", "path": "m"}]) + def test_combinepsbt_tap_leaf_script_conflict(self): + self.log.info("Test that combining PSBTs with conflicting leaf scripts for the same control block keeps a single record") + + tx = CTransaction() + tx.vin = [CTxIn(outpoint=COutPoint(hash=int('aa' * 32, 16), n=0), scriptSig=b"")] + tx.vout = [CTxOut(nValue=0, scriptPubKey=b"")] + + def psbt_with_leaf_scripts(*records): + return PSBT( + g=PSBTMap({PSBT_GLOBAL_UNSIGNED_TX: tx.serialize()}), + i=[PSBTMap({ + bytes([PSBT_IN_TAP_LEAF_SCRIPT]) + control_block: bytes(leaf_script) + bytes([LEAF_VERSION_TAPSCRIPT]) + for leaf_script, control_block in records + })], + o=[PSBTMap({})], + ).to_base64() + + def combined_tap_scripts(psbts): + return self.nodes[0].decodepsbt(self.nodes[0].combinepsbt(psbts))["inputs"][0]["taproot_scripts"] + + def tap_script(leaf_script, control_blocks): + return {"script": leaf_script.hex(), "leaf_ver": LEAF_VERSION_TAPSCRIPT, "control_blocks": [cb.hex() for cb in control_blocks]} + + control_block = bytes([LEAF_VERSION_TAPSCRIPT]) + bytes.fromhex(H_POINT) + control_block_with_path = control_block + bytes(32) + control_block_with_longer_path = control_block + bytes(64) + leaf_script_a = CScript([OP_TRUE]) + leaf_script_b = CScript([OP_TRUE, OP_TRUE]) + leaf_script_c = CScript([OP_TRUE, OP_TRUE, OP_TRUE]) + + psbt_a = psbt_with_leaf_scripts((leaf_script_a, control_block)) + psbt_b = psbt_with_leaf_scripts((leaf_script_b, control_block)) + psbt_c = psbt_with_leaf_scripts((leaf_script_c, control_block)) + + # The same control block under two leaf scripts would serialize as duplicate keys + assert_equal(combined_tap_scripts([psbt_a, psbt_b]), [tap_script(leaf_script_a, [control_block])]) + # Reversed, so the leaf script kept is decided by the argument order and not by its content + assert_equal(combined_tap_scripts([psbt_b, psbt_a]), [tap_script(leaf_script_b, [control_block])]) + # A third PSBT conflicting with what the first merge kept is dropped the same way + assert_equal(combined_tap_scripts([psbt_a, psbt_b, psbt_c]), [tap_script(leaf_script_a, [control_block])]) + # Combining a PSBT with itself leaves it untouched + assert_equal(self.nodes[0].combinepsbt([psbt_a, psbt_a]), psbt_a) + + # Records that do not conflict are all kept, whether or not they share a leaf script + psbt_same_leaf = psbt_with_leaf_scripts((leaf_script_a, control_block_with_path)) + assert_equal(combined_tap_scripts([psbt_a, psbt_same_leaf]), [tap_script(leaf_script_a, [control_block, control_block_with_path])]) + psbt_other_leaf = psbt_with_leaf_scripts((leaf_script_b, control_block_with_path)) + assert_equal(combined_tap_scripts([psbt_a, psbt_other_leaf]), [ + tap_script(leaf_script_a, [control_block]), + tap_script(leaf_script_b, [control_block_with_path]), + ]) + + # Only the conflicting control block of an incoming leaf script is dropped, not all of them + psbt_leaf_a_two_blocks = psbt_with_leaf_scripts((leaf_script_a, control_block), (leaf_script_a, control_block_with_path)) + psbt_leaf_b_two_blocks = psbt_with_leaf_scripts((leaf_script_b, control_block_with_path), (leaf_script_b, control_block_with_longer_path)) + assert_equal(combined_tap_scripts([psbt_leaf_a_two_blocks, psbt_leaf_b_two_blocks]), [ + tap_script(leaf_script_a, [control_block, control_block_with_path]), + tap_script(leaf_script_b, [control_block_with_longer_path]), + ]) + def test_sighash_mismatch(self): self.log.info("Test sighash type mismatches") self.nodes[0].createwallet("sighash_mismatch") @@ -1431,6 +1492,7 @@ class PSBTTest(BitcoinTestFramework): self.test_combinepsbt_preserves_proprietary_fields() self.test_combinepsbt_global_xpub_origin_conflict() + self.test_combinepsbt_tap_leaf_script_conflict() self.log.info("Test that combining PSBTs with different transactions fails") tx = CTransaction()