From 26d12faeb80a749a287642538d87f3dd2d11c936 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Sun, 30 Aug 2026 16:20:42 +0700
Subject: [PATCH 01/15] refactor: carry the coin type in CoinSelectionParams

CoinType is threaded through AttemptSelection and ChooseSelectionResult
as a defaulted trailing parameter. Upstream keeps rewriting exactly
these signatures (bitcoin#26661, bitcoin#27183), so every backport has
to re-resolve the Dash-only argument, and a resolution that drops the
trailing argument silently falls back to CoinType::ALL_COINS, losing
the fully-mixed selection restrictions.

Store the coin type in CoinSelectionParams instead, which is built once
per selection request and already flows through the whole pipeline. The
enum moves from coincontrol.h to coinselection.h so the field can be
declared there without pulling the much heavier coincontrol.h into the
selection module; coin control is a consumer of the selection layer, so
the include direction stays sound.
---
 src/wallet/coincontrol.h   | 14 +-------------
 src/wallet/coinselection.h | 16 ++++++++++++++++
 src/wallet/spend.cpp       |  7 +++++--
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index a07a4cf2182f..b5ada332d828 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -12,6 +12,7 @@
 #include <script/keyorigin.h>
 #include <script/signingprovider.h>
 #include <script/standard.h>
+#include <wallet/coinselection.h>
 
 #include <algorithm>
 #include <map>
@@ -19,19 +20,6 @@
 #include <set>
 
 namespace wallet {
-enum class CoinType : uint8_t
-{
-    ALL_COINS,
-    ONLY_FULLY_MIXED,
-    ONLY_READY_TO_MIX,
-    ONLY_NONDENOMINATED,
-    ONLY_MASTERNODE_COLLATERAL, // find masternode outputs including locked ones (use with caution)
-    ONLY_COINJOIN_COLLATERAL,
-    // Attributes
-    MIN_COIN_TYPE = ALL_COINS,
-    MAX_COIN_TYPE = ONLY_COINJOIN_COLLATERAL,
-};
-
 //! Default for -avoidpartialspends
 static constexpr bool DEFAULT_AVOIDPARTIALSPENDS = false;
 
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 2d97331942b4..5dfb048c7002 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -11,6 +11,7 @@
 #include <random.h>
 #include <util/result.h>
 
+#include <cstdint>
 #include <optional>
 
 namespace wallet {
@@ -19,6 +20,19 @@ static constexpr CAmount CHANGE_LOWER{50000};
 //! upper bound for randomly-chosen target change amount
 static constexpr CAmount CHANGE_UPPER{1000000};
 
+enum class CoinType : uint8_t
+{
+    ALL_COINS,
+    ONLY_FULLY_MIXED,
+    ONLY_READY_TO_MIX,
+    ONLY_NONDENOMINATED,
+    ONLY_MASTERNODE_COLLATERAL, // find masternode outputs including locked ones (use with caution)
+    ONLY_COINJOIN_COLLATERAL,
+    // Attributes
+    MIN_COIN_TYPE = ALL_COINS,
+    MAX_COIN_TYPE = ONLY_COINJOIN_COLLATERAL,
+};
+
 /** A UTXO under consideration for use in funding a new transaction. */
 struct COutput {
 private:
@@ -146,6 +160,8 @@ struct CoinSelectionParams {
      * associated with the same address. This helps reduce privacy leaks resulting from address
      * reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */
     bool m_avoid_partial_spends = false;
+    /** Dash: which class of coins may be selected (from CCoinControl::nCoinType). */
+    CoinType m_coin_type{CoinType::ALL_COINS};
 
     CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size,
                         CAmount min_change_target, CFeeRate effective_feerate,
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index b76873551a4f..420caa0e7948 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -484,13 +484,15 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
     // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.
     std::vector<OutputGroup> all_groups = GroupOutputs(wallet, available_coins, coin_selection_params, eligibility_filter, /*positive_only=*/false);
     if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue, coin_selection_params.m_min_change_target,
-                                            coin_selection_params.rng_fast, max_inputs_weight, nCoinType == CoinType::ONLY_FULLY_MIXED,
+                                            coin_selection_params.rng_fast, max_inputs_weight,
+                                            coin_selection_params.m_coin_type == CoinType::ONLY_FULLY_MIXED,
                                             wallet.m_default_max_tx_fee)}) {
         knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
         results.push_back(*knapsack_result);
     }
 
-    if (auto srd_result{SelectCoinsSRD(positive_groups, nTargetValue, coin_selection_params.rng_fast, max_inputs_weight, nCoinType == CoinType::ONLY_FULLY_MIXED)}) {
+    if (auto srd_result{SelectCoinsSRD(positive_groups, nTargetValue, coin_selection_params.rng_fast, max_inputs_weight,
+                                       coin_selection_params.m_coin_type == CoinType::ONLY_FULLY_MIXED)}) {
         srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
         results.push_back(*srd_result);
     }
@@ -827,6 +829,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
 
     CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy
     coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
+    coin_selection_params.m_coin_type = coin_control.nCoinType;
 
     // Set the long term feerate estimate to the wallet's consolidate feerate
     coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate;

From 4ae4ab01eeb89b3e372b8cc5b65360b00de7c4fb Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Sun, 30 Aug 2026 16:21:07 +0700
Subject: [PATCH 02/15] refactor: drop CoinType threading from
 AttemptSelection/ChooseSelectionResult

The coin type now travels in CoinSelectionParams, so the Dash-only
trailing parameter and its eleven call-site arguments are dead weight.
Removing them restores both signatures to their upstream Bitcoin Core
shape, which lets future coin-selection backports (bitcoin#26661,
bitcoin#27183) apply without Dash-specific conflicts and removes the
failure mode where a conflict resolution drops the trailing argument
and silently selects with CoinType::ALL_COINS.
---
 src/wallet/spend.cpp | 24 ++++++++++++------------
 src/wallet/spend.h   |  4 ++--
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 420caa0e7948..a6b4c23cfdf5 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -443,11 +443,11 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
 }
 
 std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
-                                                const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types, CoinType nCoinType)
+                                                const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types)
 {
     // Run coin selection on each OutputType and compute the Waste Metric
     std::vector<SelectionResult> results;
-    if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.legacy, coin_selection_params, nCoinType)}) {
+    if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.legacy, coin_selection_params)}) {
         results.push_back(*result);
     }
 
@@ -455,7 +455,7 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
     // over all available coins, else pick the best solution from the results
     if (results.size() == 0) {
         if (allow_mixed_output_types) {
-            if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.all(), coin_selection_params, nCoinType)}) {
+            if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.all(), coin_selection_params)}) {
                 return result;
             }
         }
@@ -466,7 +466,7 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
 };
 
 std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
-                                                     const CoinSelectionParams& coin_selection_params, CoinType nCoinType)
+                                                     const CoinSelectionParams& coin_selection_params)
 {
     // Vector of results. We will choose the best one based on waste.
     std::vector<SelectionResult> results;
@@ -639,27 +639,27 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
 
         // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
         // confirmations on outputs received from other wallets and only spend confirmed change.
-        if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false, nCoinType)}) return r1;
+        if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false)}) return r1;
         // Allow mixing only if no solution from any single output type can be found
-        if (auto r2{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) return r2;
+        if (auto r2{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r2;
 
         // Fall back to using zero confirmation change (but with as few ancestors in the mempool as
         // possible) if we cannot fund the transaction otherwise.
         if (wallet.m_spend_zero_conf_change) {
-            if (auto r3{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) return r3;
+            if (auto r3{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r3;
             if (auto r4{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::min(size_t{4}, max_ancestors/3), std::min(size_t{4}, max_descendants/3)),
-                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) {
+                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
                 return r4;
             }
             if (auto r5{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2),
-                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) {
+                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
                 return r5;
             }
             // If partial groups are allowed, relax the requirement of spending OutputGroups (groups
             // of UTXOs sent to the same address, which are obviously controlled by a single wallet)
             // in their entirety.
             if (auto r6{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */),
-                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) {
+                                         available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
                 return r6;
             }
             // Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs
@@ -667,7 +667,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
             if (coin_control.m_include_unsafe_inputs) {
                 if (auto r7{AttemptSelection(wallet, value_to_select,
                     CoinEligibilityFilter(0 /* conf_mine */, 0 /* conf_theirs */, max_ancestors-1, max_descendants-1, true /* include_partial_groups */),
-                    available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) {
+                    available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
                     return r7;
                 }
             }
@@ -677,7 +677,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
             if (!fRejectLongChains) {
                 if (auto r8{AttemptSelection(wallet, value_to_select,
                                       CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), true /* include_partial_groups */),
-                                      available_coins, coin_selection_params, /*allow_mixed_output_types=*/true, nCoinType)}) {
+                                      available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
                     return r8;
                 }
             }
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 41f2e828910b..0bd8bca600d2 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -104,7 +104,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
  *                                       If failed, a nullopt
  */
 std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
-                                                const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types, CoinType nCoinType = CoinType::ALL_COINS);
+                                                const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types);
 
 /**
  * Attempt to find a valid input set that meets the provided eligibility filter and target.
@@ -120,7 +120,7 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
  *                                       If failed, a nullopt
  */
 std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
-                                                     const CoinSelectionParams& coin_selection_params, CoinType nCoinType = CoinType::ALL_COINS);
+                                                     const CoinSelectionParams& coin_selection_params);
 
 /**
  * Select a set of coins such that nTargetValue is met and at least

From 22441a3b23161df1b8fd548ffafd56b4ebad75de Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Sun, 30 Aug 2026 18:38:11 +0700
Subject: [PATCH 03/15] refactor: fold SelectTxDSInsByDenomination overloads
 into one

The three-argument overload only forwards to the four-argument one with
CoinType::ONLY_READY_TO_MIX; a default argument expresses the same
without a second method. Also include coincontrol.h directly in
wallet/coinjoin.cpp, which uses CCoinControl but only received it
transitively.
---
 src/wallet/coinjoin.cpp | 6 +-----
 src/wallet/wallet.h     | 3 +--
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp
index d95c635a89bd..9d1dfad00548 100644
--- a/src/wallet/coinjoin.cpp
+++ b/src/wallet/coinjoin.cpp
@@ -6,6 +6,7 @@
 #include <wallet/coinjoin.h>
 
 #include <key_io.h>
+#include <wallet/coincontrol.h>
 #include <wallet/receive.h>
 #include <wallet/spend.h>
 #include <wallet/transaction.h>
@@ -45,11 +46,6 @@ bool CWallet::SetCoinJoinSalt(const uint256& cj_salt)
     return false;
 }
 
-bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector<CTxDSIn>& vecTxDSInRet)
-{
-    return SelectTxDSInsByDenomination(nDenom, nValueMax, vecTxDSInRet, CoinType::ONLY_READY_TO_MIX);
-}
-
 bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector<CTxDSIn>& vecTxDSInRet, CoinType nCoinType)
 {
     LOCK(cs_wallet);
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index d42a524849e1..5493a5c5a5e3 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -616,8 +616,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); }
 
     // Coin selection
-    bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector<CTxDSIn>& vecTxDSInRet);
-    bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector<CTxDSIn>& vecTxDSInRet, CoinType nCoinType);
+    bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector<CTxDSIn>& vecTxDSInRet, CoinType nCoinType = CoinType::ONLY_READY_TO_MIX);
     bool SelectDenominatedAmounts(CAmount nValueMax, std::set<CAmount>& setAmountsRet) const;
 
     std::vector<CompactTallyItem> SelectCoinsGroupedByAddresses(bool fSkipDenominated = true, bool fAnonymizable = true, bool fSkipUnconfirmed = true, int nMaxOupointsPerAddress = -1) const;

From ee1ba220cb256f6011532c2775ea7cdc225b7884 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Thu, 16 Feb 2023 13:12:20 -0500
Subject: [PATCH 04/15] Merge bitcoin/bitcoin#25344: New `outputs` argument for
 `bumpfee`/`psbtbumpfee`

4c8ecccdcd813fac3a7ef6a1493ef3977220421d test: add tests for `outputs` argument to `bumpfee`/`psbtbumpfee` (Seibart Nedor)
c0ebb9838252fb187db8719755801758d89651f7 wallet: add `outputs` arguments to `bumpfee` and `psbtbumpfee` (Seibart Nedor)
a804f3cfc0b4761b9ca7976e6e4472cd93599bbf wallet: extract and reuse RPC argument format definition for outputs (Seibart Nedor)

Pull request description:

  This implements a modification of the proposal in #22007: instead of **adding** outputs to the set of outputs in the original transaction, the outputs given by `outputs` argument **completely replace** the outputs in the original transaction.

  As noted below, this makes it easier to "cancel" a transaction or to reduce the amounts in the outputs, which is not the case with the original proposal in #22007, but it seems from the discussion in this PR that the **replace** behavior is more desirable than **add** one.

ACKs for top commit:
  achow101:
    ACK 4c8ecccdcd813fac3a7ef6a1493ef3977220421d
  1440000bytes:
    Code Review ACK https://github.com/bitcoin/bitcoin/pull/25344/commits/4c8ecccdcd813fac3a7ef6a1493ef3977220421d
  ishaanam:
    reACK 4c8ecccdcd813fac3a7ef6a1493ef3977220421d

Tree-SHA512: 31361f4a9b79c162bda7929583b0a3fd200e09f4c1a5378b12007576d6b14e02e9e4f0bab8aa209f08f75ac25a1f4805ad16ebff4a0334b07ad2378cc0090103

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/rpc/rawtransaction_util.cpp | 42 ++++++++++++++++++------------
 src/rpc/rawtransaction_util.h   |  7 +++++
 src/wallet/rpc/spend.cpp        | 46 ++++++++++++++++-----------------
 3 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp
index 8ab727004650..ab4d935d5808 100644
--- a/src/rpc/rawtransaction_util.cpp
+++ b/src/rpc/rawtransaction_util.cpp
@@ -21,12 +21,8 @@
 #include <txmempool.h>
 #include <util/translation.h>
 
-CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime)
+void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in)
 {
-    if (outputs_in.isNull()) {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null");
-    }
-
     UniValue inputs;
     if (inputs_in.isNull()) {
         inputs = UniValue::VARR;
@@ -34,17 +30,6 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
         inputs = inputs_in.get_array();
     }
 
-    const bool outputs_is_obj = outputs_in.isObject();
-    UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array();
-
-    CMutableTransaction rawTx;
-    if (!locktime.isNull()) {
-        int64_t nLockTime = locktime.getInt<int64_t>();
-        if (nLockTime < 0 || nLockTime > LOCKTIME_MAX)
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, locktime out of range");
-        rawTx.nLockTime = nLockTime;
-    }
-
     for (unsigned int idx = 0; idx < inputs.size(); idx++) {
         const UniValue& input = inputs[idx];
         const UniValue& o = input.get_obj();
@@ -79,6 +64,16 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
 
         rawTx.vin.push_back(in);
     }
+}
+
+void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in)
+{
+    if (outputs_in.isNull()) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument must be non-null");
+    }
+
+    const bool outputs_is_obj = outputs_in.isObject();
+    UniValue outputs = outputs_is_obj ? outputs_in.get_obj() : outputs_in.get_array();
 
     if (!outputs_is_obj) {
         // Translate array of key-value pairs into dict
@@ -127,6 +122,21 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal
             rawTx.vout.push_back(out);
         }
     }
+}
+
+CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime)
+{
+    CMutableTransaction rawTx;
+
+    if (!locktime.isNull()) {
+        int64_t nLockTime = locktime.getInt<int64_t>();
+        if (nLockTime < 0 || nLockTime > LOCKTIME_MAX)
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, locktime out of range");
+        rawTx.nLockTime = nLockTime;
+    }
+
+    AddInputs(rawTx, inputs_in);
+    AddOutputs(rawTx, outputs_in);
 
     return rawTx;
 }
diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h
index 0f550a355e40..35fd2e67fc75 100644
--- a/src/rpc/rawtransaction_util.h
+++ b/src/rpc/rawtransaction_util.h
@@ -37,6 +37,13 @@ void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const
   */
 void ParsePrevouts(const UniValue& prevTxsUnival, FillableSigningProvider* keystore, std::map<COutPoint, Coin>& coins);
 
+
+/** Normalize univalue-represented inputs and add them to the transaction */
+void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in);
+
+/** Normalize univalue-represented outputs and add them to the transaction */
+void AddOutputs(CMutableTransaction& rawTx, const UniValue& outputs_in);
+
 /** Create a transaction from univalue parameters */
 CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniValue& outputs_in, const UniValue& locktime);
 
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index a25b6138fe1f..36919011d971 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -934,6 +934,26 @@ RPCHelpMan signrawtransactionwithwallet()
     };
 }
 
