Merge bitcoin/bitcoin#26066: wallet: Refactor and document CoinControl

src/qt/coincontroldialog.cpp

@@ -454,8 +454,7 @@ void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel *
     unsigned int nQuantity      = 0;
     bool fUnselectedSpent{false};
 
-    std::vector<COutPoint> vCoinControl;
-    m_coin_control.ListSelected(vCoinControl);
+    auto vCoinControl{m_coin_control.ListSelected()};
 
     size_t i = 0;
     for (const auto& out : model->wallet().getCoins(vCoinControl)) {

src/wallet/coincontrol.cpp

@@ -12,4 +12,72 @@ CCoinControl::CCoinControl(CoinType coinType)
 {
     m_avoid_partial_spends = gArgs.GetBoolArg("-avoidpartialspends", DEFAULT_AVOIDPARTIALSPENDS);
 }
+
+bool CCoinControl::HasSelected() const
+{
+    return !m_selected_inputs.empty();
+}
+
+bool CCoinControl::IsSelected(const COutPoint& output) const
+{
+    return m_selected_inputs.count(output) > 0;
+}
+
+bool CCoinControl::IsExternalSelected(const COutPoint& output) const
+{
+    return m_external_txouts.count(output) > 0;
+}
+
+std::optional<CTxOut> CCoinControl::GetExternalOutput(const COutPoint& outpoint) const
+{
+    const auto ext_it = m_external_txouts.find(outpoint);
+    if (ext_it == m_external_txouts.end()) {
+        return std::nullopt;
+    }
+
+    return std::make_optional(ext_it->second);
+}
+
+void CCoinControl::Select(const COutPoint& output)
+{
+    m_selected_inputs.insert(output);
+}
+
+void CCoinControl::SelectExternal(const COutPoint& outpoint, const CTxOut& txout)
+{
+    m_selected_inputs.insert(outpoint);
+    m_external_txouts.emplace(outpoint, txout);
+}
+
+void CCoinControl::UnSelect(const COutPoint& output)
+{
+    m_selected_inputs.erase(output);
+}
+
+void CCoinControl::UnSelectAll()
+{
+    m_selected_inputs.clear();
+}
+
+std::vector<COutPoint> CCoinControl::ListSelected() const
+{
+    return {m_selected_inputs.begin(), m_selected_inputs.end()};
+}
+
+void CCoinControl::SetInputWeight(const COutPoint& outpoint, int64_t weight)
+{
+    m_input_weights[outpoint] = weight;
+}
+
+bool CCoinControl::HasInputWeight(const COutPoint& outpoint) const
+{
+    return m_input_weights.count(outpoint) > 0;
+}
+
+int64_t CCoinControl::GetInputWeight(const COutPoint& outpoint) const
+{
+    auto it = m_input_weights.find(outpoint);
+    assert(it != m_input_weights.end());
+    return it->second;
+}
 } // namespace wallet

src/wallet/coincontrol.h

@@ -13,9 +13,9 @@
 #include <script/signingprovider.h>
 #include <script/standard.h>
 
-#include <optional>
 #include <algorithm>
 #include <map>
