wallet: ensure COutput added in set are unique

before #25806, set<COutput> was used and would not
contain same COutputs in the set.

now we use set<shared_ptr<COutput>> and it might be
possible for 2 distinct shared_ptr (different pointer
address but same COutputs) to be added into the set.

so preserve previous behaviour by making sure values
in the set are also distinct

src/util/insert.h

@@ -19,6 +19,11 @@ inline void insert(std::set<TsetT>& dst, const Tsrc& src) {
     dst.insert(src.begin(), src.end());
 }
 
+template <typename TsetT, typename Compare, typename Tsrc>
+inline void insert(std::set<TsetT, Compare>& dst, const Tsrc& src) {
+    dst.insert(src.begin(), src.end());
+}
+
 } // namespace util
 
 #endif // BITCOIN_UTIL_INSERT_H

src/wallet/coinselection.cpp

@@ -908,7 +908,7 @@ void SelectionResult::AddInput(const OutputGroup& group)
     m_weight += group.m_weight;
 }
 
-void SelectionResult::AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs)
+void SelectionResult::AddInputs(const COutputSet& inputs, bool subtract_fee_outputs)
 {
     // As it can fail, combine inputs first
     InsertInputs(inputs);
@@ -933,7 +933,7 @@ void SelectionResult::Merge(const SelectionResult& other)
     m_weight += other.m_weight;
 }
 
-const std::set<std::shared_ptr<COutput>>& SelectionResult::GetInputSet() const
+const COutputSet& SelectionResult::GetInputSet() const
 {
     return m_selected_inputs;
 }

src/wallet/coinselection.h

@@ -319,11 +319,18 @@ enum class SelectionAlgorithm : uint8_t
 
 std::string GetAlgorithmName(const SelectionAlgorithm algo);
 
+struct COutputPtrComparator {
+    bool operator()(const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) const {
+        return *a < *b;
+    }
+};
+using COutputSet = std::set<std::shared_ptr<COutput>, COutputPtrComparator>;
+
 struct SelectionResult
 {
 private:
     /** Set of inputs selected by the algorithm to use in the transaction */
-    std::set<std::shared_ptr<COutput>> m_selected_inputs;
+    COutputSet m_selected_inputs;
     /** The target the algorithm selected for. Equal to the recipient amount plus non-input fees */
     CAmount m_target;
     /** The algorithm used to produce this result */
@@ -368,7 +375,7 @@ public:
     void Clear();
 
     void AddInput(const OutputGroup& group);
-    void AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs);
+    void AddInputs(const COutputSet& inputs, bool subtract_fee_outputs);
 
     /** How much individual inputs overestimated the bump fees for shared ancestries */
     void SetBumpFeeDiscount(const CAmount discount);
@@ -409,7 +416,7 @@ public:
     void Merge(const SelectionResult& other);
 
     /** Get m_selected_inputs */
-    const std::set<std::shared_ptr<COutput>>& GetInputSet() const;
+    const COutputSet& GetInputSet() const;
     /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */
     std::vector<std::shared_ptr<COutput>> GetShuffledInputVector() const;
 

src/wallet/spend.cpp

@@ -789,7 +789,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
     // If the chosen input set has unconfirmed inputs, check for synergies from overlapping ancestry
     for (auto& result : results) {
         std::vector<COutPoint> outpoints;
-        std::set<std::shared_ptr<COutput>> coins = result.GetInputSet();
+        COutputSet coins = result.GetInputSet();
         CAmount summed_bump_fees = 0;
         for (auto& coin : coins) {
             if (coin->depth > 0) continue; // Bump fees only exist for unconfirmed inputs

src/wallet/spend.h

@@ -155,7 +155,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co
 // User manually selected inputs that must be part of the transaction
 struct PreSelectedInputs
 {
-    std::set<std::shared_ptr<COutput>> coins;
+    COutputSet coins;
     // If subtract fee from outputs is disabled, the 'total_amount'
     // will be the sum of each output effective value
     // instead of the sum of the outputs amount

src/wallet/test/coinselector_tests.cpp

@@ -30,8 +30,6 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup)
 // we repeat those tests this many times and only complain if all iterations of the test fail
 #define RANDOM_REPEATS 5
 
-typedef std::set<std::shared_ptr<COutput>> CoinSet;
-
 static const CoinEligibilityFilter filter_standard(1, 6, 0);
 static const CoinEligibilityFilter filter_confirmed(1, 1, 0);
 static const CoinEligibilityFilter filter_standard_extra(6, 6, 0);
@@ -117,7 +115,7 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
 /** Check if this selection is equal to another one. Equal means same inputs (i.e same value and prevout) */
 static bool EqualResult(const SelectionResult& a, const SelectionResult& b)
 {
-    std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(),
+    std::pair<COutputSet::iterator, COutputSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(),
         [](const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) {
             return a->outpoint == b->outpoint;
         });
@@ -1257,7 +1255,7 @@ static util::Result<SelectionResult> select_coins(const CAmount& target, const C
     return result;
 }
 
-static bool has_coin(const CoinSet& set, CAmount amount)
+static bool has_coin(const COutputSet& set, CAmount amount)
 {
     return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; });
 }

src/wallet/test/fuzz/coinselection.cpp

@@ -68,7 +68,7 @@ static CAmount CreateCoins(FuzzedDataProvider& fuzzed_data_provider, std::vector
 static SelectionResult ManualSelection(std::vector<COutput>& utxos, const CAmount& total_amount, const bool& subtract_fee_outputs)
 {
     SelectionResult result(total_amount, SelectionAlgorithm::MANUAL);
-    std::set<std::shared_ptr<COutput>> utxo_pool;
+    COutputSet utxo_pool;
     for (const auto& utxo : utxos) {
         utxo_pool.insert(std::make_shared<COutput>(utxo));
     }
@@ -319,7 +319,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
     std::vector<COutput> utxos;
     CAmount new_total_balance{CreateCoins(fuzzed_data_provider, utxos, coin_params, next_locktime)};
     if (new_total_balance > 0) {
-        std::set<std::shared_ptr<COutput>> new_utxo_pool;
+        COutputSet new_utxo_pool;
         for (const auto& utxo : utxos) {
             new_utxo_pool.insert(std::make_shared<COutput>(utxo));
         }
@@ -336,7 +336,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
     auto manual_selection{ManualSelection(manual_inputs, manual_balance, coin_params.m_subtract_fee_outputs)};
     if (result) {
         const CAmount old_target{result->GetTarget()};
-        const std::set<std::shared_ptr<COutput>> input_set{result->GetInputSet()};
+        const COutputSet input_set{result->GetInputSet()};
         const int old_weight{result->GetWeight()};
         result->Merge(manual_selection);
         assert(result->GetInputSet().size() == input_set.size() + manual_inputs.size());