fuzz: Extend mini_miner fuzz coverage to max block weight #31803

1. 
   fjahr commented at 4:46 pm on February 5, 2025: contributor

   This is follow-up to #31384 which should be merged shortly. Only the last commit is new.

   It expands the fuzz test to cover the full range of allow block sizes in BlockAssembler by utilizing the block_adjusted_max_weight option if necessary.

   See also the brief discussion here for context: https://github.com/bitcoin/bitcoin/pull/31384/files#r1942039833

2. 
   DrahtBot commented at 4:46 pm on February 5, 2025: 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/31803.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	ismaelsadeeq

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   No conflicts as of last run.

3. DrahtBot added the label Tests on Feb 5, 2025
4. fuzz: Fuzz reserved weight option in BlockAssembler 8c92624365
5. fjahr force-pushed on Feb 10, 2025
6. fjahr marked this as ready for review on Feb 10, 2025
7. 
   fjahr commented at 6:35 pm on February 10, 2025: contributor

   #31384 was merged, so this is rebased and ready for review.
8. in src/test/fuzz/mini_miner.cpp:192 in 8c92624365

   184@@ -184,6 +185,11 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
   185     node::BlockAssembler::Options miner_options;
   186     miner_options.blockMinFeeRate = target_feerate;
   187     miner_options.nBlockMaxWeight = MAX_BLOCK_WEIGHT;
   188+    // Only setting reserved weight when necessary based on the template size
   189+    const auto reserved_weight = MAX_BLOCK_WEIGHT - pool.GetTotalTxSize();
   190+    if (reserved_weight < DEFAULT_BLOCK_RESERVED_WEIGHT) {
   191+        miner_options.block_reserved_weight = reserved_weight;
   192+    }
   

   ---

   ---

   

   ismaelsadeeq commented at 10:28 pm on February 10, 2025:

   I don’t think this conditional check is necessary. It will be simpler and more straightforward to just set the minimum rather than creating a new variable and then checking the conditional statement.

   In the worst case, we are creating two variables here, whereas in my suggestion, we are just updating the initial variable.

   0    miner_options.block_reserved_weight = MINIMUM_BLOCK_RESERVED_WEIGHT;  
   

   ---

   

   fjahr commented at 4:14 pm on February 11, 2025:

   Yes, this would be simpler but I am not sure if we want it to be simpler. With fuzzing we usually try to hit as different many scenarios as possible and that’s why I chose this, sometimes setting block_reserved_weight and sometimes not, and setting it to a variety of values. I’m not an expert on fuzzing though, maybe someone with a focus on it can weight in here.
9. in src/test/fuzz/mini_miner.cpp:133 in 8c92624365

   128@@ -129,6 +129,8 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
   129     // Make a copy to preserve determinism.
   130     std::deque<COutPoint> available_coins = g_available_coins;
   131     std::vector<CTransactionRef> transactions;
   132+    // The maximum block template size we expect to produce
   133+    const auto block_adjusted_max_weight = MAX_BLOCK_WEIGHT - MINIMUM_BLOCK_RESERVED_WEIGHT;
   

   ---

   ---

   

   ismaelsadeeq commented at 10:30 pm on February 10, 2025:

   Nice moving this here, prevents creating block_adjusted_max_weight variable in each loop iteration.
10. 
    ismaelsadeeq commented at 10:30 pm on February 10, 2025: member

    Concept ACK

Contributors
fjahr DrahtBot ismaelsadeeq

Labels
Tests