Merge bitcoin/bitcoin#24644: wallet: add tracepoints and algorithm information to coin selection

ab5af9c test: Add test for coinselection tracepoints (Andrew Chow)
ca02b68 doc: document coin selection tracepoints (Andrew Chow)
8e3f39e wallet: Add some tracepoints for coin selection (Andrew Chow)
15b5838 wallet: compute waste for SelectionResults of preset inputs (Andrew Chow)
912f1ed wallet: track which coin selection algorithm produced a SelectionResult (Andrew Chow)

Pull request description:

  Tracepoints can be useful for coin selection as they would allow us to observe what is being selected, selection parameters, and calculation results. So this PR adds 4 new tracepoints:

  1. After `SelectCoins` returns in order to observe the `SelectionResult`
  2. After the first `CreateTransactionInternal` to observe the created transaction
  3. Prior to the second `CreateTransactionInternal` to notify that the optimistic avoid partial spends selection is occurring
  4. After the second `CreateTransactionInternal` to observe the created transaction and inform which solution is being used.

  This PR also adds an algorithm enum to `SelectionResult` so that the first tracepoint will be able to report which algorithm was used to produce that result.

  The primary use case for these tracepoints is in running coin selection simulations. The script I use to run these simulations use these tracepoints in order to gather data on the algorithm used and the calculated waste.

ACKs for top commit:
  jb55:
    crACK ab5af9c
  josibake:
    crACK bitcoin/bitcoin@ab5af9c
  0xB10C:
    ACK ab5af9c. Code reviewed, ran the `interface_usdt_coinselection.py` test, and tested with the above bpftrace script (updated `%d` -> `%ld` where necessary, ty achow101).

Tree-SHA512: a4bf7a910cdf464622f2f3b5d44c15b891f24852df6e7f8c5b177fe3d8aaa4a1164593a24c3960eb22b16544fa7140e5c745345367b9e291b78395084c0ac8ff

Author: fanquake

doc/tracing.md

@@ -168,6 +168,49 @@ Arguments passed:
 4. Value of the coin as `int64`
 5. If the coin is a coinbase as `bool`
 
+### Context `coin_selection`
+
+#### Tracepoint `coin_selection:selected_coins`
+
+Is called when `SelectCoins` completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Coin selection algorithm name as `pointer to C-style string`
+3. Selection target value as `int64`
+4. Calculated waste metric of the solution as `int64`
+5. Total value of the selected inputs as `int64`
+
+#### Tracepoint `coin_selection:normal_create_tx_internal`
+
+Is called when the first `CreateTransactionInternal` completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Whether `CreateTransactionInternal` succeeded as `bool`
+3. The expected transaction fee as an `int64`
+4. The position of the change output as an `int32`
+
+#### Tracepoint `coin_selection:attempting_aps_create_tx`
+
+Is called when `CreateTransactionInternal` is called the second time for the optimistic
+Avoid Partial Spends selection attempt. This is used to determine whether the next
+tracepoints called are for the Avoid Partial Spends solution, or a different transaction.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+
+#### Tracepoint `coin_selection:aps_create_tx_internal`
+
+Is called when the second `CreateTransactionInternal` with Avoid Partial Spends enabled completes.
+
+Arguments passed:
+1. Wallet name as `pointer to C-style string`
+2. Whether the Avoid Partial Spends solution will be used as `bool`
+3. Whether `CreateTransactionInternal` succeeded as` bool`
+4. The expected transaction fee as an `int64`
+5. The position of the change output as an `int32`
+
 ## Adding tracepoints to Bitcoin Core
 
 To add a new tracepoint, `#include <util/trace.h>` in the compilation unit where

src/wallet/coinselection.cpp

