Merge bitcoin/bitcoin#24091: wallet: Consolidate CInputCoin and COutput

049003fe68a4183f6f20da16f58f10079d1e02df coinselection: Remove COutput operators == and != (Andrew Chow)
f6c39c6adb6cbf9c87f04d3d667701905ef5c0a0 coinselection: Remove CInputCoin (Andrew Chow)
70f31f1a81710aa59e95770de9a84bf58cbce1e8 coinselection: Use COutput instead of CInputCoin (Andrew Chow)
14fbb57b79c664090f6a4e60d7bdfc9759ff4307 coinselection: Add effective value and fees to COutput (Andrew Chow)
f0821230b8de2eec21a869d1edf9e2b9f502de25 moveonly: move COutput to coinselection.h (Andrew Chow)
42e974e15c6deba1d9395a4da9341c9ebec6e8e5 wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow)
14d04d5ad15ae56df56edee7ca9a202b52037889 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow)
0ba4d1916e26e2a5d603edcdb7625463989d25b6 wallet: Provide input bytes to COutput (Andrew Chow)
d51f27d3bb0d6e3ca55bcd23ce53e4fe413a9360 wallet: Store whether a COutput is from the wallet (Andrew Chow)
b799814bbd53736b79495072f3c9e05989a465e8 wallet: Store tx time in COutput (Andrew Chow)
46022953ee2e8113167bafd1fd48a383a578b13c wallet: Remove use_max_sig default value (Andrew Chow)
10379f007fd2c18f4cd24d0a0783d6d929f45556 scripted-diff: Rename COutput member variables (Andrew Chow)
c7c64db41e1718584aa2f30ff27f60ab0966de62 wallet: cleanup COutput constructor (Andrew Chow)

Pull request description:

  While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`.

  `COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor.

ACKs for top commit:
  ryanofsky:
    Code review ACK 049003fe68a4183f6f20da16f58f10079d1e02df, just adding comments and removing == operators since last review
  w0xlt:
    reACK 049003f
  Xekyo:
    reACK 049003fe68a4183f6f20da16f58f10079d1e02df

Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15

1 files changed, 61 insertions, 47 deletions

diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index a0941d32df..504d57aa78 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -19,56 +19,70 @@ static constexpr CAmount MIN_CHANGE{COIN / 100};
 static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2;
 
 /** A UTXO under consideration for use in funding a new transaction. */
-class CInputCoin {
+class COutput
+{
 public:
-    CInputCoin(const CTransactionRef& tx, unsigned int i)
-    {
-        if (!tx)
-            throw std::invalid_argument("tx should not be null");
-        if (i >= tx->vout.size())
-            throw std::out_of_range("The output index is out of range");
-
-        outpoint = COutPoint(tx->GetHash(), i);
-        txout = tx->vout[i];
-        effective_value = txout.nValue;
-    }
+    /** The outpoint identifying this UTXO */
+    COutPoint outpoint;
 
-    CInputCoin(const CTransactionRef& tx, unsigned int i, int input_bytes) : CInputCoin(tx, i)
-    {
-        m_input_bytes = input_bytes;
-    }
+    /** The output itself */
+    CTxOut txout;
 
-    CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in)
-    {
-        outpoint = outpoint_in;
-        txout = txout_in;
-        effective_value = txout.nValue;
-    }
+    /**
+     * Depth in block chain.
+     * If > 0: the tx is on chain and has this many confirmations.
+     * If = 0: the tx is waiting confirmation.
+     * If < 0: a conflicting tx is on chain and has this many confirmations. */
+    int depth;
 
-    CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in, int input_bytes) : CInputCoin(outpoint_in, txout_in)
-    {
-        m_input_bytes = input_bytes;
-    }
+    /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */
+    int input_bytes;
 
-    COutPoint outpoint;
-    CTxOut txout;
+    /** Whether we have the private keys to spend this output */
+    bool spendable;
+
+    /** Whether we know how to spend this output, ignoring the lack of keys */
+    bool solvable;
+
+    /**
+     * Whether this output is considered safe to spend. Unconfirmed transactions
+     * from outside keys and unconfirmed replacement transactions are considered
+     * unsafe and will not be used to fund new spending transactions.
+     */
+    bool safe;
+
+    /** The time of the transaction containing this output as determined by CWalletTx::nTimeSmart */
+    int64_t time;
+
+    /** Whether the transaction containing this output is sent from the owning wallet */
+    bool from_me;
+
+    /** The output's value minus fees required to spend it. Initialized as the output's absolute value. */
     CAmount effective_value;
-    CAmount m_fee{0};
-    CAmount m_long_term_fee{0};
 
-    /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */
-    int m_input_bytes{-1};
+    /** The fee required to spend this output at the transaction's target feerate. */
+    CAmount fee{0};
 
-    bool operator<(const CInputCoin& rhs) const {
-        return outpoint < rhs.outpoint;
-    }
+    /** The fee required to spend this output at the consolidation feerate. */
+    CAmount long_term_fee{0};
 
-    bool operator!=(const CInputCoin& rhs) const {
-        return outpoint != rhs.outpoint;
-    }
+    COutput(const COutPoint& outpoint, const CTxOut& txout, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me)
+        : outpoint(outpoint),
+        txout(txout),
+        depth(depth),
+        input_bytes(input_bytes),
+        spendable(spendable),
+        solvable(solvable),
+        safe(safe),
+        time(time),
+        from_me(from_me),
+        effective_value(txout.nValue)
+    {}
+
+    std::string ToString() const;
 
-    bool operator==(const CInputCoin& rhs) const {
-        return outpoint == rhs.outpoint;
+    bool operator<(const COutput& rhs) const {
+        return outpoint < rhs.outpoint;
     }
 };
 
@@ -143,7 +157,7 @@ struct CoinEligibilityFilter
 struct OutputGroup
 {
     /** The list of UTXOs contained in this output group. */
-    std::vector<CInputCoin> m_outputs;
+    std::vector<COutput> m_outputs;
     /** Whether the UTXOs were sent by the wallet to itself. This is relevant because we may want at
      * least a certain number of confirmations on UTXOs received from outside wallets while trusting
      * our own UTXOs more. */
@@ -180,7 +194,7 @@ struct OutputGroup
         m_subtract_fee_outputs(params.m_subtract_fee_outputs)
     {}
 
-    void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only);
+    void Insert(const COutput& output, size_t ancestors, size_t descendants, bool positive_only);
     bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const;
     CAmount GetSelectionAmount() const;
 };
@@ -202,13 +216,13 @@ struct OutputGroup
  * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false).
  * @return The waste
  */
-[[nodiscard]] CAmount GetSelectionWaste(const std::set<CInputCoin>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true);
+[[nodiscard]] CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true);
 
 struct SelectionResult
 {
 private:
     /** Set of inputs selected by the algorithm to use in the transaction */
-    std::set<CInputCoin> m_selected_inputs;
+    std::set<COutput> m_selected_inputs;
     /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */
     const CAmount m_target;
     /** Whether the input values for calculations should be the effective value (true) or normal value (false) */
@@ -234,9 +248,9 @@ public:
     [[nodiscard]] CAmount GetWaste() const;
 
     /** Get m_selected_inputs */
-    const std::set<CInputCoin>& GetInputSet() const;
-    /** Get the vector of CInputCoins that will be used to fill in a CTransaction's vin */
-    std::vector<CInputCoin> GetShuffledInputVector() const;
+    const std::set<COutput>& GetInputSet() const;
+    /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */
+    std::vector<COutput> GetShuffledInputVector() const;
 
     bool operator<(SelectionResult other) const;
 };