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

ab5af9ca72 test: Add test for coinselection tracepoints (Andrew Chow)
ca02b68e8a doc: document coin selection tracepoints (Andrew Chow)
8e3f39e4fa wallet: Add some tracepoints for coin selection (Andrew Chow)
15b58383d0 wallet: compute waste for SelectionResults of preset inputs (Andrew Chow)
912f1ed181 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 ab5af9ca72
  josibake:
    crACK ab5af9ca72
  0xB10C:
    ACK ab5af9ca72. 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

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 //

test/functional/interface_usdt_coinselection.py

@@ -0,0 +1,208 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+"""  Tests the coin_selection:* tracepoint API interface.
+     See https://github.com/bitcoin/bitcoin/blob/master/doc/tracing.md#context-coin_selection
+"""
+
+# Test will be skipped if we don't have bcc installed
+try:
+    from bcc import BPF, USDT # type: ignore[import]
+except ImportError:
+    pass
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    assert_raises_rpc_error,
+)
+
+coinselection_tracepoints_program = """
+#include <uapi/linux/ptrace.h>
+
+#define WALLET_NAME_LENGTH 16
+#define ALGO_NAME_LENGTH 16
+
+struct event_data
+{
+    u8 type;
+    char wallet_name[WALLET_NAME_LENGTH];
+
+    // selected coins event
+    char algo[ALGO_NAME_LENGTH];
+    s64 target;
+    s64 waste;
+    s64 selected_value;
+
+    // create tx event
+    bool success;
+    s64 fee;
+    s32 change_pos;
+
+    // aps create tx event
+    bool use_aps;
+};
+
+BPF_QUEUE(coin_selection_events, struct event_data, 1024);
+
+int trace_selected_coins(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 1;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg_p(2, ctx, &data.algo, ALGO_NAME_LENGTH);
+    bpf_usdt_readarg(3, ctx, &data.target);
+    bpf_usdt_readarg(4, ctx, &data.waste);
+    bpf_usdt_readarg(5, ctx, &data.selected_value);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_normal_create_tx(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 2;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg(2, ctx, &data.success);
+    bpf_usdt_readarg(3, ctx, &data.fee);
+    bpf_usdt_readarg(4, ctx, &data.change_pos);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_attempt_aps(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 3;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+
+int trace_aps_create_tx(struct pt_regs *ctx) {
+    struct event_data data;
+    __builtin_memset(&data, 0, sizeof(data));
+    data.type = 4;
+    bpf_usdt_readarg_p(1, ctx, &data.wallet_name, WALLET_NAME_LENGTH);
+    bpf_usdt_readarg(2, ctx, &data.use_aps);
+    bpf_usdt_readarg(3, ctx, &data.success);
+    bpf_usdt_readarg(4, ctx, &data.fee);
+    bpf_usdt_readarg(5, ctx, &data.change_pos);
+    coin_selection_events.push(&data, 0);
+    return 0;
+}
+"""
+
+
+class CoinSelectionTracepointTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def skip_test_if_missing_module(self):
+        self.skip_if_platform_not_linux()
+        self.skip_if_no_bitcoind_tracepoints()
+        self.skip_if_no_python_bcc()
+        self.skip_if_no_bpf_permissions()
+        self.skip_if_no_wallet()
+
+    def get_tracepoints(self, expected_types):
+        events = []
+        try:
+            for i in range(0, len(expected_types) + 1):
+                event = self.bpf["coin_selection_events"].pop()
+                assert_equal(event.wallet_name.decode(), self.default_wallet_name)
+                assert_equal(event.type, expected_types[i])
+                events.append(event)
+            else:
+                # If the loop exits successfully instead of throwing a KeyError, then we have had
+                # more events than expected. There should be no more than len(expected_types) events.
+                assert False
+        except KeyError:
+            assert_equal(len(events), len(expected_types))
+            return events
+
+
+    def determine_selection_from_usdt(self, events):
+        success = None
+        use_aps = None
+        algo = None
+        waste = None
+        change_pos = None
+
+        is_aps = False
+        sc_events = []
+        for event in events:
+            if event.type == 1:
+                if not is_aps:
+                    algo = event.algo.decode()
+                    waste = event.waste
+                sc_events.append(event)
+            elif event.type == 2:
+                success = event.success
+                if not is_aps:
+                    change_pos = event.change_pos
+            elif event.type == 3:
+                is_aps = True
+            elif event.type == 4:
+                assert is_aps
+                if event.use_aps:
+                    use_aps = True
+                    assert_equal(len(sc_events), 2)
+                    algo = sc_events[1].algo.decode()
+                    waste = sc_events[1].waste
+                    change_pos = event.change_pos
+        return success, use_aps, algo, waste, change_pos
+
+    def run_test(self):
+        self.log.info("hook into the coin_selection tracepoints")
+        ctx = USDT(pid=self.nodes[0].process.pid)
+        ctx.enable_probe(probe="coin_selection:selected_coins", fn_name="trace_selected_coins")
+        ctx.enable_probe(probe="coin_selection:normal_create_tx_internal", fn_name="trace_normal_create_tx")
+        ctx.enable_probe(probe="coin_selection:attempting_aps_create_tx", fn_name="trace_attempt_aps")
+        ctx.enable_probe(probe="coin_selection:aps_create_tx_internal", fn_name="trace_aps_create_tx")
+        self.bpf = BPF(text=coinselection_tracepoints_program, usdt_contexts=[ctx], debug=0)
+
+        self.log.info("Prepare wallets")
+        self.generate(self.nodes[0], 101)
+        wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+
+        self.log.info("Sending a transaction should result in all tracepoints")
+        # We should have 5 tracepoints in the order:
+        # 1. selected_coins (type 1)
+        # 2. normal_create_tx_internal (type 2)
+        # 3. attempting_aps_create_tx (type 3)
+        # 4. selected_coins (type 1)
+        # 5. aps_create_tx_internal (type 4)
+        wallet.sendtoaddress(wallet.getnewaddress(), 10)
+        events = self.get_tracepoints([1, 2, 3, 1, 4])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, True)
+        assert_greater_than(change_pos, -1)
+
+        self.log.info("Failing to fund results in 1 tracepoint")
+        # We should have 1 tracepoints in the order
+        # 1. normal_create_tx_internal (type 2)
+        assert_raises_rpc_error(-6, "Insufficient funds", wallet.sendtoaddress, wallet.getnewaddress(), 102 * 50)
+        events = self.get_tracepoints([2])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, False)
+
+        self.log.info("Explicitly enabling APS results in 2 tracepoints")
+        # We should have 2 tracepoints in the order
+        # 1. selected_coins (type 1)
+        # 2. normal_create_tx_internal (type 2)
+        wallet.setwalletflag("avoid_reuse")
+        wallet.sendtoaddress(address=wallet.getnewaddress(), amount=10, avoid_reuse=True)
+        events = self.get_tracepoints([1, 2])
+        success, use_aps, algo, waste, change_pos = self.determine_selection_from_usdt(events)
+        assert_equal(success, True)
+        assert_equal(use_aps, None)
+
+        self.bpf.cleanup()
+
+
+if __name__ == '__main__':
+    CoinSelectionTracepointTest().main()

test/functional/test_runner.py

@@ -170,6 +170,7 @@ BASE_SCRIPTS = [
     'wallet_reorgsrestore.py',
     'interface_http.py',
     'interface_rpc.py',
+    'interface_usdt_coinselection.py',
     'interface_usdt_net.py',
     'interface_usdt_utxocache.py',
     'interface_usdt_validation.py',