+// Definition of allowed formats of specifying transaction outputs in
+// `send` and `walletcreatefundedpsbt` RPCs.
+static std::vector<RPCArg> OutputsDoc()
+{
+    return
+    {
+        {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",
+            {
+                {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the Dash address,\n"
+                         "the value (float or string) is the amount in " + CURRENCY_UNIT + ""},
+            },
+        },
+        {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
+            {
+                {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"},
+            },
+        },
+    };
+}
+
 RPCHelpMan send()
 {
     return RPCHelpMan{"send",
@@ -944,18 +964,7 @@ RPCHelpMan send()
                     "Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.\n"
                     "At least one output of either type must be specified.\n"
                     "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.",
-                {
-                    {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",
-                        {
-                            {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the Dash address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""},
-                        },
-                        },
-                    {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
-                        {
-                            {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"},
-                        },
-                    },
-                },
+                OutputsDoc(),
             },
             {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"},
             {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n"
@@ -1416,18 +1425,7 @@ RPCHelpMan walletcreatefundedpsbt()
                             "That is, each address can only appear once and there can only be one 'data' object.\n"
                             "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n"
                             "accepted as second parameter.",
-                        {
-                            {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "",
-                                {
-                                    {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the Dash address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""},
-                                },
-                                },
-                            {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
-                                {
-                                    {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"},
-                                },
-                            },
-                        },
+                        OutputsDoc(),
                     },
                     {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"},
                     {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",

From fde11c605d8973f2ab196ecef6b43f7c6139933d Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 4 Aug 2021 19:01:57 +0200
Subject: [PATCH 05/15] partial Merge bitcoin/bitcoin#22621: make
 ParseOutputType return a std::optional<OutputType>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

BACKPORT NOTE:
Still missing changes for m_default_address_type
-----
32fa49a18497a9b8c72e36a72ae96e7b23930223 make ParseOutputType return a std::optional<OutputType> (fanquake)

Pull request description:

  Similar to #22220. Skipped using `auto` here for the same reasons outlined in that PR.

ACKs for top commit:
  jnewbery:
    utACK 32fa49a18497a9b8c72e36a72ae96e7b23930223
  jonatack:
    Code review ACK 32fa49a18497a9b8c72e36a72ae96e7b23930223 and debian clang 13 debug build is clean / unit tests locally are green
  MarcoFalke:
    review ACK 32fa49a18497a9b8c72e36a72ae96e7b23930223 🍢

Tree-SHA512: 7752193117669b800889226185d49d164395697853828f8acb568f07651789bc5b2cddc45555957450353886e46b9a1e13c77a5e730a14c6ee621fabc8dc3d10

Co-authored-by: MarcoFalke <falke.marco@gmail.com>
---
 src/outputtype.cpp             | 8 ++++----
 src/outputtype.h               | 3 ++-
 src/test/fuzz/kitchen_sink.cpp | 1 +
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/outputtype.cpp b/src/outputtype.cpp
index 2a48c1e3c776..3facf82f0c50 100644
--- a/src/outputtype.cpp
+++ b/src/outputtype.cpp
@@ -13,6 +13,7 @@
 #include <util/vector.h>
 
 #include <assert.h>
+#include <optional>
 #include <string>
 
 static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy";
@@ -20,13 +21,12 @@ static const std::string OUTPUT_TYPE_STRING_UNKNOWN = "unknown";
 
 const std::array<OutputType, 1> OUTPUT_TYPES = {OutputType::LEGACY};
 
-bool ParseOutputType(const std::string& type, OutputType& output_type)
+std::optional<OutputType> ParseOutputType(const std::string& type)
 {
     if (type == OUTPUT_TYPE_STRING_LEGACY) {
-        output_type = OutputType::LEGACY;
-        return true;
+        return OutputType::LEGACY;
     }
-    return false;
+    return std::nullopt;
 }
 
 const std::string& FormatOutputType(OutputType type)
diff --git a/src/outputtype.h b/src/outputtype.h
index 5789f8a222c3..7fce9d655ae1 100644
--- a/src/outputtype.h
+++ b/src/outputtype.h
@@ -10,6 +10,7 @@
 #include <script/standard.h>
 
 #include <array>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -20,7 +21,7 @@ enum class OutputType {
 
 extern const std::array<OutputType, 1> OUTPUT_TYPES;
 
-[[nodiscard]] bool ParseOutputType(const std::string& str, OutputType& output_type);
+std::optional<OutputType> ParseOutputType(const std::string& str);
 const std::string& FormatOutputType(OutputType type);
 
 /**
diff --git a/src/test/fuzz/kitchen_sink.cpp b/src/test/fuzz/kitchen_sink.cpp
index 16bc0e81583b..dcc786629c77 100644
--- a/src/test/fuzz/kitchen_sink.cpp
+++ b/src/test/fuzz/kitchen_sink.cpp
@@ -13,6 +13,7 @@
 
 #include <array>
 #include <cstdint>
+#include <optional>
 #include <vector>
 
 namespace {

From c2154123530379430dbe70e264775dd00009b417 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Tue, 16 Aug 2022 20:00:15 -0400
Subject: [PATCH 06/15] partial Merge bitcoin/bitcoin#25734: wallet, refactor:
 #24584 follow-ups

BACKPORT NOTE:
missing changes are in the next files:
 - test/functional/wallet_avoid_mixing_output_types.py
 - src/wallet/scriptpubkeyman.cpp
 - src/wallet/wallet.cpp

OUTPUT_TYPES doesn't have UNKNOWN type because backport #25869 is already merged which removes it.

    Motivation:
    The `OUTPUT_TYPES` array contain the known active output types only.
    And it's solely used to create/walk-through the active spkms.

------

8cd21bb2799d37ed00dc9d0490bb5f5f1375932b refactor: improve readability for AttemptSelection (josibake)
f47ff717611182da27461e29b3c23933eb22fbce test: only run test for descriptor wallets (josibake)
0760ce0b9e646b6c86f4cc890c6ab78103a242ab test: add missing BOOST_ASSERT (josibake)
db09aec9378c5e8cc49c866fa50bfcb6c567d66c wallet: switch to new shuffle, erase, push_back (josibake)
b6b50b0f2b055d81c5d4ff9e21dd88cdc9a88ccb scripted-diff: Uppercase function names (josibake)
3f27a2adce12c6b0e7b43ba7c024331657bcf335 refactor: add new helper methods (josibake)
f5649db9d5e984ba7f376ccfd5b0a627f5c42402 refactor: add UNKNOWN OutputType (josibake)

Pull request description:

  This PR is to address follow-ups for #24584, specifically:

  * Remove redundant, hard-to-read code by adding a new `OutputType` and adding shuffle, erase, and push_back methods for `CoinsResult`
  * Add missing `BOOST_ASSERT` to unit test
  * Ensure functional test only runs if using descriptor wallets
  * Improve readability of `AttemptSelection` by removing triple-nested if statement

  Note for reviewers: commit `refactor: add new helper methods` should throw an "unused function warning"; the function is used in the next commit. Also, commit `wallet: switch to new shuffle, erase, push_back` will fail to compile, but this is fixed in the next commit with a scripted-diff. the commits are separate like this (code change then scripted-diff) to improve legibility.

ACKs for top commit:
  achow101:
    ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b
  aureleoules:
    ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b.
  LarryRuane:
    Concept, code review ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b
  furszy:
    utACK 8cd21bb2. Left a small, non-blocking, comment.

Tree-SHA512: a1bbc5962833e3df4f01a4895d8bd748cc4c608c3f296fd94e8afd8797b8d2e94e7bd44d598bd76fa5c9f5536864f396fcd097348fa0bb190a49a86b0917d60e

Co-authored-by: Andrew Chow <achow101-github@achow101.com>
---
 src/bench/coin_selection.cpp             |   2 +-
 src/coinjoin/client.cpp                  |   2 +-
 src/outputtype.cpp                       |   2 +
 src/rpc/masternode.cpp                   |   2 +-
 src/wallet/coinjoin.cpp                  |   8 +-
 src/wallet/interfaces.cpp                |   2 +-
 src/wallet/rpc/coins.cpp                 |   2 +-
 src/wallet/rpc/spend.cpp                 |   2 +-
 src/wallet/spend.cpp                     | 130 ++++++++++++++---------
 src/wallet/spend.h                       |  19 ++--
 src/wallet/test/availablecoins_tests.cpp |   6 +-
 src/wallet/test/coinjoin_tests.cpp       |   2 +-
 src/wallet/test/coinselector_tests.cpp   | 118 ++++++++++----------
 src/wallet/test/platformkeys_tests.cpp   |   4 +-
 src/wallet/test/wallet_tests.cpp         |   8 +-
 15 files changed, 172 insertions(+), 137 deletions(-)

diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index aa134894efb4..53e4e586ab8f 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -58,7 +58,7 @@ static void CoinSelection(benchmark::Bench& bench)
     wallet::CoinsResult available_coins;
     for (const auto& wtx : wtxs) {
         const auto txout = wtx->tx->vout.at(0);
-        available_coins.legacy.emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0);
+        available_coins.coins[OutputType::LEGACY].emplace_back(COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/true, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/ 0);
     }
     const CoinEligibilityFilter filter_standard(1, 6, 0);
     FastRandomContext rand{};
diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
index 7f777e2174eb..03fbed2379dc 100644
--- a/src/coinjoin/client.cpp
+++ b/src/coinjoin/client.cpp
@@ -2150,7 +2150,7 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
     AssertLockHeld(m_wallet->cs_wallet);
 
     CCoinControl coin_control(CoinType::ONLY_COINJOIN_COLLATERAL);
-    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*m_wallet, &coin_control).all()};
+    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*m_wallet, &coin_control).All()};
     if (vCoins.empty()) {
         strReason = strprintf("%s requires a collateral transaction and could not locate an acceptable input!", gCoinJoinName);
         return false;
diff --git a/src/outputtype.cpp b/src/outputtype.cpp
index 3facf82f0c50..298953845208 100644
--- a/src/outputtype.cpp
+++ b/src/outputtype.cpp
@@ -25,6 +25,8 @@ std::optional<OutputType> ParseOutputType(const std::string& type)
 {
     if (type == OUTPUT_TYPE_STRING_LEGACY) {
         return OutputType::LEGACY;
+    } else if (type == OUTPUT_TYPE_STRING_UNKNOWN) {
+        return OutputType::UNKNOWN;
     }
     return std::nullopt;
 }
diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp
index 71ed202a234b..37e928d21835 100644
--- a/src/rpc/masternode.cpp
+++ b/src/rpc/masternode.cpp
@@ -160,7 +160,7 @@ static RPCHelpMan masternode_outputs()
     CCoinControl coin_control(CoinType::ONLY_MASTERNODE_COLLATERAL);
 
     UniValue outputsArr(UniValue::VARR);
-    for (const auto& out : WITH_LOCK(wallet->cs_wallet, return AvailableCoinsListUnspent(*wallet, &coin_control).all())) {
+    for (const auto& out : WITH_LOCK(wallet->cs_wallet, return AvailableCoinsListUnspent(*wallet, &coin_control).All())) {
         outputsArr.push_back(out.outpoint.ToStringShort());
     }
 
diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp
index 9d1dfad00548..ae9a28f75a34 100644
--- a/src/wallet/coinjoin.cpp
+++ b/src/wallet/coinjoin.cpp
@@ -59,7 +59,7 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve
     CAmount nValueTotal{0};
     CCoinControl coin_control(nCoinType);
     std::set<uint256> setRecentTxIds;
-    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*this, &coin_control).all()};
+    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*this, &coin_control).All()};
 
     WalletCJLogPrint(this, "CWallet::%s -- vCoins.size(): %d, CoinType: %d\n", __func__, vCoins.size(), static_cast<int>(nCoinType));
 
@@ -105,7 +105,7 @@ bool CWallet::SelectDenominatedAmounts(CAmount nValueMax, std::set<CAmount>& set
     CAmount nValueTotal{0};
     CCoinControl coin_control(CoinType::ONLY_READY_TO_MIX);
     // CompareByPriority() cares about effective value, which is only calculable when supplied a feerate
-    std::vector<COutput> vCoins{AvailableCoins(*this, &coin_control, /*feerate=*/CFeeRate(0)).all()};
+    std::vector<COutput> vCoins{AvailableCoins(*this, &coin_control, /*feerate=*/CFeeRate(0)).All()};
 
     // larger denoms first
     std::sort(vCoins.rbegin(), vCoins.rend(), CompareByPriority());
@@ -236,7 +236,7 @@ bool CWallet::HasCollateralInputs(bool fOnlyConfirmed) const
     CCoinControl coin_control(CoinType::ONLY_COINJOIN_COLLATERAL);
     coin_control.m_include_unsafe_inputs = !fOnlyConfirmed;
 
-    return AvailableCoinsListUnspent(*this, &coin_control).size() > 0;
+    return AvailableCoinsListUnspent(*this, &coin_control).Size() > 0;
 }
 
 int CWallet::CountInputsWithAmount(CAmount nInputAmount) const
@@ -470,7 +470,7 @@ std::vector<COutPoint> CWallet::SelectFullyMixedForPromotion(int nDenom, int nCo
     // never be promoted together - they would be identifiable as one group in the final tx.
     CCoinControl coin_control(CoinType::ONLY_FULLY_MIXED);
     std::set<uint256> setRecentTxIds;
-    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*this, &coin_control).all()};
+    std::vector<COutput> vCoins{AvailableCoinsListUnspent(*this, &coin_control).All()};
 
     WalletCJLogPrint(this, "CWallet::%s -- vCoins.size(): %d\n", __func__, vCoins.size());
 
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 7e8cd5507331..60f10a77fe7f 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -469,7 +469,7 @@ class WalletImpl : public Wallet
         CCoinControl coin_control;
         coin_control.destChange = fund_destination;
         coin_control.fRequireAllInputs = false;
-        for (const auto& output : AvailableCoinsListUnspent(*m_wallet).all()) {
+        for (const auto& output : AvailableCoinsListUnspent(*m_wallet).All()) {
             CTxDestination destination;
             if (ExtractDestination(output.txout.scriptPubKey, destination) && destination == fund_destination) {
                 coin_control.Select(output.outpoint);
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 5a060111fb3a..6b8a355afb1a 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -703,7 +703,7 @@ RPCHelpMan listunspent()
         coinControl.m_include_unsafe_inputs = include_unsafe;
 
         LOCK(pwallet->cs_wallet);
-        vecOutputs = AvailableCoinsListUnspent(*pwallet, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).all();
+        vecOutputs = AvailableCoinsListUnspent(*pwallet, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).All();
     }
 
     LOCK(pwallet->cs_wallet);
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 36919011d971..f7e9a57f3af4 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1226,7 +1226,7 @@ RPCHelpMan sendall()
                     total_input_value += tx->tx->vout[input.prevout.n].nValue;
                 }
             } else {
-                for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).all()) {
+                for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).All()) {
                     CHECK_NONFATAL(output.input_bytes > 0);
                     if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) {
                         continue;
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index a6b4c23cfdf5..54322308cabc 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -81,24 +81,62 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall
     return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control);
 }
 
-uint64_t CoinsResult::size() const
+size_t CoinsResult::Size() const
 {
-    return legacy.size() + other.size();
+    size_t size{0};
+    for (const auto& it : coins) {
+        size += it.second.size();
+    }
+    return size;
 }
 
-std::vector<COutput> CoinsResult::all() const
+std::vector<COutput> CoinsResult::All() const
 {
     std::vector<COutput> all;
-    all.reserve(this->size());
-    all.insert(all.end(), legacy.begin(), legacy.end());
-    all.insert(all.end(), other.begin(), other.end());
+    all.reserve(coins.size());
+    for (const auto& it : coins) {
+        all.insert(all.end(), it.second.begin(), it.second.end());
+    }
     return all;
 }
 
-void CoinsResult::clear()
+void CoinsResult::Clear() {
+    coins.clear();
+}
+
+void CoinsResult::Erase(std::set<COutPoint>& preset_coins)
+{
+    for (auto& it : coins) {
+        auto& vec = it.second;
+        auto i = std::find_if(vec.begin(), vec.end(), [&](const COutput &c) { return preset_coins.count(c.outpoint);});
+        if (i != vec.end()) {
+            vec.erase(i);
+            break;
+        }
+    }
+}
+
+void CoinsResult::Shuffle(FastRandomContext& rng_fast)
+{
+    for (auto& it : coins) {
+        ::Shuffle(it.second.begin(), it.second.end(), rng_fast);
+    }
+}
+
+void CoinsResult::Add(OutputType type, const COutput& out)
 {
-    legacy.clear();
-    other.clear();
+    coins[type].emplace_back(out);
+}
+
+static OutputType GetOutputType(TxoutType type, bool is_from_p2sh)
+{
+    switch (type) {
+        case TxoutType::SCRIPTHASH:
+        case TxoutType::PUBKEYHASH:
+            return OutputType::LEGACY;
+        default:
+            return OutputType::UNKNOWN;
+    }
 }
 
 CoinsResult AvailableCoins(const CWallet& wallet,
@@ -219,37 +257,32 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             // Filter by spendable outputs only
             if (!spendable && only_spendable) continue;
 
-            // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script)
-            // We don't need those here, so we are leaving them in return_values_unused
-            std::vector<std::vector<uint8_t>> return_values_unused;
-            TxoutType type;
-
             // If the Output is P2SH and spendable, we want to know if it is
             // a P2SH (legacy). We can determine this from the redeemScript.
             // If the Output is not spendable, it will be classified as a P2SH (legacy),
             // since we have no way of knowing otherwise without the redeemScript
+            CScript script;
+            bool is_from_p2sh{false};
             if (output.scriptPubKey.IsPayToScriptHash() && solvable) {
-                CScript redeemScript;
                 CTxDestination destination;
                 if (!ExtractDestination(output.scriptPubKey, destination))
                     continue;
                 const CScriptID& hash = CScriptID(std::get<ScriptHash>(destination));
-                if (!provider->GetCScript(hash, redeemScript))
+                if (!provider->GetCScript(hash, script))
                     continue;
-                type = Solver(redeemScript, return_values_unused);
+                is_from_p2sh = true;
             } else {
-                type = Solver(output.scriptPubKey, return_values_unused);
+                script = output.scriptPubKey;
             }
 
             COutput coin(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate);
-            switch (type) {
-            case TxoutType::SCRIPTHASH:
-            case TxoutType::PUBKEYHASH:
-                result.legacy.push_back(coin);
-                break;
-            default:
-                result.other.push_back(coin);
-            };
+
+            // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script)
+            // We don't need those here, so we are leaving them in return_values_unused
+            std::vector<std::vector<uint8_t>> return_values_unused;
+            TxoutType type;
+            type = Solver(script, return_values_unused);
+            result.Add(GetOutputType(type, is_from_p2sh), coin);
 
             // Cache total amount as we go
             result.total_amount += output.nValue;
@@ -261,7 +294,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             }
 
             // Checks the maximum number of UTXO's.
-            if (nMaximumCount > 0 && result.size() >= nMaximumCount) {
+            if (nMaximumCount > 0 && result.Size() >= nMaximumCount) {
                 return result;
             }
         }
@@ -317,7 +350,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
 
     std::map<CTxDestination, std::vector<COutput>> result;
 
-    for (COutput& coin : AvailableCoinsListUnspent(wallet).all()) {
+    for (COutput& coin : AvailableCoinsListUnspent(wallet).All()) {
         CTxDestination address;
         if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) &&
             ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
@@ -447,22 +480,25 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
 {
     // Run coin selection on each OutputType and compute the Waste Metric
     std::vector<SelectionResult> results;
-    if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.legacy, coin_selection_params)}) {
-        results.push_back(*result);
+    for (const auto& it : available_coins.coins) {
+        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) {
+            results.push_back(*result);
+        }
     }
-
-    // If we can't fund the transaction from any individual OutputType, run coin selection
-    // over all available coins, else pick the best solution from the results
-    if (results.size() == 0) {
-        if (allow_mixed_output_types) {
-            if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.all(), coin_selection_params)}) {
-                return result;
-            }
+    // If we have at least one solution for funding the transaction without mixing, choose the minimum one according to waste metric
+    // and return the result
+    if (results.size() > 0) return *std::min_element(results.begin(), results.end());
+
+    // If we can't fund the transaction from any individual OutputType, run coin selection one last time
+    // over all available coins, which would allow mixing
+    if (allow_mixed_output_types) {
+        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) {
+            return result;
         }
-        return std::optional<SelectionResult>();
-    };
-    std::optional<SelectionResult> result{*std::min_element(results.begin(), results.end())};
-    return result;
+    }
+    // Either mixing is not allowed and we couldn't find a solution from any single OutputType, or mixing was allowed and we still couldn't
+    // find a solution using all available coins
+    return std::nullopt;
 };
 
 std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
@@ -576,7 +612,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
             // Calculate the smallest set of inputs required to meet nTargetValue from available_coins
             bool success{false};
             OutputGroup preset_candidates(coin_selection_params);
-            for (const COutput& out : available_coins.all()) {
+            for (const COutput& out : available_coins.All()) {
                 if (!out.spendable) continue;
                 if (preset_coins.count(out.outpoint)) {
                     preset_candidates.Insert(out, /*ancestors=*/0, /*descendants=*/0, /*positive_only=*/false);
@@ -606,8 +642,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
 
     // remove preset inputs from coins so that Coin Selection doesn't pick them.
     if (coin_control.HasSelected()) {
-        available_coins.legacy.erase(remove_if(available_coins.legacy.begin(), available_coins.legacy.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.legacy.end());
-        available_coins.other.erase(remove_if(available_coins.other.begin(), available_coins.other.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.other.end());
+        available_coins.Erase(preset_coins);
     }
 
     unsigned int limit_ancestor_count = 0;
@@ -619,12 +654,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
 
     // form groups from remaining coins; note that preset coins will not
     // automatically have their associated (same address) coins included
-    if (coin_control.m_avoid_partial_spends && available_coins.size() > OUTPUT_GROUP_MAX_ENTRIES) {
+    if (coin_control.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) {
         // Cases where we have 101+ outputs all pointing to the same destination may result in
         // privacy leaks as they will potentially be deterministically sorted. We solve that by
         // explicitly shuffling the outputs before processing
-        Shuffle(available_coins.legacy.begin(), available_coins.legacy.end(), coin_selection_params.rng_fast);
-        Shuffle(available_coins.other.begin(), available_coins.other.end(), coin_selection_params.rng_fast);
+        available_coins.Shuffle(coin_selection_params.rng_fast);
     }
 
     SelectionResult preselected(preset_inputs.GetSelectionAmount(), SelectionAlgorithm::MANUAL);
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 0bd8bca600d2..1abba9038482 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -33,23 +33,22 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall
  * This struct is really just a wrapper around OutputType vectors with a convenient
  * method for concatenating and returning all COutputs as one vector.
  *
- * clear(), size() methods are implemented so that one can interact with
- * the CoinsResult struct as if it was a vector
+ * Size(), Clear(), Erase(), Shuffle(), and Add() methods are implemented to
+ * allow easy interaction with the struct.
  */
 struct CoinsResult {
-    /** Vectors for each OutputType */
-    std::vector<COutput> legacy;
-
-    /** Other is a catch-all for anything that doesn't match the known OutputTypes */
-    std::vector<COutput> other;
+    std::map<OutputType, std::vector<COutput>> coins;
 
     /** Concatenate and return all COutputs as one vector */
-    std::vector<COutput> all() const;
+    std::vector<COutput> All() const;
 
     /** The following methods are provided so that CoinsResult can mimic a vector,
      * i.e., methods can work with individual OutputType vectors or on the entire object */
-    uint64_t size() const;
-    void clear();
+    size_t Size() const;
+    void Clear();
+    void Erase(std::set<COutPoint>& preset_coins);
+    void Shuffle(FastRandomContext& rng_fast);
+    void Add(OutputType type, const COutput& out);
 
     /** Sum of all available coins */
     CAmount total_amount{0};
diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp
index 50fde03159e5..3273cd2bcfcf 100644
--- a/src/wallet/test/availablecoins_tests.cpp
+++ b/src/wallet/test/availablecoins_tests.cpp
@@ -74,8 +74,8 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
         LOCK(wallet->cs_wallet);
         available_coins = AvailableCoins(*wallet);
     }
-    BOOST_CHECK_EQUAL(available_coins.size(), 1U);
-    BOOST_CHECK_EQUAL(available_coins.other.size(), 1U);
+    BOOST_CHECK_EQUAL(available_coins.Size(), 1U);
+    BOOST_CHECK_EQUAL(available_coins.coins[OutputType::UNKNOWN].size(), 1U);
 
     // We will create a self transfer for each of the OutputTypes and
     // verify it is put in the correct bucket after running GetAvailablecoins
@@ -95,7 +95,7 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
         LOCK(wallet->cs_wallet);
         available_coins = AvailableCoins(*wallet);
     }
-    BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U);
+    BOOST_CHECK_EQUAL(available_coins.coins[OutputType::LEGACY].size(), 2U);
 }
 
 BOOST_FIXTURE_TEST_CASE(UnconfirmableOutputsAreNotWalletFunds, AvailableCoinsTestingSetup)
diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp
index 4fcec9982b0b..64058b621d0a 100644
--- a/src/wallet/test/coinjoin_tests.cpp
+++ b/src/wallet/test/coinjoin_tests.cpp
@@ -264,7 +264,7 @@ BOOST_FIXTURE_TEST_CASE(coinjoin_pending_observation_tests, CTransactionBuilderT
             LOCK(wallet->cs_wallet);
             bool fFoundPending{false};
             bool fFoundFree{false};
-            for (const auto& out : AvailableCoinsListUnspent(*wallet).all()) {
+            for (const auto& out : AvailableCoinsListUnspent(*wallet).All()) {
                 fFoundPending |= out.outpoint == outpointPending;
                 fFoundFree |= out.outpoint == outpointTimeout;
             }
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index ead064077de6..75b42019c251 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -85,7 +85,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun
     assert(ret.second);
     CWalletTx& wtx = (*ret.first).second;
     const auto& txout = wtx.tx->vout.at(nInput);
-    available_coins.legacy.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate);
+    available_coins.coins[OutputType::LEGACY].emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, custom_size == 0 ? CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr) : custom_size, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate);
 }
 
 inline std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
@@ -323,14 +323,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         CoinsResult available_coins;
 
         add_coin(available_coins, *wallet, 1, coin_selection_params_bnb.m_effective_feerate);
-        available_coins.all().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
-        BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.all()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change));
+        available_coins.All().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
+        BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change));
 
         // Test fees subtracted from output:
