coins,refactor: Reduce getblockstats RPC UTXO overhead estimation #31449

1. 
   l0rinc commented at 5:36 pm on December 9, 2024: contributor

   The getblockstats RPC currently overestimates UTXO overhead by treating the fCoinBase bitfield as a separate bool in PER_UTXO_OVERHEAD. However, fCoinBase and nHeight are packed into a single 32-bit integer (both in memory and when serialized) - the extra bool in the overhead calculation is unnecessary.

   This PR introduces the following changes across three commits:

   • Stores fCoinBase as a proper bool to minimize conversions.
   • Unifies the serialization style for better readability.
   • Adjusts UTXO overhead estimation to reflect the actual structure and updates related tests accordingly.
2. coins,refactor: Migrate Coin#fCoinBase to bool 5658c51871
3. 
   DrahtBot commented at 5:36 pm on December 9, 2024: 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/31449.

   Reviews

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

4. DrahtBot added the label CI failed on Dec 9, 2024
5. coins,refactor: Unify Coin Serialization styles ff9d484876
6. rpc: Fix utxo_size_inc(_actual) calculation

   `nHeight` and `fCoinBase` are stored in `Coin` as bitfields and are serialized as a single 32 bit number, so the UTXO overhead calculation shouldn't include an additional boolean

   72ec388244
7. l0rinc force-pushed on Dec 9, 2024
8. DrahtBot removed the label CI failed on Dec 9, 2024
9. l0rinc marked this as ready for review on Dec 9, 2024
10. in src/test/fuzz/utxo_snapshot.cpp:107 in 5658c51871 outdated

    103@@ -104,7 +104,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
    104                 outfile << coinbase->GetHash();
    105                 WriteCompactSize(outfile, 1); // number of coins for the hash
    106                 WriteCompactSize(outfile, 0); // index of coin
    107-                outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/1);
    108+                outfile << Coin(coinbase->vout[0], height, /*fCoinBaseIn=*/true);
    

    ---

    ---

    

    TheCharlatan commented at 9:38 am on December 10, 2024:

    Can a current utxo snapshot still be loaded with this change?

    ---

    

    l0rinc commented at 9:50 am on December 10, 2024:

    Coin already had a bool fCoinBaseIn parameter, this shouldn’t change anything.

    But it would be really useful if you could show me how to test loading a current utxo snapshot to make absolutely sure.

    ---

    

    TheCharlatan commented at 11:42 am on December 10, 2024:

    I looked at the changes more closely now, and yes, this should not change the format. I also checked that doing a dumptxoutset produces the same data on both master and this PR.
11. l0rinc requested review from TheCharlatan on Jan 9, 2025

Contributors
l0rinc DrahtBot TheCharlatan

Review Requested
TheCharlatan