From 7c632c0e2a2ef604cc5bca682935ee519cfbad8e Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 12 Jun 2026 08:01:08 +0200 Subject: [PATCH 1/3] psbt: avoid duplicate global xpub keys when merging The global xpubs are stored in a map of key origin to set of xpubs, while the serialization writes one record per xpub, keyed by the xpub. Merging two PSBTs origin-by-origin can store the same xpub under two origins, which serializes as a PSBT with duplicate keys that the deserializer (and BIP 174) reject. Notably, combinepsbt returned a PSBT that no RPC could parse again when the combined PSBTs provide different key origins for the same xpub. Deduplicate by xpub when merging, keeping the origin that is already present, and share the logic between combinepsbt and joinpsbts. --- src/psbt.cpp | 20 +++++++++++++------- src/psbt.h | 3 +++ src/rpc/rawtransaction.cpp | 8 +------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index 8f2e9ab16f3..3d696da7fde 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -13,6 +13,8 @@ #include #include +#include + using common::PSBTError; PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx, uint32_t version) : m_version(version) @@ -58,13 +60,7 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt) return false; } } - for (auto& xpub_pair : psbt.m_xpubs) { - if (!m_xpubs.contains(xpub_pair.first)) { - m_xpubs[xpub_pair.first] = xpub_pair.second; - } else { - m_xpubs[xpub_pair.first].insert(xpub_pair.second.begin(), xpub_pair.second.end()); - } - } + MergeGlobalXPubs(psbt); if (fallback_locktime == std::nullopt && psbt.fallback_locktime != std::nullopt) fallback_locktime = psbt.fallback_locktime; // Set m_tx_modifiable only if either PSBT had it set @@ -85,6 +81,16 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt) return true; } +void PartiallySignedTransaction::MergeGlobalXPubs(const PartiallySignedTransaction& psbt) +{ + for (const auto& [origin, xpubs] : psbt.m_xpubs) { + for (const CExtPubKey& xpub : xpubs) { + const bool known{std::ranges::any_of(m_xpubs, [&](const auto& entry) { return entry.second.contains(xpub); })}; + if (!known) m_xpubs[origin].insert(xpub); + } + } +} + std::optional PartiallySignedTransaction::ComputeTimeLock() const { if (GetVersion() >= 2) { diff --git a/src/psbt.h b/src/psbt.h index b0177a3e555..9a4c600eadd 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1260,6 +1260,9 @@ public: /** Merge psbt into this. The two psbts must have the same underlying CTransaction (i.e. the * same actual Bitcoin transaction.) Returns true if the merge succeeded, false otherwise. */ [[nodiscard]] bool Merge(const PartiallySignedTransaction& psbt); + /** Merge the global xpubs of psbt into this, keeping the existing origin for an xpub + * seen again with a different one, as the serialized records are keyed by xpub. */ + void MergeGlobalXPubs(const PartiallySignedTransaction& psbt); bool AddInput(const PSBTInput& psbtin); bool AddOutput(const PSBTOutput& psbtout); std::optional ComputeTimeLock() const; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 31a877b8f04..b5d9160cae0 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1918,13 +1918,7 @@ static RPCMethod joinpsbts() for (const PSBTOutput& output : psbt.outputs) { merged_psbt.AddOutput(output); } - for (auto& xpub_pair : psbt.m_xpubs) { - if (!merged_psbt.m_xpubs.contains(xpub_pair.first)) { - merged_psbt.m_xpubs[xpub_pair.first] = xpub_pair.second; - } else { - merged_psbt.m_xpubs[xpub_pair.first].insert(xpub_pair.second.begin(), xpub_pair.second.end()); - } - } + merged_psbt.MergeGlobalXPubs(psbt); merged_psbt.unknown.insert(psbt.unknown.begin(), psbt.unknown.end()); } From 3b7051c7e3388727402eeffe15a0df86e3c0de3c Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 12 Jun 2026 08:01:08 +0200 Subject: [PATCH 2/3] test: check combinepsbt with conflicting global xpub origins --- test/functional/rpc_psbt.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 37218f32753..bedab687250 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -8,6 +8,7 @@ from decimal import Decimal from itertools import product from random import randbytes +from test_framework.address import base58_to_byte from test_framework.blocktools import ( MAX_STANDARD_TX_WEIGHT, ) @@ -28,6 +29,7 @@ from test_framework.psbt import ( PSBT_GLOBAL_PROPRIETARY, PSBT_GLOBAL_UNSIGNED_TX, PSBT_GLOBAL_VERSION, + PSBT_GLOBAL_XPUB, PSBT_IN_RIPEMD160, PSBT_IN_SHA256, PSBT_IN_SIGHASH_TYPE, @@ -357,6 +359,32 @@ class PSBTTest(BitcoinTestFramework): proprietary_entry(key=output_key_b, value=b"\xff", identifier=b"out", subtype=6), ]) + def test_combinepsbt_global_xpub_origin_conflict(self): + self.log.info("Test that combining PSBTs with conflicting origins for the same xpub 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"")] + + xpub = "tpubD6NzVbkrYhZ4XgiXtGrdW5XDAPFCL9h7we1vwNCpn8tGbBcgfVYjXyhWo4E1xkh56hjod1RhGjxbaTLV3X4FyWuejifB9jusQ46QzG87VKp" + xpub_data, xpub_version = base58_to_byte(xpub) + xpub_key = bytes([PSBT_GLOBAL_XPUB]) + bytes([xpub_version]) + xpub_data + + def psbt_with_origin(fingerprint): + return PSBT( + g=PSBTMap({ + PSBT_GLOBAL_UNSIGNED_TX: tx.serialize(), + xpub_key: fingerprint, + }), + i=[PSBTMap({})], + o=[PSBTMap({})], + ).to_base64() + + combined = self.nodes[0].combinepsbt([psbt_with_origin(b"\x00\x00\x00\x00"), psbt_with_origin(b"\x11\x11\x11\x11")]) + # The same xpub under both origins would serialize as duplicate keys, making the combined PSBT unparseable + decoded = self.nodes[0].decodepsbt(combined) + assert_equal(decoded["global_xpubs"], [{"xpub": xpub, "master_fingerprint": "00000000", "path": "m"}]) + def test_sighash_mismatch(self): self.log.info("Test sighash type mismatches") self.nodes[0].createwallet("sighash_mismatch") @@ -1337,6 +1365,7 @@ class PSBTTest(BitcoinTestFramework): self.test_decodepsbt_musig2_input_output_types() self.test_combinepsbt_preserves_proprietary_fields() + self.test_combinepsbt_global_xpub_origin_conflict() self.log.info("Test that combining PSBTs with different transactions fails") tx = CTransaction() From 6d387af562fc85eee003a269dbea10963e941985 Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 12 Jun 2026 08:03:42 +0200 Subject: [PATCH 3/3] psbt: remove write-only global xpub tracking set The explicit duplicate global xpub check was replaced by the generic duplicate key check in 1e2d146b47, leaving the set unused. --- src/psbt.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/psbt.h b/src/psbt.h index 9a4c600eadd..32af27ab232 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1360,9 +1360,6 @@ public: // Used for duplicate key detection std::set> key_lookup; - // Track the global xpubs we have already seen. Just for sanity checking - std::set global_xpubs; - // Read global data bool found_sep = false; std::optional tx; @@ -1465,7 +1462,6 @@ public: if (!xpub.pubkey.IsFullyValid()) { throw std::ios_base::failure("Invalid pubkey"); } - global_xpubs.insert(xpub); // Read in the keypath from stream KeyOriginInfo keypath; DeserializeHDKeypath(s, keypath);