-        available_coins.clear();
+        available_coins.Clear();
         add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate);
-        available_coins.all().at(0).input_bytes = 40;
-        const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.all()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
+        available_coins.All().at(0).input_bytes = 40;
+        const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
         BOOST_CHECK(result9);
         BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT);
     }
@@ -349,7 +349,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(available_coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
         CCoinControl coin_control;
         coin_control.m_allow_other_inputs = true;
-        coin_control.Select(available_coins.all().at(0).outpoint);
+        coin_control.Select(available_coins.All().at(0).outpoint);
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(0);
         const auto result10 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(result10);
@@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         CCoinControl coin_control;
         const auto result11 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result11));
-        available_coins.clear();
+        available_coins.Clear();
 
         // more coins should be selected when effective fee < long term fee
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000);
@@ -392,7 +392,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         const auto result12 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
         // NOTE: Dash does not use BnB and therefore, this check will fail
         // BOOST_CHECK(EquivalentResult(expected_result, *result12));
-        available_coins.clear();
+        available_coins.Clear();
 
         // pre selected coin should be selected even if disadvantageous
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
@@ -406,7 +406,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(9 * CENT, 2, expected_result);
         add_coin(1 * CENT, 2, expected_result);
         coin_control.m_allow_other_inputs = true;
-        coin_control.Select(available_coins.all().at(1).outpoint); // pre select 9 coin
+        coin_control.Select(available_coins.All().at(1).outpoint); // pre select 9 coin
         const auto result13 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result13));
     }
@@ -428,28 +428,28 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
     // test multiple times to allow for differences in the shuffle order
     for (int i = 0; i < RUN_TESTS; i++)
     {
-        available_coins.clear();
+        available_coins.Clear();
 
         // with an empty wallet we can't even pay one cent
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT));
 
         add_coin(available_coins, *wallet, 1*CENT, CFeeRate(0), 4);        // add a new 1 cent coin
 
         // with a new 1 cent coin, we still can't find a mature 1 cent
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT));
 
         // but we can find a new 1 cent
-        const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT);
+        const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT);
         BOOST_CHECK(result1);
         BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT);
 
         add_coin(available_coins, *wallet, 2*CENT);           // add a mature 2 cent coin
 
         // we can't make 3 cents of mature coins
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 3 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 3 * CENT, CENT));
 
         // we can make 3 cents of new coins
-        const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 3 * CENT, CENT);
+        const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 3 * CENT, CENT);
         BOOST_CHECK(result2);
         BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT);
 
@@ -460,44 +460,44 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27.  total = 38
 
         // we can't make 38 cents only if we disallow new coins:
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 38 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 38 * CENT, CENT));
         // we can't even make 37 cents if we don't allow new coins even if they're from us
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard_extra), 38 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard_extra), 38 * CENT, CENT));
         // but we can make 37 cents if we accept new coins from ourself
-        const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 37 * CENT, CENT);
+        const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 37 * CENT, CENT);
         BOOST_CHECK(result3);
         BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 37 * CENT);
         // and we can make 38 cents if we accept all new coins
-        const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 38 * CENT, CENT);
+        const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 38 * CENT, CENT);
         BOOST_CHECK(result4);
         BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 38 * CENT);
 
         // try making 34 cents from 1,2,5,10,20 - we can't do it exactly
-        const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 34 * CENT, CENT);
+        const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 34 * CENT, CENT);
         BOOST_CHECK(result5);
         BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 35 * CENT);       // but 35 cents is closest
         BOOST_CHECK_EQUAL(result5->GetInputSet().size(), 3U);     // the best should be 20+10+5.  it's incredibly unlikely the 1 or 2 got included (but possible)
 
         // when we try making 7 cents, the smaller coins (1,2,5) are enough.  We should see just 2+5
-        const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 7 * CENT, CENT);
+        const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 7 * CENT, CENT);
         BOOST_CHECK(result6);
         BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 7 * CENT);
         BOOST_CHECK_EQUAL(result6->GetInputSet().size(), 2U);
 
         // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough.
-        const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 8 * CENT, CENT);
+        const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 8 * CENT, CENT);
         BOOST_CHECK(result7);
         BOOST_CHECK(result7->GetSelectedValue() == 8 * CENT);
         BOOST_CHECK_EQUAL(result7->GetInputSet().size(), 3U);
 
         // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10)
-        const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 9 * CENT, CENT);
+        const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 9 * CENT, CENT);
         BOOST_CHECK(result8);
         BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT);
         BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U);
 
         // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin
-        available_coins.clear();
+        available_coins.Clear();
 
         add_coin(available_coins, *wallet,  6*CENT);
         add_coin(available_coins, *wallet,  7*CENT);
@@ -506,12 +506,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total
 
         // check that we have 71 and not 72
-        const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 71 * CENT, CENT);
+        const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 71 * CENT, CENT);
         BOOST_CHECK(result9);
-        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 72 * CENT, CENT));
+        BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 72 * CENT, CENT));
 
         // now try making 16 cents.  the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20
-        const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT);
+        const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT);
         BOOST_CHECK(result10);
         BOOST_CHECK_EQUAL(result10->GetSelectedValue(), 20 * CENT); // we should get 20 in one coin
         BOOST_CHECK_EQUAL(result10->GetInputSet().size(), 1U);
@@ -519,7 +519,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet,  5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total
 
         // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20
-        const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT);
+        const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT);
         BOOST_CHECK(result11);
         BOOST_CHECK_EQUAL(result11->GetSelectedValue(), 18 * CENT); // we should get 18 in 3 coins
         BOOST_CHECK_EQUAL(result11->GetInputSet().size(), 3U);
@@ -527,13 +527,13 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet,  18*CENT); // now we have 5+6+7+8+18+20+30
 
         // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18
-        const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 16 * CENT, CENT);
+        const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT);
         BOOST_CHECK(result12);
         BOOST_CHECK_EQUAL(result12->GetSelectedValue(), 18 * CENT);  // we should get 18 in 1 coin
         BOOST_CHECK_EQUAL(result12->GetInputSet().size(), 1U); // because in the event of a tie, the biggest coin wins
 
         // now try making 11 cents.  we should get 5+6
-        const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 11 * CENT, CENT);
+        const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 11 * CENT, CENT);
         BOOST_CHECK(result13);
         BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT);
         BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U);
@@ -543,19 +543,19 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet,  2*COIN);
         add_coin(available_coins, *wallet,  3*COIN);
         add_coin(available_coins, *wallet,  4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents
-        const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 95 * CENT, CENT);
+        const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 95 * CENT, CENT);
         BOOST_CHECK(result14);
         BOOST_CHECK_EQUAL(result14->GetSelectedValue(), 1 * COIN);  // we should get 1 BTC in 1 coin
         BOOST_CHECK_EQUAL(result14->GetInputSet().size(), 1U);
 
-        const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 195 * CENT, CENT);
+        const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 195 * CENT, CENT);
         BOOST_CHECK(result15);
         BOOST_CHECK_EQUAL(result15->GetSelectedValue(), 2 * COIN);  // we should get 2 BTC in 1 coin
         BOOST_CHECK_EQUAL(result15->GetInputSet().size(), 1U);
 
         // empty the wallet and start again, now with fractions of a cent, to test small change avoidance
 
-        available_coins.clear();
+        available_coins.Clear();
         add_coin(available_coins, *wallet, CENT * 1 / 10);
         add_coin(available_coins, *wallet, CENT * 2 / 10);
         add_coin(available_coins, *wallet, CENT * 3 / 10);
@@ -564,7 +564,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
 
         // try making 1 * CENT from the 1.5 * CENT
         // we'll get change smaller than CENT whatever happens, so can expect CENT exactly
-        const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT, CENT);
+        const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT);
         BOOST_CHECK(result16);
         BOOST_CHECK_EQUAL(result16->GetSelectedValue(), CENT);
 
@@ -572,7 +572,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet, 1111*CENT);
 
         // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5
-        const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT);
+        const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT);
         BOOST_CHECK(result17);
         BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * CENT); // we should get the exact amount
 
@@ -581,17 +581,17 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         add_coin(available_coins, *wallet, CENT * 7 / 10);
 
         // and try again to make 1.0 * CENT
-        const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT);
+        const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT);
         BOOST_CHECK(result18);
         BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * CENT); // we should get the exact amount
 
         // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf)
         // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change
-        available_coins.clear();
+        available_coins.Clear();
         for (int j = 0; j < 20; j++)
             add_coin(available_coins, *wallet, 50000 * COIN);
 
-        const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 500000 * COIN, CENT);
+        const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 500000 * COIN, CENT);
         BOOST_CHECK(result19);
         BOOST_CHECK_EQUAL(result19->GetSelectedValue(), 500000 * COIN); // we should get the exact amount
         BOOST_CHECK_EQUAL(result19->GetInputSet().size(), 10U); // in ten coins
@@ -600,41 +600,41 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         // we need to try finding an exact subset anyway
 
         // sometimes it will fail, and so we use the next biggest coin:
-        available_coins.clear();
+        available_coins.Clear();
         add_coin(available_coins, *wallet, CENT * 5 / 10);
         add_coin(available_coins, *wallet, CENT * 6 / 10);
         add_coin(available_coins, *wallet, CENT * 7 / 10);
         add_coin(available_coins, *wallet, 1111 * CENT);
-        const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 1 * CENT, CENT);
+        const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT);
         BOOST_CHECK(result20);
         BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * CENT); // we get the bigger coin
         BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U);
 
         // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0)
-        available_coins.clear();
+        available_coins.Clear();
         add_coin(available_coins, *wallet, CENT * 4 / 10);
         add_coin(available_coins, *wallet, CENT * 6 / 10);
         add_coin(available_coins, *wallet, CENT * 8 / 10);
         add_coin(available_coins, *wallet, 1111 * CENT);
-        const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT, CENT);
+        const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT);
         BOOST_CHECK(result21);
         BOOST_CHECK_EQUAL(result21->GetSelectedValue(), CENT);   // we should get the exact amount
         BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6
 
         // test avoiding small change
-        available_coins.clear();
+        available_coins.Clear();
         add_coin(available_coins, *wallet, CENT * 5 / 100);
         add_coin(available_coins, *wallet, CENT * 1);
         add_coin(available_coins, *wallet, CENT * 100);
 
         // trying to make 100.01 from these three coins
-        const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT * 10001 / 100, CENT);
+        const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 10001 / 100, CENT);
         BOOST_CHECK(result22);
         BOOST_CHECK_EQUAL(result22->GetSelectedValue(), CENT * 10105 / 100); // we should get all coins
         BOOST_CHECK_EQUAL(result22->GetInputSet().size(), 3U);
 
         // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change
-        const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT);
+        const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT);
         BOOST_CHECK(result23);
         BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * CENT);
         BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U);
@@ -642,14 +642,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
 
     // test with many inputs
     for (CAmount amt=1500; amt < COIN; amt*=10) {
-        available_coins.clear();
+        available_coins.Clear();
         // Create 676 inputs (=  (old MAX_STANDARD_TX_SIZE == 100000)  / 148 bytes per input)
         for (uint16_t j = 0; j < 676; j++)
             add_coin(available_coins, *wallet, amt);
 
         // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times.
         for (int i = 0; i < RUN_TESTS; i++) {
-            const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_confirmed), 2000, CENT);
+            const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 2000, CENT);
             BOOST_CHECK(result24);
 
             if (amt - 2000 < CENT) {
@@ -668,7 +668,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
 
     // test randomness
     {
-        available_coins.clear();
+        available_coins.Clear();
         for (int i2 = 0; i2 < 100; i2++)
             add_coin(available_coins, *wallet, COIN);
 
@@ -676,9 +676,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
         for (int i = 0; i < RUN_TESTS; i++) {
             // picking 50 from 100 coins doesn't depend on the shuffle,
             // but does depend on randomness in the stochastic approximation code
-            const auto result25 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT);
+            const auto result25 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT);
             BOOST_CHECK(result25);
-            const auto result26 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT);
+            const auto result26 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT);
             BOOST_CHECK(result26);
             BOOST_CHECK(!EqualResult(*result25, *result26));
 
@@ -689,9 +689,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
                 // When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice
                 // which will cause it to fail.
                 // To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail
-                const auto result27 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT);
+                const auto result27 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT);
                 BOOST_CHECK(result27);
-                const auto result28 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT);
+                const auto result28 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT);
                 BOOST_CHECK(result28);
                 if (EqualResult(*result27, *result28))
                     fails++;
@@ -712,9 +712,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
             int fails = 0;
             for (int j = 0; j < RANDOM_REPEATS; j++)
             {
-                const auto result29 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT);
+                const auto result29 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT);
                 BOOST_CHECK(result29);
-                const auto result30 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT);
+                const auto result30 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT);
                 BOOST_CHECK(result30);
                 if (EqualResult(*result29, *result30))
                     fails++;
@@ -740,7 +740,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
         add_coin(available_coins, *wallet, 1000 * COIN);
     add_coin(available_coins, *wallet, 3 * COIN);
 
-    const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins.all(), *wallet, filter_standard), 1003 * COIN, CENT, rand);
+    const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1003 * COIN, CENT, rand);
     BOOST_CHECK(result);
     BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN);
     BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U);
@@ -973,7 +973,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test)
     };
     CCoinControl cc;
     cc.m_allow_other_inputs = false;
-    COutput output = available_coins.all().at(0);
+    COutput output = available_coins.All().at(0);
     cc.SetInputWeight(output.outpoint, 148);
     cc.SelectExternal(output.outpoint, output.txout);
 
@@ -995,7 +995,7 @@ static util::Result<SelectionResult> SelectCoinsSRDResult(const CAmount& target,
 
     CoinEligibilityFilter filter(0, 0, 0);
     auto available_coins = coin_setup(*wallet);
-    auto groups = GroupOutputs(*wallet, available_coins.all(), cs_params, filter, /*positive_only=*/true);
+    auto groups = GroupOutputs(*wallet, available_coins.All(), cs_params, filter, /*positive_only=*/true);
     return wallet::SelectCoinsSRD(groups, target, cs_params.rng_fast, max_weight, false);
 }
 
@@ -1161,7 +1161,7 @@ BOOST_AUTO_TEST_CASE(minimum_inputs_test)
     CCoinControl coin_control{};
     coin_control.m_allow_other_inputs = false;
     coin_control.fRequireAllInputs = false;
-    for (const auto& coin : available_coins.all()) {
+    for (const auto& coin : available_coins.All()) {
         coin_control.Select(coin.outpoint);
     }
 
diff --git a/src/wallet/test/platformkeys_tests.cpp b/src/wallet/test/platformkeys_tests.cpp
index 3472349c014b..5052f5ad75aa 100644
--- a/src/wallet/test/platformkeys_tests.cpp
+++ b/src/wallet/test/platformkeys_tests.cpp
@@ -327,7 +327,7 @@ BOOST_FIXTURE_TEST_CASE(friendship_own_chain_is_spendable, FriendshipWalletSetup
 
     CCoinControl coin_control;
     coin_control.m_include_unsafe_inputs = true;
-    BOOST_CHECK_EQUAL(AvailableCoins(*m_wallet, &coin_control).size(), 1U);
+    BOOST_CHECK_EQUAL(AvailableCoins(*m_wallet, &coin_control).Size(), 1U);
 }
 
 //! The contact's receiving chain must never become ours. If it did, payments to
@@ -677,7 +677,7 @@ BOOST_FIXTURE_TEST_CASE(recovery_import_after_restore_is_spendable, SeededWallet
 
     CCoinControl coin_control;
     coin_control.m_include_unsafe_inputs = true;
-    BOOST_CHECK_EQUAL(AvailableCoins(*m_wallet_b, &coin_control).size(), 1U);
+    BOOST_CHECK_EQUAL(AvailableCoins(*m_wallet_b, &coin_control).Size(), 1U);
 }
 
 //! Recovery re-runs re-import friendships it already imported; that must
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index db1bccb74f4f..b04aba66336d 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -740,7 +740,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     // Lock both coins. Confirm number of available coins drops to 0.
     {
         LOCK(wallet->cs_wallet);
-        BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 2U);
+        BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 2U);
     }
     for (const auto& group : list) {
         for (const auto& coin : group.second) {
@@ -750,7 +750,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     }
     {
         LOCK(wallet->cs_wallet);
-        BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 0U);
+        BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 0U);
     }
     // Confirm ListCoins still returns same result as before, despite coins
     // being locked.