+#include <optional>
 #include <set>
 
 namespace wallet {
@@ -78,56 +78,56 @@ class CCoinControl
 
     CCoinControl(CoinType coin_type = CoinType::ALL_COINS);
 
-    bool HasSelected() const
-    {
-        return (setSelected.size() > 0);
-    }
-
-    bool IsSelected(const COutPoint& output) const
-    {
-        return (setSelected.count(output) > 0);
-    }
-
-    bool IsExternalSelected(const COutPoint& output) const
-    {
-        return (m_external_txouts.count(output) > 0);
-    }
-
-    bool GetExternalOutput(const COutPoint& outpoint, CTxOut& txout) const
-    {
-        const auto ext_it = m_external_txouts.find(outpoint);
-        if (ext_it == m_external_txouts.end()) {
-            return false;
-        }
-        txout = ext_it->second;
-        return true;
-    }
-
-    void Select(const COutPoint& output)
-    {
-        setSelected.insert(output);
-    }
-
-    void SelectExternal(const COutPoint& outpoint, const CTxOut& txout)
-    {
-        setSelected.insert(outpoint);
-        m_external_txouts.emplace(outpoint, txout);
-    }
-
-    void UnSelect(const COutPoint& output)
-    {
-        setSelected.erase(output);
-    }
-
-    void UnSelectAll()
-    {
-        setSelected.clear();
-    }
-
-    void ListSelected(std::vector<COutPoint>& vOutpoints) const
-    {
-        vOutpoints.assign(setSelected.begin(), setSelected.end());
-    }
+    /**
+     * Returns true if there are pre-selected inputs.
+     */
+    bool HasSelected() const;
+    /**
+     * Returns true if the given output is pre-selected.
+     */
+    bool IsSelected(const COutPoint& output) const;
+    /**
+     * Returns true if the given output is selected as an external input.
+     */
+    bool IsExternalSelected(const COutPoint& output) const;
+    /**
+     * Returns the external output for the given outpoint if it exists.
+     */
+    std::optional<CTxOut> GetExternalOutput(const COutPoint& outpoint) const;
+    /**
+     * Lock-in the given output for spending.
+     * The output will be included in the transaction even if it's not the most optimal choice.
+     */
+    void Select(const COutPoint& output);
+    /**
+     * Lock-in the given output as an external input for spending because it is not in the wallet.
+     * The output will be included in the transaction even if it's not the most optimal choice.
+     */
+    void SelectExternal(const COutPoint& outpoint, const CTxOut& txout);
+    /**
+     * Unselects the given output.
+     */
+    void UnSelect(const COutPoint& output);
+    /**
+     * Unselects all outputs.
+     */
+    void UnSelectAll();
+    /**
+     * List the selected inputs.
+     */
+    std::vector<COutPoint> ListSelected() const;
+    /**
+     * Set an input's weight.
+     */
+    void SetInputWeight(const COutPoint& outpoint, int64_t weight);
+    /**
+     * Returns true if the input weight is set.
+     */
+    bool HasInputWeight(const COutPoint& outpoint) const;
+    /**
+     * Returns the input weight.
+     */
+    int64_t GetInputWeight(const COutPoint& outpoint) const;
 
     // Dash-specific helpers
     void UseCoinJoin(bool fUseCoinJoin)
@@ -141,8 +141,13 @@ class CCoinControl
     }
 
 private:
-    std::set<COutPoint> setSelected;
+    //! Selected inputs (inputs that will be used, regardless of whether they're optimal or not)
+    std::set<COutPoint> m_selected_inputs;
+    //! Map of external inputs to include in the transaction
+    //! These are not in the wallet, so we need to track them separately
     std::map<COutPoint, CTxOut> m_external_txouts;
+    //! Map of COutPoints to the maximum weight for that input
+    std::map<COutPoint, int64_t> m_input_weights;
 };
 } // namespace wallet
 

src/wallet/spend.cpp

@@ -65,11 +65,11 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wall
             assert(input.prevout.n < mi->second.tx->vout.size());
             txouts.emplace_back(mi->second.tx->vout.at(input.prevout.n));
         } else if (coin_control) {
-            CTxOut txout;
-            if (!coin_control->GetExternalOutput(input.prevout, txout)) {
+            const auto& txout{coin_control->GetExternalOutput(input.prevout)};
+            if (!txout) {
                 return -1;
             }
-            txouts.emplace_back(txout);
+            txouts.emplace_back(*txout);
         } else {
             return -1;
         }
@@ -522,9 +522,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
     // calculate value from preset inputs and store them
     std::set<COutPoint> preset_coins;
 
-    std::vector<COutPoint> vPresetInputs;
-    coin_control.ListSelected(vPresetInputs);
-    for (const COutPoint& outpoint : vPresetInputs) {
+    for (const COutPoint& outpoint : coin_control.ListSelected()) {
         int input_bytes = -1;
         CTxOut txout;
         auto ptr_wtx = wallet.GetWalletTx(outpoint.hash);
@@ -537,9 +535,11 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
             input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control);
         } else {
             // The input is external. We did not find the tx in mapWallet.
-            if (!coin_control.GetExternalOutput(outpoint, txout)) {
+            const auto external_txout{coin_control.GetExternalOutput(outpoint)};
+            if (!external_txout) {
                 return std::nullopt;
             }
+            txout = *external_txout;
             input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control);
         }
         if (nCoinType == CoinType::ONLY_FULLY_MIXED) {

src/wallet/test/fuzz/coincontrol.cpp

@@ -53,8 +53,7 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
                 (void)coin_control.IsExternalSelected(out_point);
             },
             [&] {
-                CTxOut txout;
-                (void)coin_control.GetExternalOutput(out_point, txout);
+                (void)coin_control.GetExternalOutput(out_point);
             },
             [&] {
                 (void)coin_control.Select(out_point);
@@ -70,8 +69,7 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
                 (void)coin_control.UnSelectAll();
             },
             [&] {
-                std::vector<COutPoint> selected;
-                coin_control.ListSelected(selected);
+                (void)coin_control.ListSelected();
             });
     }
 }