From 7c632c0e2a2ef604cc5bca682935ee519cfbad8e Mon Sep 17 00:00:00 2001 From: Thomas Date: Fri, 12 Jun 2026 08:01:08 +0200 Subject: [PATCH] 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()); }