@@ -1441,7 +1441,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
         // Lock all other coins which were already in the wallet
         {
             LOCK(wallet->cs_wallet);
-            for (auto coin : AvailableCoinsListUnspent(*wallet).all()) {
+            for (auto coin : AvailableCoinsListUnspent(*wallet).All()) {
                 if (std::find(setCoins.begin(), setCoins.end(), coin.outpoint) == setCoins.end()) {
                     wallet->LockCoin(coin.outpoint);
                 }
@@ -1497,7 +1497,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
             // Lock all other coins
             {
                 LOCK(wallet->cs_wallet);
-                for (auto coin : AvailableCoinsListUnspent(*wallet).all()) {
+                for (auto coin : AvailableCoinsListUnspent(*wallet).All()) {
                     wallet->LockCoin(coin.outpoint);
                 }
             }

From bcea7fb4c1f4fab0fa30a27e2e134ae30494e0ca Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Thu, 27 Oct 2022 17:48:51 -0400
Subject: [PATCH 07/15] Merge bitcoin/bitcoin#25685: wallet: Faster transaction
 creation by removing pre-set-inputs fetching responsibility from Coin
 Selection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

3fcb545ab26be3e785b5e5654be0bdc77099d827 bench: benchmark transaction creation process (furszy)
a8a75346d7e7247596c8a580d65ceaad49c97b97 wallet: SelectCoins, return early if target is covered by preset-inputs (furszy)
f41712a734dc119f8a5e053a9cfa1f0411b5e8f1 wallet: simplify preset inputs selection target check (furszy)
5baedc33519661af9d19efcefd23dca8998d2547 wallet: remove fetch pre-selected-inputs responsibility from SelectCoins (furszy)
295852f61998a025b0b28a0671e6e1cf0dc08d0d wallet: encapsulate pre-selected-inputs lookup into its own function (furszy)
37e7887cb4bfd7db6eb462ed0741c45aea22a990 wallet: skip manually selected coins from 'AvailableCoins' result (furszy)
94c0766b0cd1990c1399a745c88c2ba4c685d8d1 wallet: skip available coins fetch if "other inputs" are disallowed (furszy)

Pull request description:

  #### # Context (Current Flow on Master)

  In the transaction creation process, in order to select which coins the new transaction will spend,
  we first obtain all the available coins known by the wallet, which means walking-through the
  wallet txes map, gathering the ones that fulfill certain spendability requirements in a vector.

  This coins vector is then provided to the Coin Selection process, which first checks if the user
  has manually selected any input (which could be internal, aka known by the wallet, or external),
  and if it does, it fetches them by searching each of them inside the wallet and/or inside the
  Coin Control external tx data.

  Then, after finding the pre-selected-inputs and gathering them in a vector, the Coin Selection
  process walks-through the entire available coins vector once more just to erase coins that are
  in both vectors. So the Coin Selection process doesn’t pick them twice (duplicate inputs inside
  the same transaction).

  #### # Process Workflow Changes

  Now, a new method, `FetchCoins` will be responsible for:
  1) Lookup the user pre-selected-inputs (which can be internal or external).
  2) And, fetch the available coins in the wallet (excluding the already fetched ones).

  Which will occur prior to the Coin Selection process. Which allows us to never include the
  pre-selected-inputs inside the available coins vector in the first place, as well as doing other
  nice improvements (written below).

  So, Coin Selection can perform its main responsibility without mixing it with having to fetch
  internal/external coins nor any slow and unneeded duplicate coins verification.

  #### # Summarizing the Improvements:

  1) If any pre-selected-input lookup fail, the process will return the error right away.
      (before, the wallet was fetching all the wallet available coins, walking through the
      entire txes map, and then failing for an invalid pre-selected-input inside SelectCoins)

  2) The pre-selected-inputs lookup failure causes are properly described on the return error.
      (before, we were returning an "Insufficient Funds" error for everything, even if the failure
      was due a not solvable external input)

  3) **Faster Coin Selection**: no longer need to "remove the pre-set inputs from the available coins
      vector so that Coin Selection doesn't pick them" (which meant to loop-over the entire
      available coins vector at Coin Selection time, erasing duplicate coins that were pre-selected).

      Now, the available coins vector, which is built after the pre-selected-inputs fetching,
      doesn’t include the already selected inputs in the first place.

  4) **Faster transaction creation** for transactions that only use manually selected inputs.

      We now will return early, as soon as we finish fetching the pre-selected-inputs and
      not perform the resources expensive calculation of walking-through the entire wallet
      txes map to obtain the available coins (coins that we will not use).

  ---------------------------

  Added a new bench (f6d0bb2) measuring the transaction creation process, for a wallet with ~250k UTXO, only using the pre-selected-inputs inside coin control. Setting `m_allow_other_inputs=false` to disallow the wallet to include coins automatically.

  #### Result on this PR (tip f6d0bb2d):

  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |        1,048,675.00 |              953.58 |    0.3% |      0.06 | `WalletCreateTransaction`

  vs

  #### Result on master (tip 4a4289e2):

  |               ns/op |                op/s |    err% |     total | benchmark
  |--------------------:|--------------------:|--------:|----------:|:----------
  |       96,373,458.20 |               10.38 |    0.2% |      5.30 | `WalletCreateTransaction`

  The benchmark took to run in master: **96.37 milliseconds**, while in this PR: **1 millisecond**  🚀 .

ACKs for top commit:
  S3RK:
    Code Review ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827
  achow101:
    ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827
  aureleoules:
    reACK 3fcb545ab26be3e785b5e5654be0bdc77099d827

Tree-SHA512: 42f833e92f40c348007ca565a4c98039e6f1ff25d8322bc2b27115824744779baf0b0a38452e4e2cdcba45076473f1028079bbd0f670020481ec5d3db42e4731

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/Makefile.bench.include                   |   1 +
 src/bench/wallet_create_tx.cpp               | 143 ++++++++++++
 src/qt/sendcoinsdialog.cpp                   |   4 +-
 src/test/util/wallet.cpp                     |   7 +-
 src/test/util/wallet.h                       |   5 +-
 src/wallet/coincontrol.h                     |   2 +-
 src/wallet/coinselection.cpp                 |   6 +
 src/wallet/coinselection.h                   |   1 +
 src/wallet/spend.cpp                         | 216 +++++++++----------
 src/wallet/spend.h                           |  40 +++-
 src/wallet/test/coinselector_tests.cpp       |  31 ++-
 test/functional/rpc_psbt.py                  |   2 +-
 test/functional/wallet_fundrawtransaction.py |  16 +-
 test/functional/wallet_send.py               |   2 +-
 14 files changed, 339 insertions(+), 137 deletions(-)
 create mode 100644 src/bench/wallet_create_tx.cpp

diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 7dec51f2b0c1..9bd3238c2b5d 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -94,6 +94,7 @@ bench_bench_dash_SOURCES += bench/coin_selection.cpp
 bench_bench_dash_SOURCES += bench/wallet_balance.cpp
 bench_bench_dash_SOURCES += bench/wallet_create.cpp
 bench_bench_dash_SOURCES += bench/wallet_loading.cpp
+bench_bench_dash_SOURCES += bench/wallet_create_tx.cpp
 bench_bench_dash_LDADD += $(BDB_LIBS) $(SQLITE_LIBS)
 endif
 
diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
new file mode 100644
index 000000000000..684e3e22bad4
--- /dev/null
+++ b/src/bench/wallet_create_tx.cpp
@@ -0,0 +1,143 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <chainparams.h>
+#include <wallet/coincontrol.h>
+#include <consensus/merkle.h>
+#include <kernel/chain.h>
+#include <node/context.h>
+#include <test/util/setup_common.h>
+#include <test/util/wallet.h>
+#include <validation.h>
+#include <wallet/spend.h>
+#include <wallet/wallet.h>
+
+using wallet::CWallet;
+using wallet::CreateMockWalletDatabase;
+using wallet::DBErrors;
+using wallet::WALLET_FLAG_DESCRIPTORS;
+
+struct TipBlock
+{
+    uint256 prev_block_hash;
+    int64_t prev_block_time;
+    int tip_height;
+};
+
+TipBlock getTip(const CChainParams& params, const node::NodeContext& context)
+{
+    auto tip = WITH_LOCK(::cs_main, return context.chainman->ActiveTip());
+    return (tip) ? TipBlock{tip->GetBlockHash(), tip->GetBlockTime(), tip->nHeight} :
+           TipBlock{params.GenesisBlock().GetHash(), params.GenesisBlock().GetBlockTime(), 0};
+}
+
+void generateFakeBlock(const CChainParams& params,
+                       const node::NodeContext& context,
+                       CWallet& wallet,
+                       const CScript& coinbase_out_script)
+{
+    TipBlock tip{getTip(params, context)};
+
+    // Create block
+    CBlock block;
+    CMutableTransaction coinbase_tx;
+    coinbase_tx.vin.resize(1);
+    coinbase_tx.vin[0].prevout.SetNull();
+    coinbase_tx.vout.resize(2);
+    coinbase_tx.vout[0].scriptPubKey = coinbase_out_script;
+    coinbase_tx.vout[0].nValue = 49 * COIN;
+    coinbase_tx.vin[0].scriptSig = CScript() << ++tip.tip_height << OP_0;
+    coinbase_tx.vout[1].scriptPubKey = coinbase_out_script; // extra output
+    coinbase_tx.vout[1].nValue = 1 * COIN;
+    block.vtx = {MakeTransactionRef(std::move(coinbase_tx))};
+
+    block.nVersion = VERSIONBITS_LAST_OLD_BLOCK_VERSION;
+    block.hashPrevBlock = tip.prev_block_hash;
+    block.hashMerkleRoot = BlockMerkleRoot(block);
+    block.nTime = ++tip.prev_block_time;
+    block.nBits = params.GenesisBlock().nBits;
+    block.nNonce = 0;
+
+    {
+        LOCK(::cs_main);
+        // Add it to the index
+        const uint256 hash{block.GetHash()};
+        CBlockIndex* pindex{context.chainman->m_blockman.AddToBlockIndex(block, hash, context.chainman->m_best_header)};
+        // add it to the chain
+        context.chainman->ActiveChain().SetTip(*pindex);
+    }
+
+    // notify wallet
+    const auto& pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip());
+    wallet.blockConnected(kernel::MakeBlockInfo(pindex, &block));
+}
+
+struct PreSelectInputs {
+    // How many coins from the wallet the process should select
+    int num_of_internal_inputs;
+    // future: this could have external inputs as well.
+};
+
+static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type, bool allow_other_inputs, std::optional<PreSelectInputs> preset_inputs)
+{
+    const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
+
+    CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", gArgs, CreateMockWalletDatabase()};
+    {
+        LOCK(wallet.cs_wallet);
+        wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+        wallet.SetupDescriptorScriptPubKeyMans("", "");
+        if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false);
+    }
+
+    // Generate destinations
+    CScript dest = GetScriptForDestination(getNewDestination(wallet));
+
+    // Generate chain; each coinbase will have two outputs to fill-up the wallet
+    const auto& params = Params();
+    unsigned int chain_size = 5000; // 5k blocks means 10k UTXO for the wallet (minus 200 due COINBASE_MATURITY)
+    for (unsigned int i = 0; i < chain_size; ++i) {
+        generateFakeBlock(params, test_setup->m_node, wallet, dest);
+    }
+
+    // Check available balance
+    auto bal = wallet::GetAvailableBalance(wallet); // Cache
+    assert(bal == 50 * COIN * (chain_size - COINBASE_MATURITY));
+
+    wallet::CCoinControl coin_control;
+    coin_control.m_allow_other_inputs = allow_other_inputs;
+
+    CAmount target = 0;
+    if (preset_inputs) {
+        // Select inputs, each has 49 BTC
+        const auto& res = WITH_LOCK(wallet.cs_wallet,
+                                    return wallet::AvailableCoins(wallet, nullptr, std::nullopt, 1, MAX_MONEY,
+                                                                  MAX_MONEY, preset_inputs->num_of_internal_inputs));
+        for (int i=0; i < preset_inputs->num_of_internal_inputs; i++) {
+            const auto& coin{res.coins.at(output_type)[i]};
+            target += coin.txout.nValue;
+            coin_control.Select(coin.outpoint);
+        }
+    }
+
+    // If automatic coin selection is enabled, add the value of another UTXO to the target
+    if (coin_control.m_allow_other_inputs) target += 50 * COIN;
+    std::vector<wallet::CRecipient> recipients = {{dest, target, true}};
+
+    bench.epochIterations(5).run([&] {
+        LOCK(wallet.cs_wallet);
+        const auto& tx_res = CreateTransaction(wallet, recipients, -1, coin_control);
+        assert(tx_res);
+    });
+}
+
+static void WalletCreateTxUseOnlyPresetInputs(benchmark::Bench& bench) { WalletCreateTx(bench, OutputType::LEGACY, /*allow_other_inputs=*/false,
+                                                                                        {{/*num_of_internal_inputs=*/4}}); }
+
+static void WalletCreateTxUsePresetInputsAndCoinSelection(benchmark::Bench& bench) { WalletCreateTx(bench, OutputType::LEGACY, /*allow_other_inputs=*/true,
+                                                                                                    {{/*num_of_internal_inputs=*/4}}); }
+
+BENCHMARK(WalletCreateTxUseOnlyPresetInputs, benchmark::PriorityLevel::LOW)
+BENCHMARK(WalletCreateTxUsePresetInputsAndCoinSelection, benchmark::PriorityLevel::LOW)
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 1b25c171abb5..e2a2be3e13dd 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -324,7 +324,9 @@ bool SendCoinsDialog::send(const QList<SendCoinsRecipient>& recipients, QString&
 
     updateCoinControlState();
 
-    prepareStatus = model->prepareTransaction(*m_current_transaction, *m_coin_control);
+    CCoinControl coin_control = *m_coin_control;
+    coin_control.m_allow_other_inputs = !coin_control.HasSelected(); // future, could introduce a checkbox to customize this value.
+    prepareStatus = model->prepareTransaction(*m_current_transaction, coin_control);
 
     // process prepareStatus and on error generate message shown to user
     processSendCoinsReturn(prepareStatus,
diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp
index e1d59b3bc76e..d610fe314428 100644
--- a/src/test/util/wallet.cpp
+++ b/src/test/util/wallet.cpp
@@ -25,7 +25,12 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
 #ifdef ENABLE_WALLET
 std::string getnewaddress(CWallet& w)
 {
-    return EncodeDestination(*Assert(w.GetNewDestination("")));
+    return EncodeDestination(getNewDestination(w));
+}
+
+CTxDestination getNewDestination(CWallet& w)
+{
+    return *Assert(w.GetNewDestination(""));
 }
 
 // void importaddress(CWallet& wallet, const std::string& address)
diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h
index d8a81c2d8224..a95315c08087 100644
--- a/src/test/util/wallet.h
+++ b/src/test/util/wallet.h
@@ -5,6 +5,7 @@
 #ifndef BITCOIN_TEST_UTIL_WALLET_H
 #define BITCOIN_TEST_UTIL_WALLET_H
 
+#include <outputtype.h>
 #include <string>
 
 namespace wallet {
@@ -20,8 +21,10 @@ extern const std::string ADDRESS_BCRT1_UNSPENDABLE;
 
 /** Import the address to the wallet */
 void importaddress(wallet::CWallet& wallet, const std::string& address);
-/** Returns a new address from the wallet */
+/** Returns a new encoded destination from the wallet */
 std::string getnewaddress(wallet::CWallet& w);
+/** Returns a new destination, of an specific type, from the wallet */
+CTxDestination getNewDestination(wallet::CWallet& w);
 
 
 #endif // BITCOIN_TEST_UTIL_WALLET_H
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index b5ada332d828..0d44adfa0ee4 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -36,7 +36,7 @@ class CCoinControl
     bool m_include_unsafe_inputs = false;
     //! If true, the selection process can add extra unselected inputs from the wallet
     //! while requires all selected inputs be used
-    bool m_allow_other_inputs = false;
+    bool m_allow_other_inputs = true;
     //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with m_allow_other_inputs
     bool fRequireAllInputs = true;
     //! Includes watch only addresses which are solvable
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 28c73863dff9..afe46974162e 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -587,6 +587,12 @@ void SelectionResult::AddInput(const OutputGroup& group)
     m_use_effective = !group.m_subtract_fee_outputs;
 }
 
+void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs)
+{
+    util::insert(m_selected_inputs, inputs);
+    m_use_effective = !subtract_fee_outputs;
+}
+
 void SelectionResult::Merge(const SelectionResult& other)
 {
     m_target += other.m_target;
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 5dfb048c7002..c27d63f60cc3 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -323,6 +323,7 @@ struct SelectionResult
     void Clear();
 
     void AddInput(const OutputGroup& group);
+    void AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs);
 
     /** Calculates and stores the waste for this selection via GetSelectionWaste */
     void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee);
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 54322308cabc..f0f8f3cb989f 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -139,6 +139,56 @@ static OutputType GetOutputType(TxoutType type, bool is_from_p2sh)
     }
 }
 
+// Fetch and validate the coin control selected inputs.
+// Coins could be internal (from the wallet) or external.
+util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control,
+                                            const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+{
+    PreSelectedInputs result;
+    for (const COutPoint& outpoint : coin_control.ListSelected()) {
+        int input_bytes = -1;
+        CTxOut txout;
+        if (auto ptr_wtx = wallet.GetWalletTx(outpoint.hash)) {
+            // Clearly invalid input, fail
+            if (ptr_wtx->tx->vout.size() <= outpoint.n) {
+                return util::Error{strprintf(_("Invalid pre-selected input %s"), outpoint.ToString())};
+            }
+            txout = ptr_wtx->tx->vout.at(outpoint.n);
+            input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control);
+        } else {
+            // The input is external. We did not find the tx in mapWallet.
+            const auto out{coin_control.GetExternalOutput(outpoint)};
+            if (!out) {
+                return util::Error{strprintf(_("Not found pre-selected input %s"), outpoint.ToString())};
+            }
+            txout = *out;
+        }
+
+        if (input_bytes == -1) {
+            input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control);
+        }
+
+        if (coin_control.nCoinType == CoinType::ONLY_FULLY_MIXED) {
+            // Make sure to include mixed preset inputs only,
+            // even if some non-mixed inputs were manually selected via CoinControl
+            if (!wallet.IsFullyMixed(outpoint)) continue;
+        }
+        // If available, override calculated size with coin control specified size
+        if (coin_control.HasInputWeight(outpoint)) {
+            input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0);
+        }
+
+        if (input_bytes == -1) {
+            return util::Error{strprintf(_("Not solvable pre-selected input %s"), outpoint.ToString())}; // Not solvable, can't estimate size for fee
+        }
+
+        /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */
+        COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
+        result.Insert(output, coin_selection_params.m_subtract_fee_outputs);
+    }
+    return result;
+}
+
 CoinsResult AvailableCoins(const CWallet& wallet,
                            const CCoinControl* coinControl,
                            std::optional<CFeeRate> feerate,
@@ -227,7 +277,8 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             if (output.nValue < nMinimumAmount || output.nValue > nMaximumAmount)
                 continue;
 
-            if (coinControl && coinControl->HasSelected() && !coinControl->m_allow_other_inputs && !coinControl->IsSelected(outpoint))
+            // Skip manually selected coins (the caller can fetch them directly)
+            if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint))
                 continue;
 
             if (wallet.IsLockedCoin(outpoint) && nCoinType != CoinType::ONLY_MASTERNODE_COLLATERAL)
@@ -542,109 +593,43 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
     return *std::min_element(results.begin(), results.end());
 }
 
-std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params)
+std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
+                                           const CAmount& nTargetValue, const CCoinControl& coin_control,
+                                           const CoinSelectionParams& coin_selection_params)
 {
     // Note: this function should never be used for "always free" tx types like dstx
-    CoinType nCoinType = coin_control.nCoinType;
-    CAmount value_to_select = nTargetValue;
-
-    OutputGroup preset_inputs(coin_selection_params);
+    // Deduct preset inputs amount from the search target
+    CAmount selection_target = nTargetValue - pre_set_inputs.total_amount;
 
-    // calculate value from preset inputs and store them
-    std::set<COutPoint> preset_coins;
+    // Return if automatic coin selection is disabled, and we don't cover the selection target
+    if (!coin_control.m_allow_other_inputs && selection_target > 0) return std::nullopt;
 
-    for (const COutPoint& outpoint : coin_control.ListSelected()) {
-        int input_bytes = -1;
-        CTxOut txout;
-        auto ptr_wtx = wallet.GetWalletTx(outpoint.hash);
-        if (ptr_wtx) {
-            // Clearly invalid input, fail
-            if (ptr_wtx->tx->vout.size() <= outpoint.n) {
-                return std::nullopt;
-            }
-            txout = ptr_wtx->tx->vout.at(outpoint.n);
-            input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control);
-        } else {
-            // The input is external. We did not find the tx in mapWallet.
-            const auto out{coin_control.GetExternalOutput(outpoint)};
-            if (!out) {
-                return std::nullopt;
-            }
-            txout = *out;
-        }
-
-        if (input_bytes == -1) {
-            input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control);
-        }
-        if (nCoinType == CoinType::ONLY_FULLY_MIXED) {
-            // Make sure to include mixed preset inputs only,
-            // even if some non-mixed inputs were manually selected via CoinControl
-            if (!wallet.IsFullyMixed(outpoint)) continue;
-        }
-        // If available, override calculated size with coin control specified size
-        if (coin_control.HasInputWeight(outpoint)) {
-            input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0);
-        }
-
-        if (input_bytes == -1) {
-            return std::nullopt; // Not solvable, can't estimate size for fee
-        }
-
-        /* Set some defaults for depth, spendable, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */
-        COutput output(outpoint, txout, /*depth=*/ 0, input_bytes, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, coin_selection_params.m_effective_feerate);
-        if (coin_selection_params.m_subtract_fee_outputs) {
-            value_to_select -= output.txout.nValue;
-        } else {
-            value_to_select -= output.GetEffectiveValue();
-        }
-        preset_coins.insert(outpoint);
-        /* Set ancestors and descendants to 0 as they don't matter for preset inputs since no actual selection is being done.
-         * positive_only is set to false because we want to include all preset inputs, even if they are dust.
-         */
-        preset_inputs.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
-    }
-
-    // coin control -> return all selected outputs (we want all selected to go into the transaction for sure)
-    if (coin_control.HasSelected() && !coin_control.m_allow_other_inputs) {
+    // Return if we can cover the target only with the preset inputs
+    if (selection_target <= 0) {
         SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
-        bool all_inputs{coin_control.fRequireAllInputs};
-        if (!all_inputs) {
-            // Calculate the smallest set of inputs required to meet nTargetValue from available_coins
-            bool success{false};
-            OutputGroup preset_candidates(coin_selection_params);
-            for (const COutput& out : available_coins.All()) {
-                if (!out.spendable) continue;
-                if (preset_coins.count(out.outpoint)) {
-                    preset_candidates.Insert(out, /*ancestors=*/0, /*descendants=*/0, /*positive_only=*/false);
-                }
-                if (preset_candidates.GetSelectionAmount() >= nTargetValue) {
-                    result.AddInput(preset_candidates);
-                    success = true;
-                    break;
-                }
-            }
-            // Couldn't meet target, add all inputs
-            if (!success) all_inputs = true;
-        }
-        if (all_inputs) {
-            result.AddInput(preset_inputs);
-        }
-
-        if (!coin_selection_params.m_subtract_fee_outputs && result.GetSelectedEffectiveValue() < nTargetValue) {
-            return std::nullopt;
-        } else if (result.GetSelectedValue() < nTargetValue) {
-            return std::nullopt;
-        }
-
+        result.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs);
         result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
         return result;
     }
 
