fuzz: mini_miner_selection: ASSERT: mock_template_txids.size() <= blocktemplate->block.vtx.size() #30367

1. 
   maflcko commented at 10:05 am on July 1, 2024: member

   https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69957

   https://github.com/bitcoin/bitcoin/blob/2f6dca4d1c01ee47275a4292f128d714736837a1/src/test/fuzz/mini_miner.cpp#L192

2. maflcko added the label Wallet on Jul 1, 2024
3. maflcko added the label Tests on Jul 1, 2024
4. 
   maflcko commented at 10:20 am on July 1, 2024: member

   0$ echo 'AyABAAAAACAg/wAAAAAA/yD/AQAAAAAg/yCu/w==' | base64 --decode > /tmp/oss-fuzz-crash 
   1$ FUZZ=mini_miner_selection ./src/test/fuzz/fuzz /tmp/oss-fuzz-crash
   

5. 
   dergoegge commented at 10:12 am on July 9, 2024: member

   cc @glozow @murchandamus (in case you missed it) would be nice to have this fixed and also have an explanation of the actual bug, since it is somewhat of a mystery why it’s so hard to find
6. 
   glozow commented at 11:49 am on July 9, 2024: member

   Ooh fun, this found a difference between MiniMiner and CTxMemPool’s comparators , which boils down to a lack of precision in CFeeRate.

   Debugging, the 3 entries in mempool are

   A <- B <- C A: 1sat/264vB B: 0sat/178vB C:1sat/178vB

   The minimum feerate, i.e. target_feerate or blockMinFeeRate is 1sat/vB.

   BlockAssembler selects A. MiniMiner selects ABC. They are both wrong to do so, since all transactions have a feerate below 1sat/vB. This is because CFeeRate rounds minfeerate.GetFee(packagesize) up to 1sat, and the if condition succeeds. But this isn’t the bug that was hit by the fuzzer.

   The actual bug is in the comparators: CompareTxMemPoolentryByAncestorFee in CTxMemPool casts everything to doubles and doesn’t compare using CFeeRate objects. It can see that A’s feerate is 1/264=0.0038sat/vB, while C’s ancestor feerate is 2/260=0.0032sat/vB, and orders A before C.

   CFeeRate only stores satoshis per KvB, so CFeerate(1, 264) and CFeeRate(2, 260) both become CFeerate(nSatoshisPerK=3), i.e. 0.003sat/vB.

   The AncestorFeerateComparator in MiniMiner compares CFeeRates and basically sees that 3 == 3, tiebreaks using txid, and selects package ABC first. You can observe that the fuzz input in this issue succeeds if you multiply all fees by 10 before passing them to the CFeeRate constructors in AncestorFeerateComparator (then it’s comparing 32 and 38).

7. 
   glozow commented at 12:01 pm on July 9, 2024: member

   I can open a PR to change MiniMiner to store/compare FeeFracs, which by itself would fix the imprecision bug, though I’m not sure if that’ll make it identical to txmempool.

   Changing txmempool seems less important imo since there is no current imprecision bug, and perhaps cluster mempool plans to do this anyway.

8. fanquake closed this on Jul 15, 2024

   ---

9. fanquake referenced this in commit 01ed4927f0 on Jul 15, 2024

Contributors
maflcko dergoegge glozow

Labels
Wallet Tests