bench: add WalletBalanceManySpent for high-history wallet scenario #34360

pull w0xlt wants to merge 1 commits into bitcoin:master from w0xlt:wallet-balance-many-spent-bench-3 changing 2 files +192 −6
  1. w0xlt commented at 3:15 AM on January 21, 2026: contributor

    This PR introduces a new benchmark that measures GetBalance() performance on wallets with extensive transaction history where the majority of outputs have already been spent.

    It was created to test #27865, which separates spent TXOs from spendable ones. This pattern mirrors real-world usage in high-activity wallets such as Lightning nodes, exchanges, hot wallets, and payment processors.

    Setup:

    • Creates 500 blocks × 100 coinbase outputs = 50,000 wallet TXOs
    • Matures all coinbases with COINBASE_MATURITY additional blocks
    • Spends 49,950 outputs in batched transactions (confirmed in blocks)
    • Consolidation outputs go to a burn script to avoid inflating wallet TXO count

    To keep the setup time reasonable, the benchmark uses the fast “fake block” approach, similar to wallet_create_tx.cpp.

  2. bench: add WalletBalanceManySpent for high-history wallet scenario
    Add a benchmark that measures `GetBalance()` performance on wallets with
    extensive transaction history where most outputs have been spent.
    
    Scenario: 50,000 TXOs with 49,950 spent and 50 unspent.
    
    This pattern is common in high-activity wallets (Lightning nodes, exchange
    hot wallets, payment processors) and is the target scenario for optimizations
    that separate spent TXOs from spendable ones.
    
    Setup:
    - Creates 500 blocks × 100 coinbase outputs = 50,000 wallet TXOs
    - Matures all coinbases with COINBASE_MATURITY additional blocks
    - Spends 49,950 outputs in batched transactions (confirmed in blocks)
    - Consolidation outputs go to a burn script to avoid inflating wallet TXO count
    
    Introduces `src/bench/wallet_bench_util.h` with utilities for wallet
    benchmarks (TipBlock, GetTip, GenerateFakeBlock).
    42805a1609
  3. DrahtBot added the label Tests on Jan 21, 2026
  4. DrahtBot commented at 3:15 AM on January 21, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34360.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34858 (test: Use NodeClockContext in more tests by maflcko)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1) in src/bench/wallet_balance.cpp
    • GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1, 50 * COIN, block_txs) in src/bench/wallet_balance.cpp

    <sup>2026-01-21</sup>

  5. fanquake commented at 2:13 PM on March 3, 2026: member

    cc @achow101 given this is inspired by #27865.

  6. sedited requested review from davidgumberg on Mar 19, 2026
  7. sedited requested review from davidgumberg on Mar 19, 2026
  8. DrahtBot commented at 11:03 PM on April 9, 2026: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

  9. DrahtBot added the label Needs rebase on Apr 9, 2026
  10. sedited commented at 10:44 AM on May 14, 2026: contributor

    @w0xlt can you rebase this?

  11. in src/bench/wallet_balance.cpp:153 in 42805a1609
     148 | +    for (size_t i = 0; i < spending_txs.size(); i += TXS_PER_BLOCK) {
     149 | +        std::vector<CTransactionRef> block_txs;
     150 | +        for (size_t j = i; j < std::min(i + TXS_PER_BLOCK, spending_txs.size()); ++j) {
     151 | +            block_txs.push_back(spending_txs[j]);
     152 | +        }
     153 | +        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1, 50 * COIN, block_txs);
    


    l0rinc commented at 10:44 AM on May 15, 2026:

    For clarity on what the unnamed primitive arguments mean, please consider adding name hints:

            GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, /*num_coinbase_outputs=*/1, /*total_coinbase_value=*/50 * COIN, block_txs);
    

    or even better, we could extract these as suggested in https://github.com/bitcoin/bitcoin/pull/34360/changes#r3247900786 and use them as

            GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, FILLER_OUTPUTS, FILLER_VALUE, block_txs);
    
  12. in src/bench/wallet_balance.cpp:159 in 42805a1609
     154 | +    }
     155 | +
     156 | +    // Phase 5: Run the benchmark
     157 | +    auto bal = GetBalance(wallet); // Cache
     158 | +
     159 | +    bench.run([&] {
    


    l0rinc commented at 10:59 AM on May 15, 2026:

    we could make the benchmark more stable and avoid:

    :wavy_dash: WalletBalanceManySpent (Unstable with ~1.0 iters. Increase minEpochIterations to e.g. 10)

    by specifying the minimum iteration count, something like:

        bench.minEpochIterations(10).run([&] {
    
  13. in src/bench/wallet_balance.cpp:122 in 42805a1609
     117 | +        }
     118 | +    }
     119 | +
     120 | +    // Keep ~50 outputs unspent, spend the rest
     121 | +    constexpr int KEEP_UNSPENT = 50;
     122 | +    int num_to_spend = std::max(0, static_cast<int>(outputs_to_spend.size()) - KEEP_UNSPENT);
    


    l0rinc commented at 11:24 AM on May 15, 2026:

    Not sure how to interpret this, why would we want to run a benchmark with 0? The description of the PR claims we have 50k total outputs, we could reserve and assert that for clarity:

    constexpr size_t TOTAL_WALLET_OUTPUTS{NUM_BLOCKS * OUTPUTS_PER_BLOCK};
    ...
    std::vector<COutPoint> outputs_to_spend;
    outputs_to_spend.reserve(TOTAL_WALLET_OUTPUTS);
    ...
    constexpr size_t KEEP_UNSPENT{50};
    assert(outputs_to_spend.size() == TOTAL_WALLET_OUTPUTS);
    const size_t num_to_spend{outputs_to_spend.size() - KEEP_UNSPENT};
    
  14. in src/bench/wallet_balance.cpp:138 in 42805a1609
     133 | +        for (int j = i; j < batch_end; ++j) {
     134 | +            spend_tx.vin.emplace_back(outputs_to_spend[j]);
     135 | +        }
     136 | +
     137 | +        // Send to burn script so consolidation outputs don't add to wallet TXOs
     138 | +        CAmount total_value = (batch_end - i) * OUTPUT_VALUE;
    


    l0rinc commented at 11:26 AM on May 15, 2026:

    nit: this basically duplicates the logic of the previous loop - alternatively it may be simpler to understand it if we reused the logic:

            CAmount total_value{0};
            for (size_t j{i}; j < batch_end; ++j) {
                spend_tx.vin.emplace_back(outputs_to_spend[j]);
                total_value += OUTPUT_VALUE;
            }
    
            // Send to burn script so consolidation outputs don't add to wallet TXOs
    
  15. in src/bench/wallet_balance.cpp:126 in 42805a1609
     121 | +    constexpr int KEEP_UNSPENT = 50;
     122 | +    int num_to_spend = std::max(0, static_cast<int>(outputs_to_spend.size()) - KEEP_UNSPENT);
     123 | +
     124 | +    // Create spending transactions in batches of 100 inputs each
     125 | +    constexpr int BATCH_SIZE = 100;
     126 | +    constexpr CAmount BENCHMARK_FEE = 1000;
    


    l0rinc commented at 11:37 AM on May 15, 2026:

    these intertwined constant definitions kinda' break the abstraction for me: the point of defining them is that we shouldn't care about their exact value when we're using them. If you agree, please consider extracting the constant initializations either at the beginning or as fields (and maybe use brace init to avoid narrowing and weird type conversions). That would separate the concerns: when we're interested in the configuration of the benchmark (the values used) we have that concern in one dedicated place and when we're interested in the algorithm (where we want to avoid knowing the implementation details) we can just rely on the config names:

    static void WalletBalanceManySpent(benchmark::Bench& bench)
    {
        constexpr int NUM_BLOCKS{500};
        constexpr int OUTPUTS_PER_BLOCK{100};
        constexpr CAmount OUTPUT_VALUE{50 * COIN / OUTPUTS_PER_BLOCK};
        constexpr size_t TOTAL_WALLET_OUTPUTS{NUM_BLOCKS * OUTPUTS_PER_BLOCK};
        constexpr size_t KEEP_UNSPENT{50};
        constexpr size_t BATCH_SIZE{100};
        constexpr CAmount FEE{1000};
        constexpr size_t TXS_PER_BLOCK{10};
        constexpr int FILLER_OUTPUTS{1};
        constexpr CAmount FILLER_VALUE{50 * COIN};
    ...
    
  16. in src/bench/wallet_balance.cpp:104 in 42805a1609
      99 | +    for (int i = 0; i < NUM_BLOCKS; ++i) {
     100 | +        GenerateFakeBlock(params, test_setup->m_node, wallet, wallet_script, OUTPUTS_PER_BLOCK);
     101 | +    }
     102 | +
     103 | +    // Phase 2: Add COINBASE_MATURITY blocks to mature all coinbase outputs
     104 | +    // This ensures all 50,000 TXOs are spendable
    


    l0rinc commented at 11:38 AM on May 15, 2026:

    nit: these comments lock in the values of the configuration options above. Code comments are meant to add extra context that the code can't easily explain. Here, it's partially duplicating the code. Please consider deduplicating the values from their meanings:

        // Target: many spent TXOs, few unspent TXOs.
    

    and

        // Phase 1: Create the configured wallet-owned output history.
    

    and

        // Keep the configured small remainder unspent, and spend the rest.
    

    etc.

  17. in src/bench/wallet_balance.cpp:162 in 42805a1609
     157 | +    auto bal = GetBalance(wallet); // Cache
     158 | +
     159 | +    bench.run([&] {
     160 | +        wallet.MarkDirty();
     161 | +        bal = GetBalance(wallet);
     162 | +        assert(bal.m_mine_trusted > 0);
    


    l0rinc commented at 11:41 AM on May 15, 2026:

    This would still pass if setup accidentally left more spendable wallet outputs than intended. Since the benchmark claims only the configured remainder stays unspent, could we assert the exact trusted balance?

    CAmount expected_balance{0};
    for (size_t i{0}; i < KEEP_UNSPENT; ++i) {
        expected_balance += OUTPUT_VALUE;
    }
    ...
    assert(bal.m_mine_trusted == expected_balance);
    assert(bal.m_mine_immature == 0);
    
  18. in src/bench/wallet_balance.cpp:157 in 42805a1609
     152 | +        }
     153 | +        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1, 50 * COIN, block_txs);
     154 | +    }
     155 | +
     156 | +    // Phase 5: Run the benchmark
     157 | +    auto bal = GetBalance(wallet); // Cache
    


    l0rinc commented at 11:45 AM on May 15, 2026:

    Why is this needed exactly? Wouldn't a bench.warmup(1) achieve the same? That would also allow us to define bal inside:

    bench.warmup(1).minEpochIterations(128).run([&] {
        const auto bal{GetBalance(wallet)};
        assert(bal.m_mine_trusted == expected_balance);
        assert(bal.m_mine_immature == 0);
    });
    

    Though it kinda' begs the question: don't we want to measure the uncached state at all? Is caching referring to an internal state, i.e. the first iteration being different from the rest?

  19. in src/bench/wallet_balance.cpp:160 in 42805a1609
     155 | +
     156 | +    // Phase 5: Run the benchmark
     157 | +    auto bal = GetBalance(wallet); // Cache
     158 | +
     159 | +    bench.run([&] {
     160 | +        wallet.MarkDirty();
    


    l0rinc commented at 11:58 AM on May 15, 2026:

    Is this really needed here? It walks mapWallet and invalidates per-transaction caches, while this confirmed-output balance path iterates wallet.GetTXOs(), checks spend status, and reads each remaining output value. The benchmarks indicate the performance is also similar (except for the extra iteration):

    with MarkDirty:

    | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 2,927,937.88 | 341.54 | 0.3% | 11.01 | WalletBalanceManySpent

    No MarkDirty:

    | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 2,897,842.24 | 345.08 | 0.2% | 11.01 | WalletBalanceManySpent

  20. in src/bench/wallet_balance.cpp:155 in 42805a1609
     150 | +        for (size_t j = i; j < std::min(i + TXS_PER_BLOCK, spending_txs.size()); ++j) {
     151 | +            block_txs.push_back(spending_txs[j]);
     152 | +        }
     153 | +        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1, 50 * COIN, block_txs);
     154 | +    }
     155 | +
    


    l0rinc commented at 12:06 PM on May 15, 2026:

    My understanding is that balance alone does not prove that spent wallet outputs remain in history, consider adding a sanity check here (should probably be done via a helper method to make it less noisy):

    size_t owned_outputs{0}, spent_outputs{0};
    {
        LOCK(wallet.cs_wallet);
        for (const auto& [outpoint, _] : wallet.GetTXOs()) {
            ++owned_outputs;
            spent_outputs += wallet.IsSpent(outpoint);
        }
    }
    assert(owned_outputs == TOTAL_WALLET_OUTPUTS);
    assert(spent_outputs == TOTAL_WALLET_OUTPUTS - KEEP_UNSPENT);
    
  21. in src/bench/wallet_balance.cpp:80 in 42805a1609
      75 | +    //
      76 | +    // Target: ~50,000 spent TXOs, ~50 unspent TXOs
      77 | +    const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
      78 | +    const auto& params = test_setup->m_node.chainman->GetParams();
      79 | +
      80 | +    SetMockTime(params.GenesisBlock().nTime);
    


    l0rinc commented at 12:18 PM on May 15, 2026:

    We could use NodeClockContext here instead, like the adjacent wallet balance benchmark, so the mocked clock is restored automatically when setup leaves scope (see https://github.com/bitcoin/bitcoin/pull/34858/changes#diff-242a3d315a6189295f7169824284383659b1d83ac65241351a930348d5b8ee82L10-R39):

        NodeClockContext clock_ctx{params.GenesisBlock().Time()};
    

    cc: @maflcko

  22. in src/bench/wallet_balance.cpp:118 in 42805a1609
     113 | +        LOCK(wallet.cs_wallet);
     114 | +        auto available = AvailableCoins(wallet);
     115 | +        for (const auto& coin : available.All()) {
     116 | +            outputs_to_spend.push_back(coin.outpoint);
     117 | +        }
     118 | +    }
    


    l0rinc commented at 12:23 PM on May 15, 2026:

    We could avoid copying the 50k COutput entries (and avoid resizing) before the spend phase by iterating the existing AvailableCoins result directly:

        std::vector<COutPoint> outputs_to_spend;
        outputs_to_spend.reserve(TOTAL_WALLET_OUTPUTS);
        {
            LOCK(wallet.cs_wallet);
            for (const auto& [_, coins] : AvailableCoins(wallet).coins) {
                for (const auto& coin : coins) {
                    outputs_to_spend.push_back(coin.outpoint);
                }
            }
        }
    
  23. in src/bench/wallet_balance.cpp:152 in 42805a1609
     147 | +    constexpr int TXS_PER_BLOCK = 10;
     148 | +    for (size_t i = 0; i < spending_txs.size(); i += TXS_PER_BLOCK) {
     149 | +        std::vector<CTransactionRef> block_txs;
     150 | +        for (size_t j = i; j < std::min(i + TXS_PER_BLOCK, spending_txs.size()); ++j) {
     151 | +            block_txs.push_back(spending_txs[j]);
     152 | +        }
    


    l0rinc commented at 12:27 PM on May 15, 2026:

    It seems to me we could simplify this continuous-slice copy:

            const size_t batch_end{std::min(i + TXS_PER_BLOCK, spending_txs.size())};
            std::vector<CTransactionRef> block_txs{spending_txs.begin() + i, spending_txs.begin() + batch_end};
    

    Similarly for

        // Add any extra transactions
        for (const auto& tx : extra_txs) {
            block.vtx.push_back(tx);
        }
    

    we can probably use vector::insert for that append instead of spelling out an equivalent push_back loop in the helper:

        block.vtx.insert(block.vtx.end(), extra_txs.begin(), extra_txs.end());
    
  24. in src/bench/wallet_bench_util.h:36 in 42805a1609
      31 | +inline TipBlock GetTip(const CChainParams& params, const node::NodeContext& context)
      32 | +{
      33 | +    auto tip = WITH_LOCK(::cs_main, return context.chainman->ActiveTip());
      34 | +    return (tip) ? TipBlock{tip->GetBlockHash(), tip->GetBlockTime(), tip->nHeight} :
      35 | +           TipBlock{params.GenesisBlock().GetHash(), params.GenesisBlock().GetBlockTime(), 0};
      36 | +}
    


    l0rinc commented at 12:36 PM on May 15, 2026:

    The original code calls ActiveTip() under cs_main, but then dereferences the returned CBlockIndex* after the lock is released. That is probably harmless for these immutable-looking fields in this benchmark, but the helper only needs a snapshot, so it seems clearer to copy the hash/time/height while holding the lock and avoid carrying the tip pointer outside the protected region.

    This also avoids the ternary, which my IDE parses poorly:

    inline TipBlock GetTip(const CChainParams& params, const node::NodeContext& context)
    {
        {
            LOCK(::cs_main);
            if (const auto* tip{context.chainman->ActiveTip()}) {
                return {tip->GetBlockHash(), tip->GetBlockTime(), tip->nHeight};
            }
        }
        return {params.GenesisBlock().GetHash(), params.GenesisBlock().GetBlockTime(), 0};
    }
    
  25. in src/bench/wallet_bench_util.h:84 in 42805a1609
      79 | +        LOCK(::cs_main);
      80 | +        CBlockIndex* pindex{context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header)};
      81 | +        context.chainman->ActiveChain().SetTip(*pindex);
      82 | +    }
      83 | +
      84 | +    const auto* pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip());
    


    l0rinc commented at 12:42 PM on May 15, 2026:

    Do we need to reread the active tip here or could we do it in the previous locked scope instead?

        CBlockIndex* pindex{nullptr};
        {
            LOCK(::cs_main);
            pindex = context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header);
            context.chainman->ActiveChain().SetTip(*pindex);
        }
    
  26. in src/bench/wallet_bench_util.h:48 in 42805a1609
      43 | +                              const node::NodeContext& context,
      44 | +                              CWallet& wallet,
      45 | +                              const CScript& coinbase_script,
      46 | +                              int num_coinbase_outputs,
      47 | +                              CAmount total_coinbase_value = 50 * COIN,
      48 | +                              const std::vector<CTransactionRef>& extra_txs = {})
    


    l0rinc commented at 12:51 PM on May 15, 2026:

    This may be slightly bigger unrelated change (generalizing wallet_create_tx.cpp to use our newly introduced methods), but maybe the coinbase_outputs should be exposed as a single param:

    inline void GenerateFakeBlock(const CChainParams& params,
                                  const node::NodeContext& context,
                                  CWallet& wallet,
                                  const std::vector<CTxOut>& coinbase_outputs,
                                  const std::vector<CTransactionRef>& extra_txs = {})
    

    add a helper delegating to this with the old signature:

    inline void GenerateFakeBlock(const CChainParams& params,
                                  const node::NodeContext& context,
                                  CWallet& wallet,
                                  const CScript& coinbase_script,
                                  int num_coinbase_outputs,
                                  CAmount total_coinbase_value = 50 * COIN,
                                  const std::vector<CTransactionRef>& extra_txs = {})
    {
        std::vector<CTxOut> coinbase_outputs;
        coinbase_outputs.reserve(num_coinbase_outputs);
        const CAmount per_output{total_coinbase_value / num_coinbase_outputs};
        for (int i{0}; i < num_coinbase_outputs; ++i) {
            coinbase_outputs.emplace_back(per_output, coinbase_script);
        }
        GenerateFakeBlock(params, context, wallet, coinbase_outputs, extra_txs);
    }
    

    which would enable us to call it from the benchmark's generateFakeBlock and avoid a lot of duplication between wallet_create_tx.cpp and wallet_bench_util.h.

  27. in src/bench/wallet_bench_util.h:28 in 42805a1609
      23 | +
      24 | +struct TipBlock
      25 | +{
      26 | +    uint256 prev_block_hash;
      27 | +    int64_t prev_block_time;
      28 | +    int tip_height;
    


    l0rinc commented at 12:53 PM on May 15, 2026:

    nit: we could initialize the fields so static analysis in my IDE stops complaining:

        uint256 prev_block_hash{};
        int64_t prev_block_time{0};
        int tip_height{0};
    

    Just resolve if you don't think it's important.

  28. l0rinc changes_requested
  29. l0rinc commented at 1:13 PM on May 15, 2026: contributor

    I might have gone a bit overboard again, I left comments documenting my review journey, please see the individual explanations - feel free to pick and choose between them.

    For convenience here's my local patch against your rebased branch:

    <details><summary>Local pending changes</summary>

    diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp
    index ee8244501a..7ce15529d6 100644
    --- a/src/bench/wallet_balance.cpp
    +++ b/src/bench/wallet_balance.cpp
    @@ -18,6 +18,7 @@
     #include <wallet/wallet.h>
     
     #include <cassert>
    +#include <cstddef>
     #include <memory>
     #include <optional>
     #include <string>
    @@ -72,15 +73,26 @@ BENCHMARK(WalletBalanceWatch);
     
     static void WalletBalanceManySpent(benchmark::Bench& bench)
     {
    +    constexpr int NUM_BLOCKS{500};
    +    constexpr int OUTPUTS_PER_BLOCK{100};
    +    constexpr CAmount OUTPUT_VALUE{50 * COIN / OUTPUTS_PER_BLOCK};
    +    constexpr size_t TOTAL_WALLET_OUTPUTS{NUM_BLOCKS * OUTPUTS_PER_BLOCK};
    +    constexpr size_t KEEP_UNSPENT{50};
    +    constexpr size_t BATCH_SIZE{100};
    +    constexpr CAmount FEE{1000};
    +    constexpr size_t TXS_PER_BLOCK{10};
    +    constexpr int FILLER_OUTPUTS{1};
    +    constexpr CAmount FILLER_VALUE{50 * COIN};
    +
         // Benchmark GetBalance with many spent TXOs and few unspent TXOs.
         // This scenario benefits from optimizations that separate definitely-spent
         // outputs from potentially-spendable ones (such as m_unusable_txos).
         //
    -    // Target: ~50,000 spent TXOs, ~50 unspent TXOs
    +    // Target: many spent TXOs, few unspent TXOs
         const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
         const auto& params = test_setup->m_node.chainman->GetParams();
     
    -    SetMockTime(params.GenesisBlock().nTime);
    +    NodeClockContext clock_ctx{params.GenesisBlock().Time()};
         CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockableWalletDatabase()};
         {
             LOCK(wallet.cs_wallet);
    @@ -91,55 +103,51 @@ static void WalletBalanceManySpent(benchmark::Bench& bench)
     
         const auto dest = getNewDestination(wallet, OutputType::BECH32);
         const CScript wallet_script = GetScriptForDestination(dest);
    -    // Burn script for outputs we don't want in the wallet
    +    // OP_TRUE keeps the output spendable without adding it to the wallet
         const CScript burn_script{CScript() << OP_TRUE};
     
    -    // Phase 1: Create many UTXOs (100 outputs per block * 500 blocks = 50,000 TXOs)
    -    constexpr int NUM_BLOCKS = 500;
    -    constexpr int OUTPUTS_PER_BLOCK = 100;
    -    constexpr CAmount OUTPUT_VALUE = 50 * COIN / OUTPUTS_PER_BLOCK;
    -
    +    // Phase 1: Create the configured wallet-owned output history
         for (int i = 0; i < NUM_BLOCKS; ++i) {
             GenerateFakeBlock(params, test_setup->m_node, wallet, wallet_script, OUTPUTS_PER_BLOCK);
         }
     
         // Phase 2: Add COINBASE_MATURITY blocks to mature all coinbase outputs
    -    // This ensures all 50,000 TXOs are spendable
         for (int i = 0; i < COINBASE_MATURITY; ++i) {
    -        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1);
    +        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, FILLER_OUTPUTS);
         }
     
         // Phase 3: Spend most UTXOs by creating spending transactions
         // Get all available coins and spend them in batches
         std::vector<COutPoint> outputs_to_spend;
    +    outputs_to_spend.reserve(TOTAL_WALLET_OUTPUTS);
         {
             LOCK(wallet.cs_wallet);
    -        auto available = AvailableCoins(wallet);
    -        for (const auto& coin : available.All()) {
    -            outputs_to_spend.push_back(coin.outpoint);
    +        for (const auto& [_, coins] : AvailableCoins(wallet).coins) {
    +            for (const auto& coin : coins) {
    +                outputs_to_spend.push_back(coin.outpoint);
    +            }
             }
         }
     
    -    // Keep ~50 outputs unspent, spend the rest
    -    constexpr int KEEP_UNSPENT = 50;
    -    int num_to_spend = std::max(0, static_cast<int>(outputs_to_spend.size()) - KEEP_UNSPENT);
    +    // Keep the configured small remainder unspent, and spend the rest
    +    assert(outputs_to_spend.size() == TOTAL_WALLET_OUTPUTS);
    +    const size_t num_to_spend{outputs_to_spend.size() - KEEP_UNSPENT};
     
    -    // Create spending transactions in batches of 100 inputs each
    -    constexpr int BATCH_SIZE = 100;
    -    constexpr CAmount BENCHMARK_FEE = 1000;
    +    // Create spending transactions in configured-size batches
         std::vector<CTransactionRef> spending_txs;
     
    -    for (int i = 0; i < num_to_spend; i += BATCH_SIZE) {
    +    for (size_t i{0}; i < num_to_spend; i += BATCH_SIZE) {
             CMutableTransaction spend_tx;
    -        int batch_end = std::min(i + BATCH_SIZE, num_to_spend);
    +        const size_t batch_end{std::min(i + BATCH_SIZE, num_to_spend)};
     
    -        for (int j = i; j < batch_end; ++j) {
    +        CAmount total_value{0};
    +        for (size_t j{i}; j < batch_end; ++j) {
                 spend_tx.vin.emplace_back(outputs_to_spend[j]);
    +            total_value += OUTPUT_VALUE;
             }
     
             // Send to burn script so consolidation outputs don't add to wallet TXOs
    -        CAmount total_value = (batch_end - i) * OUTPUT_VALUE;
    -        spend_tx.vout.emplace_back(total_value - BENCHMARK_FEE, burn_script);
    +        spend_tx.vout.emplace_back(total_value - FEE, burn_script);
     
             spending_txs.push_back(MakeTransactionRef(std::move(spend_tx)));
         }
    @@ -147,22 +155,33 @@ static void WalletBalanceManySpent(benchmark::Bench& bench)
         // Phase 4: Confirm all spending transactions in new blocks
         // Include multiple spending txs per block for efficiency
         // Coinbase outputs go to burn script to avoid adding wallet TXOs
    -    constexpr int TXS_PER_BLOCK = 10;
    -    for (size_t i = 0; i < spending_txs.size(); i += TXS_PER_BLOCK) {
    -        std::vector<CTransactionRef> block_txs;
    -        for (size_t j = i; j < std::min(i + TXS_PER_BLOCK, spending_txs.size()); ++j) {
    -            block_txs.push_back(spending_txs[j]);
    +    for (size_t i{0}; i < spending_txs.size(); i += TXS_PER_BLOCK) {
    +        const size_t batch_end{std::min(i + TXS_PER_BLOCK, spending_txs.size())};
    +        std::vector<CTransactionRef> block_txs{spending_txs.begin() + i, spending_txs.begin() + batch_end};
    +        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, FILLER_OUTPUTS, FILLER_VALUE, block_txs);
    +    }
    +
    +    // Balance alone does not prove that spent wallet outputs remain in history.
    +    size_t owned_outputs{0}, spent_outputs{0};
    +    {
    +        LOCK(wallet.cs_wallet);
    +        for (const auto& [outpoint, _] : wallet.GetTXOs()) {
    +            ++owned_outputs;
    +            spent_outputs += wallet.IsSpent(outpoint);
             }
    -        GenerateFakeBlock(params, test_setup->m_node, wallet, burn_script, 1, 50 * COIN, block_txs);
         }
    +    assert(owned_outputs == TOTAL_WALLET_OUTPUTS);
    +    assert(spent_outputs == TOTAL_WALLET_OUTPUTS - KEEP_UNSPENT);
     
    -    // Phase 5: Run the benchmark
    -    auto bal = GetBalance(wallet); // Cache
    +    CAmount expected_balance{0};
    +    for (size_t i{0}; i < KEEP_UNSPENT; ++i) {
    +        expected_balance += OUTPUT_VALUE;
    +    }
     
    -    bench.run([&] {
    -        wallet.MarkDirty();
    -        bal = GetBalance(wallet);
    -        assert(bal.m_mine_trusted > 0);
    +    bench.minEpochIterations(10).run([&] {
    +        const auto bal{GetBalance(wallet)};
    +        assert(bal.m_mine_trusted == expected_balance);
    +        assert(bal.m_mine_immature == 0);
         });
     }
     
    diff --git a/src/bench/wallet_bench_util.h b/src/bench/wallet_bench_util.h
    index 7c7ebbb1d5..edc1bc15e3 100644
    --- a/src/bench/wallet_bench_util.h
    +++ b/src/bench/wallet_bench_util.h
    @@ -19,20 +19,26 @@
     #include <versionbits.h>
     #include <wallet/wallet.h>
     
    +#include <vector>
    +
     namespace wallet {
     
     struct TipBlock
     {
    -    uint256 prev_block_hash;
    -    int64_t prev_block_time;
    -    int tip_height;
    +    uint256 prev_block_hash{};
    +    int64_t prev_block_time{0};
    +    int tip_height{0};
     };
     
     inline 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};
    +    {
    +        LOCK(::cs_main);
    +        if (const auto* tip{context.chainman->ActiveTip()}) {
    +            return {tip->GetBlockHash(), tip->GetBlockTime(), tip->nHeight};
    +        }
    +    }
    +    return {params.GenesisBlock().GetHash(), params.GenesisBlock().GetBlockTime(), 0};
     }
     
     /**
    @@ -42,9 +48,7 @@ inline TipBlock GetTip(const CChainParams& params, const node::NodeContext& cont
     inline void GenerateFakeBlock(const CChainParams& params,
                                   const node::NodeContext& context,
                                   CWallet& wallet,
    -                              const CScript& coinbase_script,
    -                              int num_coinbase_outputs,
    -                              CAmount total_coinbase_value = 50 * COIN,
    +                              const std::vector<CTxOut>& coinbase_outputs,
                                   const std::vector<CTransactionRef>& extra_txs = {})
     {
         TipBlock tip{GetTip(params, context)};
    @@ -54,19 +58,11 @@ inline void GenerateFakeBlock(const CChainParams& params,
         coinbase_tx.vin.resize(1);
         coinbase_tx.vin[0].prevout.SetNull();
         coinbase_tx.vin[0].scriptSig = CScript() << ++tip.tip_height << OP_0;
    -
    -    // Create coinbase outputs
    -    CAmount per_output = total_coinbase_value / num_coinbase_outputs;
    -    for (int i = 0; i < num_coinbase_outputs; ++i) {
    -        coinbase_tx.vout.emplace_back(per_output, coinbase_script);
    -    }
    +    coinbase_tx.vout = coinbase_outputs;
     
         block.vtx = {MakeTransactionRef(std::move(coinbase_tx))};
     
    -    // Add any extra transactions
    -    for (const auto& tx : extra_txs) {
    -        block.vtx.push_back(tx);
    -    }
    +    block.vtx.insert(block.vtx.end(), extra_txs.begin(), extra_txs.end());
     
         block.nVersion = VERSIONBITS_LAST_OLD_BLOCK_VERSION;
         block.hashPrevBlock = tip.prev_block_hash;
    @@ -75,16 +71,33 @@ inline void GenerateFakeBlock(const CChainParams& params,
         block.nBits = params.GenesisBlock().nBits;
         block.nNonce = 0;
     
    +    CBlockIndex* pindex{nullptr};
         {
             LOCK(::cs_main);
    -        CBlockIndex* pindex{context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header)};
    +        pindex = context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header);
             context.chainman->ActiveChain().SetTip(*pindex);
         }
     
    -    const auto* pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip());
         wallet.blockConnected(kernel::ChainstateRole{}, kernel::MakeBlockInfo(pindex, &block));
     }
     
    +inline void GenerateFakeBlock(const CChainParams& params,
    +                              const node::NodeContext& context,
    +                              CWallet& wallet,
    +                              const CScript& coinbase_script,
    +                              int num_coinbase_outputs,
    +                              CAmount total_coinbase_value = 50 * COIN,
    +                              const std::vector<CTransactionRef>& extra_txs = {})
    +{
    +    std::vector<CTxOut> coinbase_outputs;
    +    coinbase_outputs.reserve(num_coinbase_outputs);
    +    const CAmount per_output{total_coinbase_value / num_coinbase_outputs};
    +    for (int i{0}; i < num_coinbase_outputs; ++i) {
    +        coinbase_outputs.emplace_back(per_output, coinbase_script);
    +    }
    +    GenerateFakeBlock(params, context, wallet, coinbase_outputs, extra_txs);
    +}
    +
     } // namespace wallet
     
     #endif // BITCOIN_BENCH_WALLET_BENCH_UTIL_H
    diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp
    index 11125e26fe..309c950671 100644
    --- a/src/bench/wallet_create_tx.cpp
    +++ b/src/bench/wallet_create_tx.cpp
    @@ -4,28 +4,20 @@
     
     #include <addresstype.h>
     #include <bench/bench.h>
    -#include <chain.h>
    +#include <bench/wallet_bench_util.h>
     #include <chainparams.h>
     #include <consensus/amount.h>
     #include <consensus/consensus.h>
    -#include <consensus/merkle.h>
     #include <interfaces/chain.h>
    -#include <kernel/chain.h>
    -#include <kernel/types.h>
    -#include <node/blockstorage.h>
     #include <outputtype.h>
     #include <policy/feerate.h>
    -#include <primitives/block.h>
     #include <primitives/transaction.h>
     #include <script/script.h>
     #include <sync.h>
     #include <test/util/setup_common.h>
     #include <test/util/time.h>
    -#include <uint256.h>
     #include <util/result.h>
     #include <util/time.h>
    -#include <validation.h>
    -#include <versionbits.h>
     #include <wallet/coincontrol.h>
     #include <wallet/coinselection.h>
     #include <wallet/spend.h>
    @@ -41,70 +33,28 @@
     #include <utility>
     #include <vector>
     
    -using kernel::ChainstateRole;
     using wallet::CWallet;
     using wallet::CreateMockableWalletDatabase;
     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 = 48 * 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;
    +    constexpr int NON_WALLET_OUTPUTS{50};
    +    std::vector<CTxOut> coinbase_outputs;
    +    coinbase_outputs.reserve(2 + NON_WALLET_OUTPUTS);
    +    coinbase_outputs.emplace_back(48 * COIN, coinbase_out_script);
    +    coinbase_outputs.emplace_back(1 * COIN, coinbase_out_script);
     
         // Fill the coinbase with outputs that don't belong to the wallet in order to benchmark
         // AvailableCoins' behavior with unnecessary TXOs
    -    for (int i = 0; i < 50; ++i) {
    -        coinbase_tx.vout.emplace_back(1 * COIN / 50, CScript(OP_TRUE));
    -    }
    -
    -    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
    -        CBlockIndex* pindex{context.chainman->m_blockman.AddToBlockIndex(block, context.chainman->m_best_header)};
    -        // add it to the chain
    -        context.chainman->ActiveChain().SetTip(*pindex);
    +    for (int i{0}; i < NON_WALLET_OUTPUTS; ++i) {
    +        coinbase_outputs.emplace_back(1 * COIN / NON_WALLET_OUTPUTS, CScript(OP_TRUE));
         }
     
    -    // notify wallet
    -    const auto& pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip());
    -    wallet.blockConnected(ChainstateRole{}, kernel::MakeBlockInfo(pindex, &block));
    +    wallet::GenerateFakeBlock(params, context, wallet, coinbase_outputs);
     }
     
     struct PreSelectInputs {
    

    </details>


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:52 UTC