-    // remove preset inputs from coins so that Coin Selection doesn't pick them.
-    if (coin_control.HasSelected()) {
-        available_coins.Erase(preset_coins);
+    // Start wallet Coin Selection procedure
+    auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params);
+    if (!op_selection_result) return op_selection_result;
+
+    // If needed, add preset inputs to the automatic coin selection result
+    if (!pre_set_inputs.coins.empty()) {
+        SelectionResult preselected(pre_set_inputs.total_amount, SelectionAlgorithm::MANUAL);
+        preselected.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs);
+        op_selection_result->Merge(preselected);
+        op_selection_result->ComputeAndSetWaste(coin_selection_params.min_viable_change,
+                                                coin_selection_params.m_cost_of_change,
+                                                coin_selection_params.m_change_fee);
     }
+    return op_selection_result;
+}
 
+std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params)
+{
     unsigned int limit_ancestor_count = 0;
     unsigned int limit_descendant_count = 0;
     wallet.chain().getPackageLimits(limit_ancestor_count, limit_descendant_count);
@@ -661,16 +646,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
         available_coins.Shuffle(coin_selection_params.rng_fast);
     }
 
-    SelectionResult preselected(preset_inputs.GetSelectionAmount(), SelectionAlgorithm::MANUAL);
-    preselected.AddInput(preset_inputs);
-
     // Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the
     // transaction at a target feerate. If an attempt fails, more attempts may be made using a more
     // permissive CoinEligibilityFilter.
     std::optional<SelectionResult> res = [&] {
-        // Pre-selected inputs already cover the target amount.
-        if (value_to_select <= 0) return std::make_optional(SelectionResult(value_to_select, SelectionAlgorithm::MANUAL));
-
+        CoinType nCoinType = coin_control.nCoinType;
         // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
         // confirmations on outputs received from other wallets and only spend confirmed change.
         if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false)}) return r1;
@@ -720,14 +700,6 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
         return std::optional<SelectionResult>();
     }();
 
-    if (!res) return std::nullopt;
-
-    // Add preset inputs to result
-    res->Merge(preselected);
-    if (res->GetAlgo() == SelectionAlgorithm::MANUAL) {
-        res->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee);
-    }
-
     return res;
 }
 
@@ -988,17 +960,29 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
         return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")};
     }
 
-    // Get available coins
-    auto available_coins = AvailableCoins(wallet,
-                                              &coin_control,
-                                              coin_selection_params.m_effective_feerate,
-                                              1,            /*nMinimumAmount*/
-                                              MAX_MONEY,    /*nMaximumAmount*/
-                                              MAX_MONEY,    /*nMinimumSumAmount*/
-                                              0);           /*nMaximumCount*/
+    // Fetch manually selected coins
+    PreSelectedInputs preset_inputs;
+    if (coin_control.HasSelected()) {
+        auto res_fetch_inputs = FetchSelectedInputs(wallet, coin_control, coin_selection_params);
+        if (!res_fetch_inputs) return util::Error{util::ErrorString(res_fetch_inputs)};
+        preset_inputs = *res_fetch_inputs;
+    }
+
+    // Fetch wallet available coins if "other inputs" are
+    // allowed (coins automatically selected by the wallet)
+    CoinsResult available_coins;
+    if (coin_control.m_allow_other_inputs) {
+        available_coins = AvailableCoins(wallet,
+                                         &coin_control,
+                                         coin_selection_params.m_effective_feerate,
+                                         1,            /*nMinimumAmount*/
+                                         MAX_MONEY,    /*nMaximumAmount*/
+                                         MAX_MONEY,    /*nMinimumSumAmount*/
+                                         0);           /*nMaximumCount*/
+    }
 
     // Choose coins to use
-    std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
+    std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
     if (!result) {
         if (coin_control.nCoinType == CoinType::ONLY_NONDENOMINATED) {
             return util::Error{_("Unable to locate enough non-denominated funds for this transaction.")};
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 1abba9038482..6e8550ca65b0 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -121,9 +121,35 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
 std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
                                                      const CoinSelectionParams& coin_selection_params);
 
+// User manually selected inputs that must be part of the transaction
+struct PreSelectedInputs
+{
+    std::set<COutput> 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
+    CAmount total_amount{0};
+
+    void Insert(const COutput& output, bool subtract_fee_outputs)
+    {
+        if (subtract_fee_outputs) {
+            total_amount += output.txout.nValue;
+        } else {
+            total_amount += output.GetEffectiveValue();
+        }
+        coins.insert(output);
+    }
+};
+
 /**
- * Select a set of coins such that nTargetValue is met and at least
- * all coins from coin_control are selected; never select unconfirmed coins if they are not ours
+ * Fetch and validate coin control selected inputs.
+ * Coins could be internal (from the wallet) or external.
+*/
+util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control,
+                                                    const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+
+/**
+ * Select a set of coins such that nTargetValue is met; never select unconfirmed coins if they are not ours
  * param@[in]   wallet                 The wallet which provides data necessary to spend the selected coins
  * param@[in]   available_coins        The struct of coins, organized by OutputType, available for selection prior to filtering
  * param@[in]   nTargetValue           The target value
@@ -132,9 +158,17 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
  * returns                             If successful, a SelectionResult containing the selected coins
  *                                     If failed, a nullopt.
  */
-std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control,
+std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control,
                  const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
+/**
+ * Select all coins from coin_control, and if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to
+ * select a set of coins such that nTargetValue - pre_set_inputs.total_amount is met.
+ */
+std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
+                                           const CAmount& nTargetValue, const CCoinControl& coin_control,
+                                           const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+
 struct CreatedTransactionResult
 {
     CTransactionRef tx;
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 75b42019c251..d47057244756 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -349,9 +349,13 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(available_coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
         CCoinControl coin_control;
         coin_control.m_allow_other_inputs = true;
-        coin_control.Select(available_coins.All().at(0).outpoint);
+        COutput select_coin = available_coins.All().at(0);
+        coin_control.Select(select_coin.outpoint);
+        PreSelectedInputs selected_input;
+        selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs);
+        available_coins.coins[OutputType::LEGACY].erase(available_coins.coins[OutputType::LEGACY].begin());
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(0);
-        const auto result10 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+        const auto result10 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(result10);
     }
     {
@@ -374,7 +378,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         expected_result.Clear();
         add_coin(10 * CENT, 2, expected_result);
         CCoinControl coin_control;
-        const auto result11 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+        const auto result11 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result11));
         available_coins.Clear();
 
@@ -389,7 +393,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         expected_result.Clear();
         add_coin(9 * CENT, 2, expected_result);
         add_coin(1 * CENT, 2, expected_result);
-        const auto result12 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+        const auto result12 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb);
         // NOTE: Dash does not use BnB and therefore, this check will fail
         // BOOST_CHECK(EquivalentResult(expected_result, *result12));
         available_coins.Clear();
@@ -406,8 +410,12 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(9 * CENT, 2, expected_result);
         add_coin(1 * CENT, 2, expected_result);
         coin_control.m_allow_other_inputs = true;
-        coin_control.Select(available_coins.All().at(1).outpoint); // pre select 9 coin
-        const auto result13 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb);
+        COutput select_coin = available_coins.All().at(1); // pre select 9 coin
+        coin_control.Select(select_coin.outpoint);
+        PreSelectedInputs selected_input;
+        selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs);
+        available_coins.coins[OutputType::LEGACY].erase(++available_coins.coins[OutputType::LEGACY].begin());
+        const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result13));
     }
 }
@@ -795,7 +803,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
         cs_params.m_cost_of_change = 1;
         cs_params.min_viable_change = 1;
         CCoinControl cc;
-        const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params);
+        const auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, target, cc, cs_params);
         BOOST_CHECK(result);
         BOOST_CHECK_GE(result->GetSelectedValue(), target);
     }
@@ -977,7 +985,10 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test)
     cc.SetInputWeight(output.outpoint, 148);
     cc.SelectExternal(output.outpoint, output.txout);
 
-    const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params);
+    const auto preset_inputs = *Assert(FetchSelectedInputs(*wallet, cc, cs_params));
+    available_coins.coins[OutputType::LEGACY].erase(available_coins.coins[OutputType::LEGACY].begin());
+
+    const auto result = SelectCoins(*wallet, available_coins, preset_inputs, target, cc, cs_params);
     BOOST_CHECK(!result);
 }
 
@@ -1064,7 +1075,7 @@ static std::optional<SelectionResult> select_coins(const CAmount& target, const
     wallet->SetupDescriptorScriptPubKeyMans("", "");
 
     auto available_coins = coin_setup(*wallet);
-    auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params);
+    auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, target, cc, cs_params);
     if (result) {
         BOOST_CHECK_LE(static_cast<int>(cs_params.tx_noinputs_size) + GetSelectionWeight(*result), MAX_STANDARD_TX_SIZE);
         BOOST_CHECK_GE(result->GetSelectedValue(), target);
@@ -1178,7 +1189,7 @@ BOOST_AUTO_TEST_CASE(minimum_inputs_test)
         /*tx_noinputs_size=*/0,
         /*avoid_partial=*/false,
     };
-    auto result = SelectCoins(*wallet, available_coins, target, coin_control, coin_selection_params);
+    auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, target, coin_control, coin_selection_params);
     BOOST_REQUIRE(result);
 
     // Should consume only the first two coins (9 + 16) >= 25 and account correctly
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 7cd93f499aab..c87c84512293 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -517,7 +517,7 @@ def test_psbt_input_keys(psbt_input, keys):
         ext_utxo = self.nodes[0].listunspent(addresses=[addr])[0]
 
         # An external input without solving data should result in an error
-        assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15})
+        assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"], ext_utxo["vout"]), wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15})
 
         # But funding should work when the solving data is provided
         psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}})
diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py
index 73662a3e4e81..bfa6cd75256a 100755
--- a/test/functional/wallet_fundrawtransaction.py
+++ b/test/functional/wallet_fundrawtransaction.py
@@ -378,7 +378,9 @@ def test_two_vin_two_vout(self):
 
     def test_invalid_input(self):
         self.log.info("Test fundrawtxn with an invalid vin")
-        inputs  = [ {'txid' : "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1", 'vout' : 0} ] #invalid vin!
+        txid = "1c7f966dab21119bac53213a2bc7532bff1fa844c124fd750a7d0b1332440bd1"
+        vout = 0
+        inputs  = [ {'txid' : txid, 'vout' : vout} ] #invalid vin!
         outputs = { self.nodes[0].getnewaddress() : 10}
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
         assert_raises_rpc_error(-4, "Unable to find UTXO for external input", self.nodes[2].fundrawtransaction, rawtx)
@@ -959,7 +961,7 @@ def test_external_inputs(self):
 
         # An external input without solving data should result in an error
         raw_tx = wallet.createrawtransaction([ext_utxo], {self.nodes[0].getnewaddress(): ext_utxo["amount"] / 2})
-        assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx)
+        assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"], ext_utxo["vout"]), wallet.fundrawtransaction, raw_tx)
 
         # Error conditions
         assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}})
@@ -1042,6 +1044,8 @@ def test_add_inputs_default_value(self):
         #       Expect: only preset inputs are used.
         # 5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set):
         #       Expect: include inputs from the wallet.
+        # 6. Explicit add_inputs=false, no preset inputs:
+        #       Expect: failure as we did not provide inputs and the process cannot automatically select coins.
 
         # Case (1), 'send' command
         # 'add_inputs' value is true unless "inputs" are specified, in such case, add_inputs=false.
@@ -1093,6 +1097,10 @@ def test_add_inputs_default_value(self):
         tx = wallet.send(outputs=[{addr1: 8}], options=options)
         assert tx["complete"]
 
+        # 6. Explicit add_inputs=false, no preset inputs:
+        options = {"add_inputs": False}
+        assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{addr1: 3}], options=options)
+
         ################################################
 
         # Case (1), 'walletcreatefundedpsbt' command
@@ -1133,6 +1141,10 @@ def test_add_inputs_default_value(self):
         }
         assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, options=options)
 
+        # Case (6). Explicit add_inputs=false, no preset inputs:
+        options = {"add_inputs": False}
+        assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, inputs=[], outputs=outputs, options=options)
+
         self.nodes[2].unloadwallet("test_preset_inputs")
     def test_include_unsafe(self):
         self.log.info("Test fundrawtxn with unsafe inputs")
diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
index d7479267c26a..950b46449ae7 100755
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@@ -491,7 +491,7 @@ def run_test(self):
         ext_utxo = ext_fund.listunspent(addresses=[addr])[0]
 
         # An external input without solving data should result in an error
-        self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Insufficient funds"))
+        self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"], ext_utxo["vout"])))
 
         # But funding should work when the solving data is provided
         res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]})

From 79233dd1c4a91ae548a81f7bcfa35ce6833d863e Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Fri, 28 Aug 2026 01:50:13 +0700
Subject: [PATCH 08/15] fix: remove fRequireAllInputs and fix relevant code for
 bitcoin/bitcoin#25685

---
 src/wallet/coincontrol.h               |  2 --
 src/wallet/interfaces.cpp              | 21 +++++++++---
 src/wallet/spend.cpp                   |  1 +
 src/wallet/test/coinselector_tests.cpp | 46 --------------------------
 src/wallet/test/wallet_tests.cpp       |  3 ++
 5 files changed, 20 insertions(+), 53 deletions(-)

diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index 0d44adfa0ee4..c77a4bf687fe 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -37,8 +37,6 @@ class CCoinControl
     //! If true, the selection process can add extra unselected inputs from the wallet
     //! while requires all selected inputs be used
     bool m_allow_other_inputs = true;
-    //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with m_allow_other_inputs
-    bool fRequireAllInputs = true;
     //! Includes watch only addresses which are solvable
     bool fAllowWatchOnly = false;
     //! Override automatic min/max checks on fee, m_feerate must be set if true
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 60f10a77fe7f..2c31206bd824 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -468,20 +468,31 @@ class WalletImpl : public Wallet
 
         CCoinControl coin_control;
         coin_control.destChange = fund_destination;
-        coin_control.fRequireAllInputs = false;
+        // Spend only from fund_destination
+        coin_control.m_allow_other_inputs = false;
+        std::vector<COutput> address_coins;
         for (const auto& output : AvailableCoinsListUnspent(*m_wallet).All()) {
             CTxDestination destination;
             if (ExtractDestination(output.txout.scriptPubKey, destination) && destination == fund_destination) {
-                coin_control.Select(output.outpoint);
+                address_coins.push_back(output);
             }
         }
-        if (!coin_control.HasSelected()) {
+        if (address_coins.empty()) {
             return util::Error{
                 strprintf(Untranslated("No funds at specified address %s"), EncodeDestination(fund_destination))};
         }
 
-        auto result{CreateTransaction(*m_wallet, recipients, RANDOM_CHANGE_POSITION, coin_control,
-                                      /*sign=*/true, tx.vExtraPayload.size())};
+        // Every selected coin is force-spent, so select as few as possible: largest
+        // first, adding another one only while they don't cover amount plus fees
+        std::sort(address_coins.begin(), address_coins.end(),
+                  [](const COutput& a, const COutput& b) { return a.txout.nValue > b.txout.nValue; });
+        util::Result<CreatedTransactionResult> result{util::Error{}};
+        for (const COutput& coin : address_coins) {
+            coin_control.Select(coin.outpoint);
+            result = CreateTransaction(*m_wallet, recipients, RANDOM_CHANGE_POSITION, coin_control,
+                                       /*sign=*/true, tx.vExtraPayload.size());
+            if (result) break;
+        }
         if (!result) return util::Error{util::ErrorString(result)};
 
         tx.vin = result->tx->vin;
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index f0f8f3cb989f..97f076ab2cf3 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -1308,6 +1308,7 @@ bool GenBudgetSystemCollateralTx(CWallet& wallet, CTransactionRef& tx, uint256 h
     CCoinControl coinControl;
     if (!outpoint.IsNull()) {
         coinControl.Select(outpoint);
+        coinControl.m_allow_other_inputs = false;
     }
 
     auto res{CreateTransaction(wallet, vecSend, RANDOM_CHANGE_POSITION, coinControl)};
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index d47057244756..47f3e41658de 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -1151,51 +1151,5 @@ BOOST_AUTO_TEST_CASE(check_max_weight)
     }
 }
 
