From f4d79477ff0946b0bd340ade9251fa38e3b95dd7 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Wed, 7 Dec 2022 14:55:55 -0300
Subject: [PATCH] wallet: coin selection, add duplicated inputs checks

As no process should be able to trigger this error
using the regular transaction creation process, throw
a runtime_error if happens to tell users/devs to
report the bug if happens.
---
 src/wallet/coinselection.cpp | 12 ++++++------
 src/wallet/coinselection.h   | 13 +++++++++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index ba1d60fe44..229437a68a 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -446,7 +446,8 @@ void SelectionResult::Clear()
 
 void SelectionResult::AddInput(const OutputGroup& group)
 {
-    util::insert(m_selected_inputs, group.m_outputs);
+    // As it can fail, combine inputs first
+    InsertInputs(group.m_outputs);
     m_use_effective = !group.m_subtract_fee_outputs;
 
     m_weight += group.m_weight;
@@ -454,7 +455,8 @@ void SelectionResult::AddInput(const OutputGroup& group)
 
 void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs)
 {
-    util::insert(m_selected_inputs, inputs);
+    // As it can fail, combine inputs first
+    InsertInputs(inputs);
     m_use_effective = !subtract_fee_outputs;
 
     m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) {
@@ -464,16 +466,14 @@ void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_f
 
 void SelectionResult::Merge(const SelectionResult& other)
 {
-    // Obtain the expected selected inputs count after the merge (for now, duplicates are not allowed)
-    const size_t expected_count = m_selected_inputs.size() + other.m_selected_inputs.size();
+    // As it can fail, combine inputs first
+    InsertInputs(other.m_selected_inputs);
 
     m_target += other.m_target;
     m_use_effective |= other.m_use_effective;
     if (m_algo == SelectionAlgorithm::MANUAL) {
         m_algo = other.m_algo;
     }
-    util::insert(m_selected_inputs, other.m_selected_inputs);
-    assert(m_selected_inputs.size() == expected_count);
 
     m_weight += other.m_weight;
 }
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index b703e2325c..20910819e0 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -10,6 +10,8 @@
 #include <policy/feerate.h>
 #include <primitives/transaction.h>
 #include <random.h>
+#include <util/system.h>
+#include <util/check.h>
 
 #include <optional>
 
@@ -302,6 +304,17 @@ private:
     /** Total weight of the selected inputs */
     int m_weight{0};
 
+    template<typename T>
+    void InsertInputs(const T& inputs)
+    {
+        // Store sum of combined input sets to check that the results have no shared UTXOs
+        const size_t expected_count = m_selected_inputs.size() + inputs.size();
+        util::insert(m_selected_inputs, inputs);
+        if (m_selected_inputs.size() != expected_count) {
+            throw std::runtime_error(STR_INTERNAL_BUG("Shared UTXOs among selection results"));
+        }
+    }
+
 public:
     explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
         : m_target(target), m_algo(algo) {}