bench: fix benchmark fixtures and setup checks #35124

1. 
   l0rinc commented at 2:48 PM on April 20, 2026: contributor

   Context

   Found while reviewing #35018, which led to a few additional benchmark fixes.

   Problem

   MempoolCheckEphemeralSpends populated every child input by writing to vin[0]. That left only one child input pointing at the parent transaction, while the remaining inputs kept default prevouts.

   WalletBalanceMine duplicated WalletBalanceClean exactly.

   The same review also showed that nanobench setup() can be misused as though it runs once per timed call. It actually runs once per epoch, so using it with multiple epoch iterations can silently leave later iterations with different preconditions.

   Fix

   Write each ephemeral-spend prevout to the matching child input and assert that the last input spends the last parent output. Remove the duplicate wallet balance benchmark registration. Add a nanobench assertion that setup() is only used with epochIterations(1).

   [!NOTE] This PR included a few benchmark adjustments originally which made the tests unstable and were reverted.

2. DrahtBot added the label Tests on Apr 20, 2026
3. 
   DrahtBot commented at 2:49 PM on April 20, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	sedited, davidgumberg
   Stale ACK	achow101

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. 
   achow101 commented at 7:35 PM on April 20, 2026: member

   ACK b33375459764a3cc716723d596164299dd8c6e22

5. 
   sedited commented at 12:08 PM on April 23, 2026: contributor

   Does this improve anything that should be observable? On current master the benchmarks seem very reproducible to me, while if I run this patch I get the following warning every so often:

   |                1.09 |      914,285,714.29 |   12.5% |      0.00 | :wavy_dash: `CHACHA20_64BYTES` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
   

6. l0rinc force-pushed on Apr 23, 2026
7. l0rinc renamed this:
   bench: improve stateful benchmark reproducibility
   bench: fix benchmark fixtures and setup checks
   on Apr 23, 2026
8. 
   l0rinc commented at 1:46 PM on April 23, 2026: contributor

   Does this improve anything that should be observable

   We need to do multiple runs inside to stabilize them, let's do that in a follow-up instead - dropped the last commit, rebased, adjusted the PR description and title.

9. 
   davidgumberg commented at 5:53 PM on April 23, 2026: contributor

   I think nanobench should set epochIterations = 1 when setup() is used. It seems wrong to force the caller to configure something when nanobench already knows what it wants and will assert on you if you don't do it, could be done maybe e.g.:

   diff --git a/src/bench/nanobench.h b/src/bench/nanobench.h
   index 12a03bc42d..27307700c9 100644
   --- a/src/bench/nanobench.h
   +++ b/src/bench/nanobench.h
   @@ -1239,8 +1239,9 @@ public:
        template <typename Op>
        ANKERL_NANOBENCH_NO_SANITIZE("integer")
        Bench& run(Op&& op) {
   -        assert(mBench.epochIterations() == 1 &&
   -               "setup() runs once per epoch, not once per iteration; use epochIterations(1) when setup() must reset state for each timed call");
   +        assert(mBench.epochIterations() == 0 &&
   +               "setup() is not compatible with setting epochIterations since it runs once per epoch, not once per iteration;");
   +        mBench.epochIterations(1);
            return mBench.runImpl(mSetupOp, std::forward<Op>(op));
        }
   

   https://github.com/davidgumberg/bitcoin/commit/f774d594868f19410bf7cfa30526fb7aad141d81

10. bench: drop duplicate balance benchmark

    `WalletBalanceMine` duplicated `WalletBalanceClean` exactly.
    Remove the duplicate registration so the balance benchmark list stays distinct.

    b8b7f896e8
11. bench: fix ephemeral spend inputs

    `MempoolCheckEphemeralSpends` wrote every prevout to `tx2.vin[0]` instead of `tx2.vin[i]`.
    That left only one child input pointing at the parent transaction, while the remaining inputs kept default prevouts.
    
    Write each prevout to `vin[i]` instead.
    Add an assertion that the last child input spends the last parent output.
    
    Co-authored-by: David Gumberg <davidzgumberg@gmail.com>

    ba0078e3bf
12. bench: make `setup()` use single-iteration epochs

    `setup()` in nanobench runs once per epoch, not once per timed call.
    If an epoch executes the benchmark body multiple times, `setup()` can silently leave later iterations with different preconditions.
    
    Make `setup()` force `epochIterations(1)` itself: keep rejecting incompatible larger explicit epoch sizes, but allow existing single-iteration callers such as `-sanity-check`.
    
    With `setup()` handling this centrally, remove the redundant `epochIterations(1)` calls from the benchmarks that use it.
    
    Co-authored-by: David Gumberg <davidzgumberg@gmail.com>

    e6430b2773
13. in src/bench/mempool_ephemeral_spends.cpp:74 in 7a76795028

      70 | @@ -71,6 +71,7 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench)
      71 |      const CTransactionRef tx2_r{MakeTransactionRef(tx2)};
      72 |  
      73 |      AddTx(tx1_r, pool);
      74 | +    assert(tx2_r->vin.back().prevout == COutPoint(parent_txid, tx2_r->vin.size() - 1));
    

    ---

    ---

    

    davidgumberg commented at 5:57 PM on April 23, 2026:

    nanonit: I think this makes more sense as something like:

        assert(tx2_r->vin.back().prevout == COutPoint(parent_txid, tx1_r->vout.size() - 1));
    

    ---

    

    l0rinc commented at 6:55 PM on April 23, 2026:

    Done, thanks, added you as coauthor.

14. l0rinc force-pushed on Apr 23, 2026
15. sedited approved
16. 
    sedited commented at 7:29 PM on April 23, 2026: contributor

    ACK e6430b2773436c65d447fff5e0a326087abc4950

17. DrahtBot requested review from achow101 on Apr 23, 2026
18. 
    davidgumberg commented at 7:47 PM on April 23, 2026: contributor

    crACK https://github.com/bitcoin/bitcoin/commit/e6430b2773436c65d447fff5e0a326087abc4950

19. fanquake merged this on Apr 24, 2026
20. fanquake closed this on Apr 24, 2026

    ---

21. l0rinc deleted the branch on Apr 24, 2026

Contributors

l0rincDrahtBotachow101sediteddavidgumberg

Review Requested
achow101