-/* --------------------------- Dash-specific tests start here --------------------------- */
-BOOST_AUTO_TEST_CASE(minimum_inputs_test)
-{
-    std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateMockWalletDatabase());
-    wallet->LoadWallet();
-    LOCK(wallet->cs_wallet);
-    wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-    wallet->SetupDescriptorScriptPubKeyMans("", "");
-
-    // Create coins (denominations) for a target that can be met without consuming all the coins
-    CoinsResult available_coins{};
-    CAmount target{25 * COIN};
-    add_coin(available_coins, *wallet,  9 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true);
-    add_coin(available_coins, *wallet, 16 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true);
-    add_coin(available_coins, *wallet, 24 * COIN, CFeeRate(0), /*nAge=*/6*24, /*fIsFromMe=*/false, /*nInput=*/0, /*spendable=*/true);
-
-    // Setup coin control to select from the given coins (!m_allow_other_inputs) *but* consume as little
-    // as possible (!fRequireAllInputs) and select our coins.
-    CCoinControl coin_control{};
-    coin_control.m_allow_other_inputs = false;
-    coin_control.fRequireAllInputs = false;
-    for (const auto& coin : available_coins.All()) {
-        coin_control.Select(coin.outpoint);
-    }
-
-    // Select coins
-    FastRandomContext rand{};
-    CoinSelectionParams coin_selection_params{
-        rand,
-        /*change_output_size=*/0,
-        /*change_spend_size=*/0,
-        /*min_change_target=*/CENT,
-        /*effective_feerate=*/CFeeRate(0),
-        /*long_term_feerate=*/CFeeRate(0),
-        /*discard_feerate=*/CFeeRate(0),
-        /*tx_noinputs_size=*/0,
-        /*avoid_partial=*/false,
-    };
-    auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, target, coin_control, coin_selection_params);
-    BOOST_REQUIRE(result);
-
-    // Should consume only the first two coins (9 + 16) >= 25 and account correctly
-    BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2);
-    BOOST_CHECK_EQUAL(result->GetSelectedValue(), 25 * COIN);
-}
-
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index b04aba66336d..00d7c3f5dd70 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -1304,6 +1304,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
     // First run the tests with only one input containing 100k duffs
     {
         coinControl = CCoinControl();
+        coinControl.m_allow_other_inputs = false;
         coinControl.Select(GetCoins({{100000, false}})[0]);
 
         // Start with fallback feerate
@@ -1344,6 +1345,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
     // Now use 4 different inputs with a total of 100k duff
     {
         coinControl = CCoinControl();
+        coinControl.m_allow_other_inputs = false;
         auto setCoins = GetCoins({{1000, false}, {5000, false}, {10000, false}, {84000, false}});
         for (auto coin : setCoins) {
             coinControl.Select(coin);
@@ -1388,6 +1390,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
     // Last use 10 equal inputs with a total of 100k duff
     {
         coinControl = CCoinControl();
+        coinControl.m_allow_other_inputs = false;
         auto setCoins = GetCoins({{10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false},
                                   {10000, false}, {10000, false}, {10000, false}, {10000, false}, {10000, false}});
 

From d8c6b9c05df61a109419ef12affbda794d226eec Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Thu, 27 Aug 2020 20:17:22 +0200
Subject: [PATCH 09/15] Merge #19169: rpc: Validate provided keys for
 query_options parameter in listunspent

a99a3c0bd6d5476503c015e23be569295fdd190c rpc: Validate provided keys for query_options parameter in listunspent (pasta)

Pull request description:

  At Dash, one of our developers was working with the `listunspent` RPC command, but instead of saying "minimumAmount" he said "minimmumAmount" as such the RPC wasn't working as expected.

  In https://github.com/dashpay/dash/pull/3507 we implemented a check so that `listunspent` returns an error if an unrecognized option is given. I figured I might as well adapt the code and throw up a PR here.

  Cheers!

ACKs for top commit:
  adaminsky:
    ACK `a99a3c0bd`
  meshcollider:
    Seems fine to me. utACK a99a3c0bd6d5476503c015e23be569295fdd190c

Tree-SHA512: 9fccf14979849879a51b352afa3e1932ce4a6cfc2ee97b8d405ec6e65673fe94e302795e3ec0b440e6d252f13acda620e1f6a0e86c3fa918883c3fb4600a372c

Co-authored-by: MarcoFalke <falke.marco@gmail.com>
---
 src/wallet/rpc/coins.cpp | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 6b8a355afb1a..4b171d52a072 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -652,20 +652,15 @@ RPCHelpMan listunspent()
     if (!request.params[4].isNull()) {
         const UniValue& options = request.params[4].get_obj();
 
-        // Note: Keep this vector up to date with the options processed below
-        const std::vector<std::string> vecOptions {
-            "minimumAmount",
-            "maximumAmount",
-            "minimumSumAmount",
-            "maximumCount",
-            "coinType"
-        };
-
-        for (const auto& key : options.getKeys()) {
-            if (std::find(vecOptions.begin(), vecOptions.end(), key) == vecOptions.end()) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid key used in query_options JSON object: ") + key);
-            }
-        }
+        RPCTypeCheckObj(options,
+            {
+                {"minimumAmount", UniValueType()},
+                {"maximumAmount", UniValueType()},
+                {"minimumSumAmount", UniValueType()},
+                {"maximumCount", UniValueType(UniValue::VNUM)},
+                {"coinType", UniValueType(UniValue::VNUM)},
+            },
+            true, true);
 
         if (options.exists("minimumAmount"))
             nMinimumAmount = AmountFromValue(options["minimumAmount"]);

From 912c3b35c42451230d0f0c9700115b62b4e4fb09 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Tue, 15 Nov 2022 19:40:25 -0500
Subject: [PATCH 10/15] Merge bitcoin/bitcoin#25730: RPC: listunspent, add
 "include immature coinbase" flag

fa84df1f033a5d1a8342ea941eca0b5ef73d78e7 scripted-diff: wallet: rename AvailableCoinsParams members to snake_case (furszy)
61c2265629fdf11a2cc266fad54ceb0a1247bb5e wallet: group AvailableCoins filtering parameters in a single struct (furszy)
f0f6a3577bef2e9ebd084fe35850e4e9580128a9 RPC: listunspent, add "include immature coinbase" flag (furszy)

Pull request description:

  Simple PR; adds a "include_immature_coinbase" flag to `listunspent` to include the immature coinbase UTXOs on the response.  Requested by #25728.

ACKs for top commit:
  danielabrozzoni:
    reACK fa84df1f033a5d1a8342ea941eca0b5ef73d78e7
  achow101:
    ACK fa84df1f033a5d1a8342ea941eca0b5ef73d78e7
  aureleoules:
    reACK fa84df1f033a5d1a8342ea941eca0b5ef73d78e7
  kouloumos:
    reACK fa84df1f033a5d1a8342ea941eca0b5ef73d78e7
  theStack:
    Code-review ACK fa84df1f033a5d1a8342ea941eca0b5ef73d78e7

Tree-SHA512: 0f3544cb8cfd0378a5c74594480f78e9e919c6cfb73a83e0f3112f8a0132a9147cf846f999eab522cea9ef5bd3ffd60690ea2ca367dde457b0554d7f38aec792

Co-authored-by: Andrew Chow <github@achow101.com>
---
 doc/release-notes-25730.md        |  6 +++++
 src/bench/wallet_create_tx.cpp    |  5 ++--
 src/wallet/rpc/coins.cpp          | 22 ++++++++++-------
 src/wallet/rpc/spend.cpp          |  4 +++-
 src/wallet/spend.cpp              | 39 ++++++++++---------------------
 src/wallet/spend.h                | 25 ++++++++++++++------
 test/functional/wallet_balance.py | 10 ++++++++
 7 files changed, 65 insertions(+), 46 deletions(-)
 create mode 100644 doc/release-notes-25730.md

diff --git a/doc/release-notes-25730.md b/doc/release-notes-25730.md
new file mode 100644
index 000000000000..33393cf31490
--- /dev/null
+++ b/doc/release-notes-25730.md
@@ -0,0 +1,6 @@
+RPC Wallet
+----------
+
+- RPC `listunspent` now has a new argument `include_immature_coinbase`
+  to include coinbase UTXOs that don't meet the minimum spendability
+  depth requirement (which before were silently skipped). (#25730)
\ No newline at end of file
diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
index 684e3e22bad4..698a0f55b6d8 100644
--- a/src/bench/wallet_create_tx.cpp
+++ b/src/bench/wallet_create_tx.cpp
@@ -112,9 +112,10 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
     CAmount target = 0;
     if (preset_inputs) {
         // Select inputs, each has 49 BTC
+        wallet::CoinFilterParams filter_coins;
+        filter_coins.max_count = preset_inputs->num_of_internal_inputs;
         const auto& res = WITH_LOCK(wallet.cs_wallet,
-                                    return wallet::AvailableCoins(wallet, nullptr, std::nullopt, 1, MAX_MONEY,
-                                                                  MAX_MONEY, preset_inputs->num_of_internal_inputs));
+                                    return wallet::AvailableCoins(wallet, /*coinControl=*/nullptr, /*feerate=*/std::nullopt, filter_coins));
         for (int i=0; i < preset_inputs->num_of_internal_inputs; i++) {
             const auto& coin{res.coins.at(output_type)[i]};
             target += coin.txout.nValue;
diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp
index 4b171d52a072..cfa7ea64f8fa 100644
--- a/src/wallet/rpc/coins.cpp
+++ b/src/wallet/rpc/coins.cpp
@@ -569,6 +569,7 @@ RPCHelpMan listunspent()
                             {"coinType", RPCArg::Type::NUM, RPCArg::Default{0}, "Filter coinTypes as follows:\n"
                                 "0=ALL_COINS, 1=ONLY_FULLY_MIXED, 2=ONLY_READY_TO_MIX, 3=ONLY_NONDENOMINATED,\n"
                                 "4=ONLY_MASTERNODE_COLLATERAL, 5=ONLY_COINJOIN_COLLATERAL" },
+                            {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase UTXOs"}
                         },
                         RPCArgOptions{.oneline_description="query_options"}},
                 },
@@ -643,10 +644,8 @@ RPCHelpMan listunspent()
         include_unsafe = request.params[3].get_bool();
     }
 
-    CAmount nMinimumAmount = 0;
-    CAmount nMaximumAmount = MAX_MONEY;
-    CAmount nMinimumSumAmount = MAX_MONEY;
-    uint64_t nMaximumCount = 0;
+    CoinFilterParams filter_coins;
+    filter_coins.min_amount = 0;
     CCoinControl coinControl(CoinType::ALL_COINS);
 
     if (!request.params[4].isNull()) {
@@ -659,20 +658,21 @@ RPCHelpMan listunspent()
                 {"minimumSumAmount", UniValueType()},
                 {"maximumCount", UniValueType(UniValue::VNUM)},
                 {"coinType", UniValueType(UniValue::VNUM)},
+                {"include_immature_coinbase", UniValueType(UniValue::VBOOL)}
             },
             true, true);
 
         if (options.exists("minimumAmount"))
-            nMinimumAmount = AmountFromValue(options["minimumAmount"]);
+            filter_coins.min_amount = AmountFromValue(options["minimumAmount"]);
 
         if (options.exists("maximumAmount"))
-            nMaximumAmount = AmountFromValue(options["maximumAmount"]);
+            filter_coins.max_amount = AmountFromValue(options["maximumAmount"]);
 
         if (options.exists("minimumSumAmount"))
-            nMinimumSumAmount = AmountFromValue(options["minimumSumAmount"]);
+            filter_coins.min_sum_amount = AmountFromValue(options["minimumSumAmount"]);
 
         if (options.exists("maximumCount"))
-            nMaximumCount = options["maximumCount"].getInt<int64_t>();
+            filter_coins.max_count = options["maximumCount"].getInt<int64_t>();
 
         if (options.exists("coinType")) {
             int64_t nCoinType = options["coinType"].getInt<int64_t>();
@@ -683,6 +683,10 @@ RPCHelpMan listunspent()
 
             coinControl.nCoinType = static_cast<CoinType>(nCoinType);
         }
+
+        if (options.exists("include_immature_coinbase")) {
+            filter_coins.include_immature_coinbase = options["include_immature_coinbase"].get_bool();
+        }
     }
 
     // Make sure the results are valid at least up to the most recent block
@@ -698,7 +702,7 @@ RPCHelpMan listunspent()
         coinControl.m_include_unsafe_inputs = include_unsafe;
 
         LOCK(pwallet->cs_wallet);
-        vecOutputs = AvailableCoinsListUnspent(*pwallet, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).All();
+        vecOutputs = AvailableCoinsListUnspent(*pwallet, &coinControl, filter_coins).All();
     }
 
     LOCK(pwallet->cs_wallet);
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index f7e9a57f3af4..636feb0a439c 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -1226,7 +1226,9 @@ RPCHelpMan sendall()
                     total_input_value += tx->tx->vout[input.prevout.n].nValue;
                 }
             } else {
-                for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).All()) {
+                CoinFilterParams coins_params;
+                coins_params.min_amount = 0;
+                for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, coins_params).All()) {
                     CHECK_NONFATAL(output.input_bytes > 0);
                     if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) {
                         continue;
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 97f076ab2cf3..3984989b9f5d 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -192,11 +192,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
 CoinsResult AvailableCoins(const CWallet& wallet,
                            const CCoinControl* coinControl,
                            std::optional<CFeeRate> feerate,
-                           const CAmount& nMinimumAmount,
-                           const CAmount& nMaximumAmount,
-                           const CAmount& nMinimumSumAmount,
-                           const uint64_t nMaximumCount,
-                           bool only_spendable)
+                           const CoinFilterParams& params)
 {
     AssertLockHeld(wallet.cs_wallet);
 
@@ -215,7 +211,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
         const uint256& wtxid = pwtx->GetHash();
         const CWalletTx& wtx = *pwtx;
 
-        if (wallet.IsTxImmatureCoinBase(wtx))
+        if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase)
             continue;
 
         int nDepth = wallet.GetTxDepthInMainChain(wtx);
@@ -274,7 +270,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             } // no default case, so the compiler can warn about missing cases
             if (!found) continue;
 
-            if (output.nValue < nMinimumAmount || output.nValue > nMaximumAmount)
+            if (output.nValue < params.min_amount || output.nValue > params.max_amount)
                 continue;
 
             // Skip manually selected coins (the caller can fetch them directly)
@@ -306,7 +302,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
 
             // Filter by spendable outputs only
-            if (!spendable && only_spendable) continue;
+            if (!spendable && params.only_spendable) continue;
 
             // If the Output is P2SH and spendable, we want to know if it is
             // a P2SH (legacy). We can determine this from the redeemScript.
@@ -338,14 +334,14 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             // Cache total amount as we go
             result.total_amount += output.nValue;
             // Checks the sum amount of all UTXO's.
-            if (nMinimumSumAmount != MAX_MONEY) {
-                if (result.total_amount >= nMinimumSumAmount) {
+            if (params.min_sum_amount != MAX_MONEY) {
+                if (result.total_amount >= params.min_sum_amount) {
                     return result;
                 }
             }
 
             // Checks the maximum number of UTXO's.
-            if (nMaximumCount > 0 && result.Size() >= nMaximumCount) {
+            if (params.max_count > 0 && result.Size() >= params.max_count) {
                 return result;
             }
         }
@@ -354,21 +350,16 @@ CoinsResult AvailableCoins(const CWallet& wallet,
     return result;
 }
 
-CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount)
+CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl, CoinFilterParams params)
 {
-    return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, /*only_spendable=*/false);
+    params.only_spendable = false;
+    return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, params);
 }
 
 CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl)
 {
     LOCK(wallet.cs_wallet);
-    return AvailableCoins(wallet, coinControl,
-            /*feerate=*/ std::nullopt,
-            /*nMinimumAmount=*/ 1,
-            /*nMaximumAmount=*/ MAX_MONEY,
-            /*nMinimumSumAmount=*/ MAX_MONEY,
-            /*nMaximumCount=*/ 0
-    ).total_amount;
+    return AvailableCoins(wallet, coinControl).total_amount;
 }
 
 const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output)
@@ -972,13 +963,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
     // allowed (coins automatically selected by the wallet)
     CoinsResult available_coins;
     if (coin_control.m_allow_other_inputs) {
-        available_coins = AvailableCoins(wallet,
-                                         &coin_control,
-                                         coin_selection_params.m_effective_feerate,
-                                         1,            /*nMinimumAmount*/
-                                         MAX_MONEY,    /*nMaximumAmount*/
-                                         MAX_MONEY,    /*nMinimumSumAmount*/
-                                         0);           /*nMaximumCount*/
+        available_coins = AvailableCoins(wallet, &coin_control, coin_selection_params.m_effective_feerate);
     }
 
     // Choose coins to use
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 6e8550ca65b0..6462aeae6db8 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -54,23 +54,34 @@ struct CoinsResult {
     CAmount total_amount{0};
 };
 
+struct CoinFilterParams {
+    // Outputs below the minimum amount will not get selected
+    CAmount min_amount{1};
+    // Outputs above the maximum amount will not get selected
+    CAmount max_amount{MAX_MONEY};
+    // Return outputs until the minimum sum amount is covered
+    CAmount min_sum_amount{MAX_MONEY};
+    // Maximum number of outputs that can be returned
+    uint64_t max_count{0};
+    // By default, return only spendable outputs
+    bool only_spendable{true};
+    // By default, do not include immature coinbase outputs
+    bool include_immature_coinbase{false};
+};
+
 /**
  * Populate the CoinsResult struct with vectors of available COutputs, organized by OutputType.
  */
 CoinsResult AvailableCoins(const CWallet& wallet,
                            const CCoinControl* coinControl = nullptr,
                            std::optional<CFeeRate> feerate = std::nullopt,
-                           const CAmount& nMinimumAmount = 1,
-                           const CAmount& nMaximumAmount = MAX_MONEY,
-                           const CAmount& nMinimumSumAmount = MAX_MONEY,
-                           const uint64_t nMaximumCount = 0,
-                           bool only_spendable = true) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+                           const CoinFilterParams& params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
 /**
- * Wrapper function for AvailableCoins which skips the `feerate` parameter. Use this function
+ * Wrapper function for AvailableCoins which skips the `feerate` and `CoinFilterParams::only_spendable` parameters. Use this function
  * to list all available coins (e.g. listunspent RPC) while not intending to fund a transaction.
  */
-CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl = nullptr, CoinFilterParams params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
 CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl = nullptr);
 
diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py
index 5454a43406c9..a500d696388d 100755
--- a/test/functional/wallet_balance.py
+++ b/test/functional/wallet_balance.py
@@ -81,8 +81,18 @@ def run_test(self):
         self.log.info("Mining blocks ...")
         self.generate(self.nodes[0], 1)
         self.generate(self.nodes[1], 1)
+
+        # Verify listunspent returns immature coinbase if 'include_immature_coinbase' is set
+        assert_equal(len(self.nodes[0].listunspent(query_options={'include_immature_coinbase': True})), 1)
+        assert_equal(len(self.nodes[0].listunspent(query_options={'include_immature_coinbase': False})), 0)
+
         self.generatetoaddress(self.nodes[1], COINBASE_MATURITY + 1, ADDRESS_WATCHONLY)
 
+        # Verify listunspent returns all immature coinbases if 'include_immature_coinbase' is set
+        # For now, only the legacy wallet will see the coinbases going to the imported 'ADDRESS_WATCHONLY'
+        assert_equal(len(self.nodes[0].listunspent(query_options={'include_immature_coinbase': False})), 1 if self.options.descriptors else 2)
+        assert_equal(len(self.nodes[0].listunspent(query_options={'include_immature_coinbase': True})), 1 if self.options.descriptors else COINBASE_MATURITY + 2)
+
         if not self.options.descriptors:
             # Tests legacy watchonly behavior which is not present (and does not need to be tested) in descriptor wallets
             assert_equal(self.nodes[0].getbalances()['mine']['trusted'], 500)

From 5c3a1d2042bed44997af38ca84c946d48f9f3a8c Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Tue, 29 Nov 2022 18:38:40 -0500
Subject: [PATCH 11/15] Merge bitcoin/bitcoin#26532: wallet: bugfix, invalid
 crypted key "checksum_valid" set

13d97608297bd56ed033d0e754d2e50447b02af0 test: load wallet, coverage for crypted keys (furszy)
373c99633ec7f20557db2734c49116ee4ad15423 refactor: move DuplicateMockDatabase to wallet/test/util.h (furszy)
ee7a984f85015b610be4929b7c35cb501c1fbf7c refactor: unify test/util/wallet.h with wallet/test/util.h (furszy)
cc5a5e81217506ec6f9fff34056290f8f40a7396 wallet: bugfix, invalid crypted key "checksum_valid" set (furszy)

Pull request description:

  At wallet load time, the crypted key "checksum_valid" variable is always set to false. Which, on every wallet decryption call, forces the process to re-write all the ckeys to db when it's not needed.

  Note:
  The first commit fixes the issue, the two commits in the middle are cleanups so `DuplicateMockDatabase`
  can be used without duplicating code. And, the last one is pure test coverage for the crypted keys loading
  process.

  Includes test coverage for the following scenarios:

  1) "All ckeys checksums valid" test:
  Loads an encrypted wallet with all the crypted keys with a valid checksum and
  verifies that 'CWallet::Unlock' doesn't force an entire crypted keys re-write.

      (we force a complete ckeys re-write if we find any missing crypted key checksum
  during the wallet loading process)

  2) "Missing checksum in one ckey" test:
  Verifies that loading up a wallet with, at least one, 'ckey' with no checksum
  triggers a complete re-write of the crypted keys.

  3) "Invalid ckey checksum error" test:
  Verifies that loading up a ckey with an invalid checksum stops the wallet loading
  process with a corruption error.

  4) "Invalid ckey pubkey error" test:
  Verifies that loading up a ckey with an invalid pubkey stops the wallet loading
  process with a corruption error.

ACKs for top commit:
  achow101:
    ACK 13d97608297bd56ed033d0e754d2e50447b02af0
  aureleoules:
    ACK 13d97608297bd56ed033d0e754d2e50447b02af0

Tree-SHA512: 9ea630ee4a355282fbeee61ca04737294382577bb4b2631f50e732568fdab8f72491930807fbda58206446c4f26200cdc34d8afa14dbe1241aec713887d06a0b

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/Makefile.test.include            |   2 -
 src/Makefile.test_util.include       |  15 ++-
 src/bench/block_assemble.cpp         |   1 -
 src/bench/wallet_balance.cpp         |   4 +-
 src/bench/wallet_create_tx.cpp       |   2 +-
 src/bench/wallet_loading.cpp         |  26 +-----
 src/test/util/wallet.cpp             |  49 ----------
 src/test/util/wallet.h               |  30 ------
 src/wallet/test/util.cpp             |  45 +++++++--
 src/wallet/test/util.h               |  13 +++
 src/wallet/test/walletload_tests.cpp | 135 +++++++++++++++++++++++++++
 11 files changed, 203 insertions(+), 119 deletions(-)
 delete mode 100644 src/test/util/wallet.cpp
 delete mode 100644 src/test/util/wallet.h

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index a34ea4b120a3..63635c7daa66 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -259,8 +259,6 @@ FUZZ_WALLET_SRC += \
 endif # USE_SQLITE
 
 BITCOIN_TEST_SUITE += \
