Remove op== on PSBTs; check compatibility in Merge

Remove the op== on PartiallySignedTransaction, which only checks that the
CTransactions are equal. Instead, check this directly in Merge, and return
false if the CTransactions are not equal (so the PSBTs cannot be merged.)

3 files changed, 13 insertions, 14 deletions

diff --git a/src/psbt.cpp b/src/psbt.cpp
index 97fb39f1c8..06032d6953 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -16,8 +16,13 @@ bool PartiallySignedTransaction::IsNull() const
     return !tx && inputs.empty() && outputs.empty() && unknown.empty();
 }
 
-void PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
+bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
 {
+    // Prohibited to merge two PSBTs over different transactions
+    if (tx->GetHash() != psbt.tx->GetHash()) {
+        return false;
+    }
+
     for (unsigned int i = 0; i < inputs.size(); ++i) {
         inputs[i].Merge(psbt.inputs[i]);
     }
@@ -25,6 +30,8 @@ void PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
         outputs[i].Merge(psbt.outputs[i]);
     }
     unknown.insert(psbt.unknown.begin(), psbt.unknown.end());
+
+    return true;
 }
 
 bool PartiallySignedTransaction::IsSane() const
diff --git a/src/psbt.h b/src/psbt.h
index ad6f003015..4b7ea4383a 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -384,22 +384,15 @@ struct PartiallySignedTransaction
     std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown;
 
     bool IsNull() const;
-    void Merge(const PartiallySignedTransaction& psbt);
+
+    /** 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);
     bool IsSane() const;
     PartiallySignedTransaction() {}
     PartiallySignedTransaction(const PartiallySignedTransaction& psbt_in) : tx(psbt_in.tx), inputs(psbt_in.inputs), outputs(psbt_in.outputs), unknown(psbt_in.unknown) {}
     explicit PartiallySignedTransaction(const CMutableTransaction& tx);
 
-    // Only checks if they refer to the same transaction
-    friend bool operator==(const PartiallySignedTransaction& a, const PartiallySignedTransaction &b)
-    {
-        return a.tx->GetHash() == b.tx->GetHash();
-    }
-    friend bool operator!=(const PartiallySignedTransaction& a, const PartiallySignedTransaction &b)
-    {
-        return !(a == b);
-    }
-
     template <typename Stream>
     inline void Serialize(Stream& s) const {
 
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index bc836614ae..4205a3d10f 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1481,10 +1481,9 @@ UniValue combinepsbt(const JSONRPCRequest& request)
 
     // Merge
     for (auto it = std::next(psbtxs.begin()); it != psbtxs.end(); ++it) {
-        if (*it != merged_psbt) {
+        if (!merged_psbt.Merge(*it)) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "PSBTs do not refer to the same transactions.");
         }
-        merged_psbt.Merge(*it);
     }
     if (!merged_psbt.IsSane()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Merged PSBT is inconsistent");