@@ -64,7 +64,7 @@ static const size_t TOTAL_TRIES = 100000;
 
 std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
 {
-    SelectionResult result(selection_target);
+    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     CAmount curr_value = 0;
     std::vector<size_t> curr_selection; // selected utxo indexes
 
@@ -167,7 +167,7 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
 
 std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng)
 {
-    SelectionResult result(target_value);
+    SelectionResult result(target_value, SelectionAlgorithm::SRD);
 
     std::vector<size_t> indexes;
     indexes.resize(utxo_pool.size());
@@ -249,7 +249,7 @@ static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::v
 std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue,
                                               CAmount change_target, FastRandomContext& rng)
 {
-    SelectionResult result(nTargetValue);
+    SelectionResult result(nTargetValue, SelectionAlgorithm::KNAPSACK);
 
     // List of values less than target
     std::optional<OutputGroup> lowest_larger;
@@ -460,4 +460,17 @@ std::string COutput::ToString() const
 {
     return strprintf("COutput(%s, %d, %d) [%s]", outpoint.hash.ToString(), outpoint.n, depth, FormatMoney(txout.nValue));
 }
+
+std::string GetAlgorithmName(const SelectionAlgorithm algo)
+{
+    switch (algo)
+    {
+    case SelectionAlgorithm::BNB: return "bnb";
+    case SelectionAlgorithm::KNAPSACK: return "knapsack";
+    case SelectionAlgorithm::SRD: return "srd";
+    case SelectionAlgorithm::MANUAL: return "manual";
+    // No default case to allow for compiler to warn
+    }
+    assert(false);
+}
 } // namespace wallet

src/wallet/coinselection.h

@@ -239,21 +239,34 @@ struct OutputGroup
  */
 [[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng);
 
+enum class SelectionAlgorithm : uint8_t
+{
+    BNB = 0,
+    KNAPSACK = 1,
+    SRD = 2,
+    MANUAL = 3,
+};
+
+std::string GetAlgorithmName(const SelectionAlgorithm algo);
+
 struct SelectionResult
 {
 private:
     /** Set of inputs selected by the algorithm to use in the transaction */
     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) */
     bool m_use_effective{false};
     /** The computed waste */
     std::optional<CAmount> m_waste;
 
 public:
-    explicit SelectionResult(const CAmount target)
-        : m_target(target) {}
+    /** 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;
+    /** The algorithm used to produce this result */
+    const SelectionAlgorithm m_algo;
+
+    explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
+        : m_target(target), m_algo(algo) {}
 
     SelectionResult() = delete;
 

src/wallet/spend.cpp

@@ -11,6 +11,7 @@
 #include <util/fees.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
+#include <util/trace.h>
 #include <util/translation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/fees.h>
@@ -435,9 +436,10 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
              */
             preset_inputs.Insert(out, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false);
         }
-        SelectionResult result(nTargetValue);
+        SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL);
         result.AddInput(preset_inputs);
         if (result.GetSelectedValue() < nTargetValue) return std::nullopt;
+        result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
         return result;
     }
 
@@ -519,7 +521,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
     // permissive CoinEligibilityFilter.
     std::optional<SelectionResult> res = [&] {
         // Pre-selected inputs already cover the target amount.
-        if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue));
+        if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue, SelectionAlgorithm::MANUAL));
 
         // 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.
@@ -573,6 +575,9 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
 
     // Add preset inputs to result
     res->AddInput(preset_inputs);
+    if (res->m_algo == SelectionAlgorithm::MANUAL) {
+        res->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
+    }
 
     return res;
 }
@@ -788,6 +793,7 @@ static bool CreateTransactionInternal(
         error = _("Insufficient funds");
         return false;
     }
+    TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue());
 
     // Always make a change output
     // We will reduce the fee from this change output later, and remove the output if it is too small.
@@ -978,8 +984,10 @@ bool CreateTransaction(
     int nChangePosIn = nChangePosInOut;
     Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr)
     bool res = CreateTransactionInternal(wallet, vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign);
+    TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res, nFeeRet, nChangePosInOut);
     // try with avoidpartialspends unless it's enabled already
     if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) {
+        TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str());
         CCoinControl tmp_cc = coin_control;
         tmp_cc.m_avoid_partial_spends = true;
         CAmount nFeeRet2;
@@ -990,6 +998,7 @@ bool CreateTransaction(
             // if fee of this alternative one is within the range of the max fee, we use this one
             const bool use_aps = nFeeRet2 <= nFeeRet + wallet.m_max_aps_fee;
             wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped");
+            TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, res, nFeeRet2, nChangePosInOut2);
             if (use_aps) {
                 tx = tx2;
                 nFeeRet = nFeeRet2;

src/wallet/test/coinselector_tests.cpp

@@ -168,7 +168,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     FastRandomContext rand{};
     // Setup
     std::vector<COutput> utxo_pool;
-    SelectionResult expected_result(CAmount(0));
+    SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
 
     /////////////////////////
     // Known Outcome tests //