-  wallet/test/util.cpp \
-  wallet/test/util.h \
   wallet/test/wallet_test_fixture.cpp \
   wallet/test/wallet_test_fixture.h \
   wallet/test/init_test_fixture.cpp \
diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include
index 0f18badb0d0c..b6441a1e3e15 100644
--- a/src/Makefile.test_util.include
+++ b/src/Makefile.test_util.include
@@ -25,10 +25,13 @@ TEST_UTIL_H = \
   test/util/str.h \
   test/util/transaction_utils.h \
   test/util/txmempool.h \
-  test/util/wallet.h \
   test/util/validation.h \
   test/util/xoroshiro128plusplus.h
 
+if ENABLE_WALLET
+TEST_UTIL_H += wallet/test/util.h
+endif # ENABLE_WALLET
+
 libtest_util_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS)
 libtest_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 libtest_util_a_SOURCES = \
@@ -45,6 +48,10 @@ libtest_util_a_SOURCES = \
   test/util/str.cpp \
   test/util/transaction_utils.cpp \
   test/util/txmempool.cpp \
-  test/util/validation.cpp \
-  test/util/wallet.cpp \
-  $(TEST_UTIL_H)
+  test/util/validation.cpp
+
+if ENABLE_WALLET
+libtest_util_a_SOURCES += wallet/test/util.cpp
+endif # ENABLE_WALLET
+
+libtest_util_a_SOURCES += $(TEST_UTIL_H)
diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp
index d5ae9480edd5..07c422f7ef67 100644
--- a/src/bench/block_assemble.cpp
+++ b/src/bench/block_assemble.cpp
@@ -10,7 +10,6 @@
 #include <test/util/mining.h>
 #include <test/util/script.h>
 #include <test/util/setup_common.h>
-#include <test/util/wallet.h>
 #include <txmempool.h>
 #include <validation.h>
 
diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp
index 36a95f5765a0..3f6a73b8f9f4 100644
--- a/src/bench/wallet_balance.cpp
+++ b/src/bench/wallet_balance.cpp
@@ -7,7 +7,7 @@
 #include <node/context.h>
 #include <test/util/mining.h>
 #include <test/util/setup_common.h>
-#include <test/util/wallet.h>
+#include <wallet/test/util.h>
 #include <validationinterface.h>
 #include <wallet/receive.h>
 #include <wallet/wallet.h>
@@ -19,6 +19,8 @@ using wallet::CWallet;
 using wallet::DBErrors;
 using wallet::WALLET_FLAG_DESCRIPTORS;
 
+const std::string ADDRESS_B58T_UNSPENDABLE = "yXXXXXXXXXXXXXXXXXXXXXXXXXXXVd2rXU";
+
 static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine, const uint32_t epoch_iters)
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
index 698a0f55b6d8..06fa42c9f1e7 100644
--- a/src/bench/wallet_create_tx.cpp
+++ b/src/bench/wallet_create_tx.cpp
@@ -9,9 +9,9 @@
 #include <kernel/chain.h>
 #include <node/context.h>
 #include <test/util/setup_common.h>
-#include <test/util/wallet.h>
 #include <validation.h>
 #include <wallet/spend.h>
+#include <wallet/test/util.h>
 #include <wallet/wallet.h>
 
 using wallet::CWallet;
diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp
index 37f2c90ea281..91a54d9ee850 100644
--- a/src/bench/wallet_loading.cpp
+++ b/src/bench/wallet_loading.cpp
@@ -7,7 +7,7 @@
 #include <node/context.h>
 #include <test/util/mining.h>
 #include <test/util/setup_common.h>
-#include <test/util/wallet.h>
+#include <wallet/test/util.h>
 #include <util/translation.h>
 #include <validationinterface.h>
 #include <wallet/context.h>
@@ -52,30 +52,6 @@ static void AddTx(CWallet& wallet)
     wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{});
 }
 
-static std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options)
-{
-    auto new_database = CreateMockWalletDatabase(options);
-
-    // Get a cursor to the original database
-    auto batch = database.MakeBatch();
-    batch->StartCursor();
-
-    // Get a batch for the new database
-    auto new_batch = new_database->MakeBatch();
-
-    // Read all records from the original database and write them to the new one
-    while (true) {
-        CDataStream key(SER_DISK, CLIENT_VERSION);
-        CDataStream value(SER_DISK, CLIENT_VERSION);
-        bool complete;
-        batch->ReadAtCursor(key, value, complete);
-        if (complete) break;
-        new_batch->Write(key, value);
-    }
-
-    return new_database;
-}
-
 static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
 {
     const auto test_setup = MakeNoLogFileContext<TestingSetup>();
diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp
deleted file mode 100644
index d610fe314428..000000000000
--- a/src/test/util/wallet.cpp
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright (c) 2019-2021 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#if defined(HAVE_CONFIG_H)
-#include <config/bitcoin-config.h>
-#endif
-
-#include <test/util/wallet.h>
-
-#include <key_io.h>
-#include <outputtype.h>
-#include <script/standard.h>
-#ifdef ENABLE_WALLET
-#include <util/check.h>
-#include <util/translation.h>
-#include <wallet/wallet.h>
-#endif
-
-using wallet::CWallet;
-
-const std::string ADDRESS_B58T_UNSPENDABLE = "yXXXXXXXXXXXXXXXXXXXXXXXXXXXVd2rXU";
-const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj";
-
-#ifdef ENABLE_WALLET
-std::string getnewaddress(CWallet& w)
-{
-    return EncodeDestination(getNewDestination(w));
-}
-
-CTxDestination getNewDestination(CWallet& w)
-{
-    return *Assert(w.GetNewDestination(""));
-}
-
-// void importaddress(CWallet& wallet, const std::string& address)
-// {
-//     auto spk_man = wallet.GetLegacyScriptPubKeyMan();
-//     assert(spk_man != nullptr);
-//     LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore);
-//     const auto dest = DecodeDestination(address);
-//     assert(IsValidDestination(dest));
-//     const auto script = GetScriptForDestination(dest);
-//     wallet.MarkDirty();
-//     assert(!spk_man->HaveWatchOnly(script));
-//     if (!spk_man->AddWatchOnly(script, 0 /* nCreateTime */)) assert(false);
-//     wallet.SetAddressBook(dest, /*label=*/"", "receive");
-// }
-#endif // ENABLE_WALLET
diff --git a/src/test/util/wallet.h b/src/test/util/wallet.h
deleted file mode 100644
index a95315c08087..000000000000
--- a/src/test/util/wallet.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2019 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_TEST_UTIL_WALLET_H
-#define BITCOIN_TEST_UTIL_WALLET_H
-
-#include <outputtype.h>
-#include <string>
-
-namespace wallet {
-class CWallet;
-} // namespace wallet
-
-// Constants //
-
-extern const std::string ADDRESS_B58T_UNSPENDABLE;
-extern const std::string ADDRESS_BCRT1_UNSPENDABLE;
-
-// RPC-like //
-
-/** Import the address to the wallet */
-void importaddress(wallet::CWallet& wallet, const std::string& address);
-/** Returns a new encoded destination from the wallet */
-std::string getnewaddress(wallet::CWallet& w);
-/** Returns a new destination, of an specific type, from the wallet */
-CTxDestination getNewDestination(wallet::CWallet& w);
-
-
-#endif // BITCOIN_TEST_UTIL_WALLET_H
diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp
index 915304828a13..01737f277f41 100644
--- a/src/wallet/test/util.cpp
+++ b/src/wallet/test/util.cpp
@@ -13,8 +13,6 @@
 #include <wallet/wallet.h>
 #include <wallet/walletdb.h>
 
-#include <boost/test/unit_test.hpp>
-
 #include <memory>
 
 namespace wallet {
@@ -51,10 +49,45 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, interfaces
     WalletRescanReserver reserver(*wallet);
     reserver.reserve();
     CWallet::ScanResult result = wallet->ScanForWalletTransactions(chain_info.genesis_hash, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false);
-    BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS);
-    BOOST_CHECK_EQUAL(result.last_scanned_block, chain_info.tip_hash);
-    BOOST_CHECK_EQUAL(*result.last_scanned_height, chain_info.height);
-    BOOST_CHECK(result.last_failed_block.IsNull());
+    assert(result.status == CWallet::ScanResult::SUCCESS);
+    assert(result.last_scanned_block == chain_info.tip_hash);
+    assert(*result.last_scanned_height == chain_info.height);
+    assert(result.last_failed_block.IsNull());
     return wallet;
 }
+
+std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options)
+{
+    auto new_database = CreateMockWalletDatabase(options);
+
+    // Get a cursor to the original database
+    auto batch = database.MakeBatch();
+    batch->StartCursor();
+
+    // Get a batch for the new database
+    auto new_batch = new_database->MakeBatch();
+
+    // Read all records from the original database and write them to the new one
+    while (true) {
+        CDataStream key(SER_DISK, CLIENT_VERSION);
+        CDataStream value(SER_DISK, CLIENT_VERSION);
+        bool complete;
+        batch->ReadAtCursor(key, value, complete);
+        if (complete) break;
+        new_batch->Write(key, value);
+    }
+
+    return new_database;
+}
+
+std::string getnewaddress(CWallet& w)
+{
+    return EncodeDestination(getNewDestination(w));
+}
+
+CTxDestination getNewDestination(CWallet& w)
+{
+    return *Assert(w.GetNewDestination(""));
+}
+
 } // namespace wallet
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index 39df9a4eb9b8..c5af7bf8755a 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -5,11 +5,13 @@
 #ifndef BITCOIN_WALLET_TEST_UTIL_H
 #define BITCOIN_WALLET_TEST_UTIL_H
 
+#include <script/standard.h>
 #include <memory>
 
 class ArgsManager;
 class ChainstateManager;
 class CKey;
+enum class OutputType;
 namespace interfaces {
 class Chain;
 namespace CoinJoin {
@@ -19,8 +21,19 @@ class Loader;
 
 namespace wallet {
 class CWallet;
+struct DatabaseOptions;
+class WalletDatabase;
 
 std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, ChainstateManager& chainman, ArgsManager& args, const CKey& key);
+
+// Creates a copy of the provided database
+std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options);
+
+/** Returns a new encoded destination from the wallet */
+std::string getnewaddress(wallet::CWallet& w);
+/** Returns a new destination, of an specific type, from the wallet */
+CTxDestination getNewDestination(wallet::CWallet& w);
+
 } // namespace wallet
 
 #endif // BITCOIN_WALLET_TEST_UTIL_H
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index c77768a20564..1951e63bb2fc 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
+#include <wallet/test/util.h>
 #include <wallet/wallet.h>
 #include <test/util/setup_common.h>
 
@@ -50,5 +51,139 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup)
     }
 }
 
+bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key)
+{
+    std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(false);
+    BOOST_CHECK(batch->StartCursor());
+    while (true) {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        CDataStream ssValue(SER_DISK, CLIENT_VERSION);
+        bool complete;
+        BOOST_CHECK(batch->ReadAtCursor(ssKey, ssValue, complete));
+        if (complete) break;
+        std::string type;
+        ssKey >> type;
+        if (type == key) return true;
+    }
+    return false;
+}
+
+BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup)
+{
+    // The test duplicates the db so each case has its own db instance.
+    int NUMBER_OF_TESTS = 4;
+    std::vector<std::unique_ptr<WalletDatabase>> dbs;
+    CKey first_key;
+    auto get_db = [](std::vector<std::unique_ptr<WalletDatabase>>& dbs) {
+        std::unique_ptr<WalletDatabase> db = std::move(dbs.back());
+        dbs.pop_back();
+        return db;
+    };
+
+    {   // Context setup.
+        // Create and encrypt legacy wallet
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateMockWalletDatabase()));
+        LOCK(wallet->cs_wallet);
+        auto legacy_spkm = wallet->GetOrCreateLegacyScriptPubKeyMan();
+        BOOST_CHECK(legacy_spkm->CanGenerateKeys() && legacy_spkm->TopUp());
+
+        // Get the first key in the wallet
+        CTxDestination dest = *Assert(legacy_spkm->GetNewDestination());
+        CKeyID key_id = GetKeyForDestination(*legacy_spkm, dest);
+        BOOST_CHECK(legacy_spkm->GetKey(key_id, first_key));
+
+        // Encrypt the wallet and duplicate database
+        BOOST_CHECK(wallet->EncryptWallet("encrypt"));
+        wallet->Flush();
+
+        DatabaseOptions options;
+        for (int i=0; i < NUMBER_OF_TESTS; i++) {
+            dbs.emplace_back(DuplicateMockDatabase(wallet->GetDatabase(), options));
+        }
+    }
+
+    {
+        // First test case:
+        // Erase all the crypted keys from db and unlock the wallet.
+        // The wallet will only re-write the crypted keys to db if any checksum is missing at load time.
+        // So, if any 'ckey' record re-appears on db, then the checksums were not properly calculated, and we are re-writing
+        // the records every time that 'CWallet::Unlock' gets called, which is not good.
+
+        // Load the wallet and check that is encrypted
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, get_db(dbs)));
+        BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK);
+        BOOST_CHECK(wallet->IsCrypted());
+        BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+
+        // Now delete all records and check that the 'Unlock' function doesn't re-write them
+        BOOST_CHECK(wallet->GetLegacyScriptPubKeyMan()->DeleteRecords());
+        BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+        BOOST_CHECK(wallet->Unlock("encrypt"));
+        BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+    }
+
+    {
+        // Second test case:
+        // Verify that loading up a 'ckey' with no checksum triggers a complete re-write of the crypted keys.
+        std::unique_ptr<WalletDatabase> db = get_db(dbs);
+        {
+            std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false);
+            std::pair<std::vector<unsigned char>, uint256> value;
+            BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), value));
+
+            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey());
+            BOOST_CHECK(batch->Write(key, value.first, /*fOverwrite=*/true));
+        }
+
+        // Load the wallet and check that is encrypted
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, std::move(db)));
+        BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK);
+        BOOST_CHECK(wallet->IsCrypted());
+        BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+
+        // Now delete all ckey records and check that the 'Unlock' function re-writes them
+        // (this is because the wallet, at load time, found a ckey record with no checksum)
+        BOOST_CHECK(wallet->GetLegacyScriptPubKeyMan()->DeleteRecords());
+        BOOST_CHECK(!HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+        BOOST_CHECK(wallet->Unlock("encrypt"));
+        BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY));
+    }
+
+    {
+        // Third test case:
+        // Verify that loading up a 'ckey' with an invalid checksum throws an error.
+        std::unique_ptr<WalletDatabase> db = get_db(dbs);
+        {
+            std::unique_ptr<DatabaseBatch> batch = db->MakeBatch(false);
+            std::vector<unsigned char> crypted_data;
+            BOOST_CHECK(batch->Read(std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey()), crypted_data));
+
+            // Write an invalid checksum
+            std::pair<std::vector<unsigned char>, uint256> value = std::make_pair(crypted_data, uint256::ONE);
+            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, first_key.GetPubKey());
+            BOOST_CHECK(batch->Write(key, value, /*fOverwrite=*/true));
+        }
+
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, std::move(db)));
+        BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT);
+    }
+
+    {
+        // Fourth test case:
+        // Verify that loading up a 'ckey' with an invalid pubkey throws an error
+        std::unique_ptr<WalletDatabase> db = get_db(dbs);
+        {
+            CPubKey invalid_key;
+            BOOST_ASSERT(!invalid_key.IsValid());
+            const auto key = std::make_pair(DBKeys::CRYPTED_KEY, invalid_key);
+            std::pair<std::vector<unsigned char>, uint256> value;
+            BOOST_CHECK(db->MakeBatch(false)->Write(key, value, /*fOverwrite=*/true));
+        }
+
+        std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, std::move(db)));
+        BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT);
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

From 7cb417df76311011223704c598b130ea30ae6aa1 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 30 Nov 2022 11:10:29 -0500
Subject: [PATCH 12/15] Merge bitcoin/bitcoin#25942: test: add `ismine` test
 for descriptor ScriptPubKeyMan

1b77db265317a6470d0914b520f04eb64b3c0942 test: add `ismine` test for descriptor scriptpubkeyman (w0xlt)

Pull request description:

  Currently `src/wallet/test/ismine_tests.cpp` has tests for the legacy ScriptPubKeyMan only.
  This PR adds tests for the descriptor ScriptPubKeyMan.

ACKs for top commit:
  ishaanam:
    ACK 1b77db265317a6470d0914b520f04eb64b3c0942
  achow101:
    ACK 1b77db265317a6470d0914b520f04eb64b3c0942
  furszy:
    ACK 1b77db26 with a non-blocking comment.

Tree-SHA512: 977b5d1e71f9468331aeb4ebaf3708dd651f9f3018d4544a395b87ca6d7fb8bfa6d20acc1a4f6e096e240e81d30fb7a6e8add190e52536e7a3cb5a80f392883f

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/wallet/test/ismine_tests.cpp | 158 +++++++++++++++++++++++++++++--
 1 file changed, 150 insertions(+), 8 deletions(-)

diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp
index a9a54b575aea..1253c81dfb6b 100644
--- a/src/wallet/test/ismine_tests.cpp
+++ b/src/wallet/test/ismine_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <key.h>
+#include <key_io.h>
 #include <script/script.h>
 #include <script/script_error.h>
 #include <script/standard.h>
@@ -17,6 +18,25 @@ using node::NodeContext;
 namespace wallet {
 BOOST_FIXTURE_TEST_SUITE(ismine_tests, BasicTestingSetup)
 
+wallet::ScriptPubKeyMan* CreateDescriptor(CWallet& keystore, const std::string& desc_str, const bool success)
+{
+    keystore.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+
+    FlatSigningProvider keys;
+    std::string error;
+    std::unique_ptr<Descriptor> parsed_desc = Parse(desc_str, keys, error, false);
+    BOOST_CHECK(success == (parsed_desc != nullptr));
+    if (!success) return nullptr;
+
+    const int64_t range_start = 0, range_end = 1, next_index = 0, timestamp = 1;
+
+    WalletDescriptor w_desc(std::move(parsed_desc), timestamp, range_start, range_end, next_index);
+
+    LOCK(keystore.cs_wallet);
+
+    return Assert(keystore.AddWalletDescriptor(w_desc, keys,/*label=*/"", /*internal=*/false));
+};
+
 BOOST_AUTO_TEST_CASE(ismine_standard)
 {
     CKey keys[2];
@@ -34,7 +54,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
     CScript scriptPubKey;
     isminetype result;
 
-    // P2PK compressed
+    // P2PK compressed - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -53,7 +73,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
-    // P2PK uncompressed
+    // P2PK compressed - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "pk(" + EncodeSecret(keys[0]) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // P2PK uncompressed - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -72,7 +104,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
-    // P2PKH compressed
+    // P2PK uncompressed - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "pk(" + EncodeSecret(uncompressedKey) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // P2PKH compressed - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -91,7 +135,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
-    // P2PKH uncompressed
+    // P2PKH compressed - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "pkh(" + EncodeSecret(keys[0]) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0]));
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // P2PKH uncompressed - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -110,7 +166,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
-    // P2SH
+    // P2PKH uncompressed - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "pkh(" + EncodeSecret(uncompressedKey) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey));
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // P2SH - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -137,7 +205,20 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
-    //  (P2PKH inside) P2SH inside P2SH (invalid)
+    // P2SH - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "sh(pkh(" + EncodeSecret(keys[0]) + "))";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        CScript redeemScript = GetScriptForDestination(PKHash(pubkeys[0]));
+        scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript));
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // (P2PKH inside) P2SH inside P2SH (invalid) - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -156,7 +237,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0);
     }
 
-    // scriptPubKey multisig
+    // (P2PKH inside) P2SH inside P2SH (invalid) - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "sh(sh(" + EncodeSecret(keys[0]) + "))";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, false);
+        BOOST_CHECK_EQUAL(spk_manager, nullptr);
+    }
+
+    // P2WPKH uncompressed (invalid) - Descriptor
+    // Not applicable for Dash Core because there is no segwit
+
+    // scriptPubKey multisig - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -191,7 +284,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0);
     }
 
