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

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.

DrahtBot added the label Tests on Jan 21, 2026

DrahtBot commented at 3:15 AM on January 21, 2026: contributor

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

Code Coverage & Benchmarks

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35414 (iwyu: Fix warnings in src/bench and treat them as error by hebasto)
#34698 (wallet: handle MiniMiner bump fee calculation failures by shuv-amp)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)

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.

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

cc @achow101 given this is inspired by #27865.

sedited requested review from davidgumberg on Mar 19, 2026

DrahtBot added the label Needs rebase on Apr 9, 2026

sedited commented at 10:44 AM on May 14, 2026: contributor

@w0xlt can you rebase this?

in src/bench/wallet_balance.cpp:153 in 42805a1609 outdated

 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);

w0xlt commented at 2:08 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:159 in 42805a1609 outdated

 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([&] {

w0xlt commented at 2:08 AM on May 22, 2026:

Thanks. Done

l0rinc commented at 3:15 PM on May 22, 2026:

Does minEpochIterations(128) addresses benchmark stability better than the suggested 10?

in src/bench/wallet_balance.cpp:122 in 42805a1609 outdated

 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};

w0xlt commented at 2:08 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:138 in 42805a1609 outdated

 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

w0xlt commented at 2:08 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:126 in 42805a1609 outdated

 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};
...

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:104 in 42805a1609 outdated

  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.

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:162 in 42805a1609 outdated

 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);

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:157 in 42805a1609 outdated

 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?

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:160 in 42805a1609 outdated

 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

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:155 in 42805a1609 outdated

 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);

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:80 in 42805a1609 outdated

  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

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:118 in 42805a1609 outdated

 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);
            }
        }
    }

w0xlt commented at 2:07 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_balance.cpp:152 in 42805a1609 outdated

 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());

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_bench_util.h:36 in 42805a1609 outdated

  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};
}

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_bench_util.h:84 in 42805a1609 outdated

  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);
    }

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_bench_util.h:48 in 42805a1609 outdated

  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.

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

in src/bench/wallet_bench_util.h:28 in 42805a1609 outdated

  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.

w0xlt commented at 2:06 AM on May 22, 2026:

Thanks. Done

l0rinc changes_requested

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>

w0xlt force-pushed on May 22, 2026

bench: share wallet fake block generation

Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

3957b47fac

w0xlt force-pushed on May 22, 2026

DrahtBot added the label CI failed on May 22, 2026

DrahtBot removed the label CI failed on May 22, 2026

bench: add WalletBalanceManySpent

Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

5defb96252

w0xlt force-pushed on May 22, 2026

w0xlt commented at 2:10 AM on May 22, 2026: contributor

@l0rinc Thanks for the thorough review. I split the changes into two commits to make them easier to review, and added you as a co-author on both since many of the improvements came from your suggestions.

w0xlt commented at 2:17 AM on May 22, 2026: contributor

I've re-run this benchmark to validate bitcoin/bitcoin#27865, which changes how wallet TXOs are retained for balance calculation.

Command used:

build/bin/bench_bitcoin -filter=WalletBalanceManySpent -min-time=10000

Results:

branch	median	notes
current master	8,861,448 ns/op	higher variance in this run, ~17% error
#27865	2,973 ns/op	lower variance in this run, ~0.8% error

This is about a 2,981x speedup with #27865 for this workload.

The benchmark's purpose is to model a wallet with many spent TXOs and only a few unspent TXOs, then measure GetBalance(). On current master, the balance path still has to deal with the large spent-output history.

With #27865, the balance path only sees the relevant unspent outputs, which is the performance improvement this benchmark is intended to demonstrate.

w0xlt commented at 2:22 AM on May 22, 2026: contributor

@w0xlt can you rebase this? @sedited Done.

DrahtBot removed the label Needs rebase on May 22, 2026

sedited requested review from l0rinc on May 22, 2026

in src/bench/wallet_balance.cpp:176 in 5defb96252

 171 | +    }
 172 | +    if (owned_outputs == TOTAL_WALLET_OUTPUTS) {
 173 | +        assert(spent_outputs == TOTAL_WALLET_OUTPUTS - KEEP_UNSPENT);
 174 | +    } else {
 175 | +        assert(owned_outputs == KEEP_UNSPENT);
 176 | +        assert(spent_outputs == 0);

l0rinc commented at 3:17 PM on May 22, 2026:

nit: Is this meant to support running the benchmark on top of #27865? On this PR’s current base the else branch should be dead code, so a short comment explaining the forward-compatibility case would help.

l0rinc commented at 3:20 PM on May 22, 2026: contributor

ACK 5defb96252e0b955d1a3863f78a87ca064d9071a

Thanks for taking the suggestions. I see you've made it forward-compatible - I'm usually more strict about adding dead code, but in this case I personally don't mind. If you touch this again, either remove it and let the other PR extend the benchmark (it likely needs other changes anyway) or add a comment explaining it. I'm also fine with just adding a note to the PR's description.