-    // P2SH multisig
+    // scriptPubKey multisig - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+        std::string desc_str = "multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // P2SH multisig - Legacy
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
         keystore.SetupLegacyScriptPubKeyMan();
@@ -214,6 +319,43 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
         BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1);
     }
 
+    // P2SH multisig - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+
+        std::string desc_str = "sh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        CScript redeemScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
+        scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript));
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
+    // Combo - Descriptor
+    {
+        CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());
+
+        std::string desc_str = "combo(" + EncodeSecret(keys[0]) + ")";
+
+        auto spk_manager = CreateDescriptor(keystore, desc_str, true);
+
+        // Test P2PK
+        result = spk_manager->IsMine(GetScriptForRawPubKey(pubkeys[0]));
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+
+        // Test P2PKH
+        result = spk_manager->IsMine(GetScriptForDestination(PKHash(pubkeys[0])));
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+
+        // Test P2SH-P2PKH (Dash's combo descriptor describes it in place of upstream's P2SH-P2WPKH)
+        CScript redeemScript = GetScriptForDestination(PKHash(pubkeys[0]));
+        scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript));
+        result = spk_manager->IsMine(scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+    }
+
     // OP_RETURN
     {
         CWallet keystore(chain.get(), /*coinjoin_loader=*/nullptr, "", m_args, CreateDummyWalletDatabase());

From 01bb9fdc02166a67adfc20f7082278caeeed91bc Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Mon, 5 Dec 2022 17:16:26 -0500
Subject: [PATCH 13/15] Merge bitcoin/bitcoin#26462: wallet: fix crash on
 loading descriptor wallet containing legacy key type entries
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

3198e4239e848bbb119e3638677aa9bcf8353ca6 test: check that loading descriptor wallet with legacy entries throws error (Sebastian Falbesoner)
349ed2a0eed3aaaf199ead93057c97730869c3a3 wallet: throw error if legacy entries are present on loading descriptor wallets (Sebastian Falbesoner)

Pull request description:

  Loading a descriptor wallet currently leads to a segfault if a legacy key type entry is present that can be deserialized successfully and needs SPKman-interaction. To reproduce with a "cscript" entry (see second commit for details):

  ```
  $ ./src/bitcoin-cli createwallet crashme
  $ ./src/bitcoin-cli unloadwallet crashme
  $ sqlite3 ~/.bitcoin/wallets/crashme/wallet.dat
  SQLite version 3.38.2 2022-03-26 13:51:10
  Enter ".help" for usage hints.
  sqlite> INSERT INTO main VALUES(x'07637363726970740000000000000000000000000000000000000000', x'00');
  $ ./src/bitcoin-cli loadwallet crashme

  --- bitcoind output: ---
  2022-11-06T13:51:01Z Using SQLite Version 3.38.2
  2022-11-06T13:51:01Z Using wallet /home/honey/.bitcoin/wallets/crashme
  2022-11-06T13:51:01Z init message: Loading wallet…
  2022-11-06T13:51:01Z [crashme] Wallet file version = 10500, last client version = 249900

  Segmentation fault (core dumped)
  ```

  Background: In the wallet key-value-loading routine, most legacy type entries require a `LegacyScriptPubKeyMan` instance after successful deserialization. On a descriptor wallet, creating that (via method `GetOrCreateLegacyScriptPubKeyMan`) fails and then leads to a null-pointer dereference crash. E.g. for CSCRIPT: https://github.com/bitcoin/bitcoin/blob/50422b770a40f5fa964201d1e99fd6b5dc1653ca/src/wallet/walletdb.cpp#L589-L594

  ~~This PR fixes this by simply ignoring legacy entries if the wallet flags indicate that we have a descriptor wallet. The second commits adds a regression test to the descriptor wallet's functional test (fortunately Python includes sqlite3 support in the standard library).~~

  ~~Probably it would be even better to throw a warning to the user if unexpected legacy entries are found in descriptor wallets, but I think as a first mitigation everything is obvisouly better than crashing. As far as I'm aware, descriptor wallets created/migrated by Bitcoin Core should never end up in a state containing legacy type entries though.~~

  This PR fixes this by throwing an error if legacy entries are found in descriptor wallets on loading.

ACKs for top commit:
  achow101:
    ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6
  aureleoules:
    ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6

Tree-SHA512: ee43da3f61248e0fde55d9a705869202cb83df678ebf4816f0e77263f0beac0d7bae9490465d1753159efb093ee37182931d76b2e2b6e8c6f8761285700ace1c

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/wallet/wallet.cpp                |  4 ++++
 src/wallet/walletdb.cpp              | 13 +++++++++++++
 src/wallet/walletdb.h                |  3 ++-
 test/functional/wallet_descriptor.py | 12 ++++++++++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index cbe4878932a4..30966ec0c462 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3199,6 +3199,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
                                 "The wallet might had been created on a newer version.\n"
                                 "Please try running the latest software version.\n"), walletFile);
             return nullptr;
+        } else if (nLoadWalletRet == DBErrors::UNEXPECTED_LEGACY_ENTRY) {
+            error = strprintf(_("Unexpected legacy entry in descriptor wallet found. Loading wallet %s\n\n"
+                                "The wallet might have been tampered with or created with malicious intent.\n"), walletFile);
+            return nullptr;
         } else {
             error = strprintf(_("Error loading %s"), walletFile);
             return nullptr;
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 0dc1f7f68203..42d8a6f13b05 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -368,6 +368,8 @@ class CWalletScanState {
     std::map<std::pair<uint256, CKeyID>, std::pair<std::vector<unsigned char>, std::vector<unsigned char>>> crypted_mnemonics;
     bool tx_corrupt{false};
     bool descriptor_unknown{false};
+    bool unexpected_legacy_entry{false};
+
     CWalletScanState() = default;
 };
 
@@ -384,6 +386,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
         if (filter_fn && !filter_fn(strType)) {
             return true;
         }
+        // Legacy entries in descriptor wallets are not allowed, abort immediately
+        if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) {
+            wss.unexpected_legacy_entry = true;
+            return false;
+        }
         if (strType == DBKeys::NAME) {
             std::string strAddress;
             ssKey >> strAddress;
@@ -918,6 +925,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
             std::string strType, strErr;
             if (!ReadKeyValue(pwallet, ssKey, ssValue, wss, strType, strErr))
             {
+                if (wss.unexpected_legacy_entry) {
+                    strErr = strprintf("Error: Unexpected legacy entry found in descriptor wallet %s. ", pwallet->GetName());
+                    strErr += "The wallet might have been tampered with or created with malicious intent.";
+                    pwallet->WalletLogPrintf("%s\n", strErr);
+                    return DBErrors::UNEXPECTED_LEGACY_ENTRY;
+                }
                 // losing keys is considered a catastrophic error, anything else
                 // we assume the user can live with:
                 if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) {
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index f06177efeb2f..98e5b7834040 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -59,7 +59,8 @@ enum class DBErrors
     LOAD_FAIL,
     NEED_REWRITE,
     NEED_RESCAN,
-    UNKNOWN_DESCRIPTOR
+    UNKNOWN_DESCRIPTOR,
+    UNEXPECTED_LEGACY_ENTRY
 };
 
 namespace DBKeys {
diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py
index b0d0384f3c33..f4bc19d6a86b 100755
--- a/test/functional/wallet_descriptor.py
+++ b/test/functional/wallet_descriptor.py
@@ -3,6 +3,8 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test descriptor wallet function."""
+import os
+import sqlite3
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -188,5 +190,15 @@ def run_test(self):
                     imp_addr = imp_rpc.getnewaddress()
                 assert_equal(exp_addr, imp_addr)
 
+        self.log.info("Test that loading descriptor wallet containing legacy key types throws error")
+        self.nodes[0].createwallet(wallet_name="crashme", descriptors=True)
+        self.nodes[0].unloadwallet("crashme")
+        wallet_db = os.path.join(self.nodes[0].datadir, self.chain, "wallets", "crashme", self.wallet_data_filename)
+        with sqlite3.connect(wallet_db) as conn:
+            # add "cscript" entry: key type is uint160 (20 bytes), value type is CScript (zero-length here)
+            conn.execute('INSERT INTO main VALUES(?, ?)', (b'\x07cscript' + b'\x00'*20, b'\x00'))
+        assert_raises_rpc_error(-4, "Unexpected legacy entry in descriptor wallet found.", self.nodes[0].loadwallet, "crashme")
+
+
 if __name__ == '__main__':
     WalletDescriptorTest().main ()

From 1402e73c142cc75d52dc2c2b085262a5c15603cf Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 14 Dec 2022 16:06:05 -0500
Subject: [PATCH 14/15] Merge bitcoin/bitcoin#26668: wallet: if only have one
 output type, don't perform "mixed" coin selection

89c1491d35389eac0c1fecc59333cdfae3b1bd2c wallet: if only have one output type, don't perform "mixed" coin selection (furszy)

Pull request description:

  For wallets that only have one output type, we are currently performing the same
  selection process over the same coins twice.

  The "mixed coin selection" doesn't add any value to the result
  (there is nothing to mix if the available coins struct has only one type).

ACKs for top commit:
  achow101:
    ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c
  john-moffett:
    ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c
  kristapsk:
    cr utACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c

Tree-SHA512: 672eaeed3ba911d13fa61a46f719c8fe1ebe4d2dc7d723040e71937c693659411bc99cdbd9f0014e836b70eebeff1b8ca861f4d81d39e6f79f437364a526edbe

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/wallet/spend.cpp | 9 +++++----
 src/wallet/spend.h   | 2 ++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 3984989b9f5d..f2fb1c5de937 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -523,7 +523,7 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
     // Run coin selection on each OutputType and compute the Waste Metric
     std::vector<SelectionResult> results;
     for (const auto& it : available_coins.coins) {
-        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) {
+        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params, nCoinType)}) {
             results.push_back(*result);
         }
     }
@@ -532,9 +532,10 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
     if (results.size() > 0) return *std::min_element(results.begin(), results.end());
 
     // If we can't fund the transaction from any individual OutputType, run coin selection one last time
-    // over all available coins, which would allow mixing
-    if (allow_mixed_output_types) {
-        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) {
+    // over all available coins, which would allow mixing.
+    // If TypesCount() <= 1, there is nothing to mix.
+    if (allow_mixed_output_types && available_coins.TypesCount() > 1) {
+        if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params, nCoinType)}) {
             return result;
         }
     }
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 6462aeae6db8..21e30a46e2a6 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -45,6 +45,8 @@ struct CoinsResult {
     /** The following methods are provided so that CoinsResult can mimic a vector,
      * i.e., methods can work with individual OutputType vectors or on the entire object */
     size_t Size() const;
+    /** Return how many different output types this struct stores */
+    size_t TypesCount() const { return coins.size(); }
     void Clear();
     void Erase(std::set<COutPoint>& preset_coins);
     void Shuffle(FastRandomContext& rng_fast);

From 11df7615d009c85d1813e6956a850abfad135360 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Tue, 11 Apr 2023 13:58:00 -0400
Subject: [PATCH 15/15] partial Merge bitcoin/bitcoin#26699: wallet, gui:
 bugfix, getAvailableBalance skips selected coins

BACKPORT NOTE
Takes the wallet and send-dialog changes; excluded:
qt/test/wallettests.cpp coverage, the test-only getCoinControl() hook in sendcoinsdialog.h, the unrelated QMessageBox parenting hunk in presentPSBT, and the WalletModel::getAvailableBalance hunk (Dash has no such wrapper).

----------------
68eed5df8656bed1be6526b014e58d3123102b03 test,gui: add coverage for PSBT creation on legacy watch-only wallets (furszy)
306aab5bb471904faed325d9f3b38b7e891c7bbb test,gui: decouple widgets and model into a MiniGui struct (furszy)
2f76ac0383904123676f1b4eeba0f772a4c5cb5d test,gui: decouple chain and wallet initialization from test case (furszy)
cd98b717398f7b13ace91ea9efac9ce1e60b4d62 gui: 'getAvailableBalance', include watch only balance (furszy)
74eac3a82fc948467d5a15a5af420b36ce8eb04a test: add coverage for 'useAvailableBalance' functionality (furszy)
dc1cc1c35995dc09085b3d9270c445b7923fdb51 gui: bugfix, getAvailableBalance skips selected coins (furszy)

Pull request description:

  Fixes https://github.com/bitcoin-core/gui/issues/688 and https://github.com/bitcoin/bitcoin/issues/26687.

  First Issue Description (https://github.com/bitcoin-core/gui/issues/688):

  The previous behavior for `getAvailableBalance`, when the coin control had selected coins, was to return the sum of them. Instead, we are currently returning the wallet's available total balance minus the selected coins total amount.

  Reason:
  Missed to update the `GetAvailableBalance` function to include the coin control selected coins on #25685.

  Context:
  Since #25685 we skip the selected coins inside `AvailableCoins`, the reason is that there is no need to waste resources walking through the entire wallet's txes map just to get coins that could have gotten by just doing a simple `mapWallet.find`).

  Places Where This Generates Issues (only when the user manually select coins via coin control):
  1) The GUI balance check prior the transaction creation process.
  2) The GUI "useAvailableBalance" functionality.

  Note 1:
  As the GUI uses a balance cache since https://github.com/bitcoin-core/gui/pull/598, this issue does not affect the regular spending process. Only arises when the user manually select coins.

  Note 2:
  Added test coverage for the `useAvailableBalance` functionality.

  ----------------------------------

  Second Issue Description (https://github.com/bitcoin/bitcoin/issues/26687):

  As we are using a cached balance on `WalletModel::getAvailableBalance`,
  the function needs to include the watch-only available balance for wallets
  with private keys disabled.

ACKs for top commit:
  Sjors:
    tACK 68eed5df8656bed1be6526b014e58d3123102b03
  achow101:
    ACK 68eed5df8656bed1be6526b014e58d3123102b03
  theStack:
    ACK 68eed5df8656bed1be6526b014e58d3123102b03

Tree-SHA512: 674f3e050024dabda2ff4a04b9ed3750cf54a040527204c920e1e38bd3d7f5fd4d096e4fd08a0fea84ee6abb5070f022b5c0d450c58fd30202ef05ebfd7af6d3

Co-authored-by: Andrew Chow <github@achow101.com>
---
 src/bench/wallet_create_tx.cpp   |  2 +-
 src/qt/sendcoinsdialog.cpp       |  7 ++++++-
 src/wallet/interfaces.cpp        | 21 +++++++++++++++++++--
 src/wallet/spend.cpp             |  6 ------
 src/wallet/spend.h               |  2 --
 src/wallet/test/wallet_tests.cpp |  8 ++++----
 6 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
index 06fa42c9f1e7..b6875cd87374 100644
--- a/src/bench/wallet_create_tx.cpp
+++ b/src/bench/wallet_create_tx.cpp
@@ -103,7 +103,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type
     }
 
     // Check available balance
-    auto bal = wallet::GetAvailableBalance(wallet); // Cache
+    auto bal = WITH_LOCK(wallet.cs_wallet, return wallet::AvailableCoins(wallet).total_amount); // Cache
     assert(bal == 50 * COIN * (chain_size - COINBASE_MATURITY));
 
     wallet::CCoinControl coin_control;
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index e2a2be3e13dd..bf8b039a4fb0 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -897,8 +897,13 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry)
     // Include watch-only for wallets without private key
     m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled() && !model->wallet().hasExternalSigner();
 
+    // Same behavior as send: if we have selected coins, only obtain their available balance.
+    // Copy to avoid modifying the member's data.
+    CCoinControl coin_control = *m_coin_control;
+    coin_control.m_allow_other_inputs = !coin_control.HasSelected();
+
     // Calculate available amount to send.
-    CAmount amount = model->wallet().getAvailableBalance(*m_coin_control);
+    CAmount amount = model->wallet().getAvailableBalance(coin_control);
     for (int i = 0; i < ui->entries->count(); ++i) {
         SendCoinsEntry* e = qobject_cast<SendCoinsEntry*>(ui->entries->itemAt(i)->widget());
         if (e && !e->isHidden() && e != entry) {
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 2c31206bd824..166c68cb1605 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -23,6 +23,7 @@
 #include <util/translation.h>
 #include <util/ui_change_type.h>
 #include <validation.h>
+#include <wallet/coincontrol.h>
 #include <wallet/coinjoin.h>
 #include <wallet/context.h>
 #include <wallet/fees.h>
@@ -665,9 +666,25 @@ class WalletImpl : public Wallet
     {
         if (coin_control.IsUsingCoinJoin()) {
             return GetBalanceAnonymized(*m_wallet, coin_control);
-        } else {
-            return GetAvailableBalance(*m_wallet, &coin_control);
         }
+        LOCK(m_wallet->cs_wallet);
+        CAmount total_amount = 0;
+        // Fetch selected coins total amount
+        if (coin_control.HasSelected()) {
+            FastRandomContext rng{};
+            CoinSelectionParams params(rng);
+            // Note: for now, swallow any error.
+            if (auto res = FetchSelectedInputs(*m_wallet, coin_control, params)) {
+                total_amount += res->total_amount;
+            }
+        }
+
+        // And fetch the wallet available coins
+        if (coin_control.m_allow_other_inputs) {
+            total_amount += AvailableCoins(*m_wallet, &coin_control).total_amount;
+        }
+
+        return total_amount;
     }
     wallet::isminetype txinIsMine(const CTxIn& txin) override
     {
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index f2fb1c5de937..9692c35f9a8a 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -356,12 +356,6 @@ CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl*
     return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, params);
 }
 
-CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl)
-{
-    LOCK(wallet.cs_wallet);
-    return AvailableCoins(wallet, coinControl).total_amount;
-}
-
 const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output)
 {
     AssertLockHeld(wallet.cs_wallet);
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 21e30a46e2a6..cfa587725cb2 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -85,8 +85,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
  */
 CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl = nullptr, CoinFilterParams params = {}) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
-CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl = nullptr);
-
 /**
  * Find non-change parent output.
  */
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 00d7c3f5dd70..3dd4ec5b4256 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -722,7 +722,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U);
 
     // Check initial balance from one mature coinbase transaction.
-    BOOST_CHECK_EQUAL(500 * COIN, GetAvailableBalance(*wallet));
+    BOOST_CHECK_EQUAL(500 * COIN, WITH_LOCK(wallet->cs_wallet, return AvailableCoins(*wallet).total_amount));
 
     // Add a transaction creating a change address, and confirm ListCoins still
     // returns the coin associated with the change address underneath the
@@ -1560,7 +1560,7 @@ BOOST_FIXTURE_TEST_CASE(CreateTransactionTest, CreateTransactionTestSetup)
 BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup)
 {
     // Check initial balance from one mature coinbase transaction.
-    BOOST_CHECK_EQUAL(GetAvailableBalance(*wallet), 500 * COIN);
+    BOOST_CHECK_EQUAL(WITH_LOCK(wallet->cs_wallet, return AvailableCoins(*wallet).total_amount), 500 * COIN);
 
     {
         std::vector<CompactTallyItem> vecTally = wallet->SelectCoinsGroupedByAddresses(/*fSkipDenominated=*/false,
@@ -1583,7 +1583,7 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup
     BOOST_CHECK(ret2);
     const auto& txr2 = ret2->tx;
     wallet->CommitTransaction(txr1, {}, {});
-    BOOST_CHECK_EQUAL(GetAvailableBalance(*wallet), 0);
+    BOOST_CHECK_EQUAL(WITH_LOCK(wallet->cs_wallet, return AvailableCoins(*wallet).total_amount), 0);
     CreateAndProcessBlock({CMutableTransaction(*txr2)}, GetScriptForRawPubKey({}));
     struct ChainInfo {
         const CBlockIndex* tip;
@@ -1620,7 +1620,7 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup
     BOOST_CHECK_EQUAL(vecTally.at(0).outpoints.size(), 1);
     BOOST_CHECK_EQUAL(vecTally.at(1).outpoints.size(), 1);
     BOOST_CHECK_EQUAL(vecTally.at(0).nAmount + vecTally.at(1).nAmount, (500 + 499) * COIN);
-    BOOST_CHECK_EQUAL(GetAvailableBalance(*wallet), (500 + 499) * COIN);
+    BOOST_CHECK_EQUAL(WITH_LOCK(wallet->cs_wallet, return AvailableCoins(*wallet).total_amount), (500 + 499) * COIN);
 }