While working on #30079, I noticed that using multiplication to compare fee rates results in a signed integer overflow. https://github.com/bitcoin/bitcoin/blob/fcc3b653dc2bd5683193a836556de07bea4d1b11/src/node/mini_miner.cpp#L191-L195
The policy estimator fuzz tests generate transaction fee and sigops values using ConsumeTxMemPoolEntry
.
https://github.com/bitcoin/bitcoin/blob/fcc3b653dc2bd5683193a836556de07bea4d1b11/src/test/fuzz/util/mempool.cpp#L17-L31
We fuzz test CBlockPolicyEstimator::processBlock
using these randomly generated transaction fee and size.
https://github.com/bitcoin/bitcoin/blob/fcc3b653dc2bd5683193a836556de07bea4d1b11/src/test/fuzz/policy_estimator.cpp#L71-L79
#30079 uses mini miner to linearize received block transactions in CBlockPolicyEstimator::processBlock
This enables the fuzzed values to reach the mini miner, revealing the overflow.
A possible is to compare the fee rates using the CFeeRate
operator<=
, as I did here https://github.com/bitcoin/bitcoin/pull/30079/commits/ba565be6fae897d43698dfa47b7c90aba6cf88c6 to fix the C.I failure in #30079.
However, @murchandamus mentioned that the reason for originally using multiplication for the comparison was due to precision loss.
https://cirrus-ci.com/task/5125969122426880?logs=ci#L3239
0INFO: Seed: 1181225158
1INFO: Loaded 1 modules (585279 inline 8-bit counters): 585279 [0x560b0df0fac8, 0x560b0df9e907),
2INFO: Loaded 1 PC tables (585279 PCs): 585279 [0x560b0df9e908,0x560b0e88ccf8),
3INFO: 973 files found in /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/policy_estimator
4INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1047554 bytes
5INFO: seed corpus: files: 973 min: 1b max: 1047554b total: 47876409b rss: 149Mb
6[#256](/bitcoin-bitcoin/256/) pulse cov: 3067 ft: 7190 corp: 194/19Kb exec/s: 85 rss: 464Mb
7node/mini_miner.cpp:196:33: runtime error: signed integer overflow: 67333229723135 * 285030 cannot be represented in type 'int64_t' (aka 'long')
8 [#0](/bitcoin-bitcoin/0/) 0x560b0b9fa461 in bool node::AncestorFeerateComparator::operator()<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>(std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>> const&, std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>> const&) const::'lambda'(node::MiniMinerMempoolEntry const&)::operator()(node::MiniMinerMempoolEntry const&) const src/node/mini_miner.cpp:196:33
9 [#1](/bitcoin-bitcoin/1/) 0x560b0b9f9dbe in bool node::AncestorFeerateComparator::operator()<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>(std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>> const&, std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>> const&) const src/node/mini_miner.cpp:200:28
10 [#2](/bitcoin-bitcoin/2/) 0x560b0b9fb0ff in bool __gnu_cxx::__ops::_Iter_comp_iter<node::AncestorFeerateComparator>::operator()<__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>>(__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/predefined_ops.h:158:23
11 [#3](/bitcoin-bitcoin/3/) 0x560b0b9fb0ff in void std::__insertion_sort<__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__ops::_Iter_comp_iter<node::AncestorFeerateComparator>>(__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__ops::_Iter_comp_iter<node::AncestorFeerateComparator>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_algo.h:1819:8
12 [#4](/bitcoin-bitcoin/4/) 0x560b0b9f074f in void std::__sort<__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__ops::_Iter_comp_iter<node::AncestorFeerateComparator>>(__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__ops::_Iter_comp_iter<node::AncestorFeerateComparator>) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_algo.h:1950:4
13 [#5](/bitcoin-bitcoin/5/) 0x560b0b9f074f in void std::sort<__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, node::AncestorFeerateComparator>(__gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, __gnu_cxx::__normal_iterator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>*, std::vector<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>, std::allocator<std::_Rb_tree_iterator<std::pair<uint256 const, node::MiniMinerMempoolEntry>>>>>, node::AncestorFeerateComparator) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_algo.h:4894:7
14 [#6](/bitcoin-bitcoin/6/) 0x560b0b9f074f in node::MiniMiner::BuildMockTemplate(std::optional<CFeeRate>) src/node/mini_miner.cpp:263:9
15 [#7](/bitcoin-bitcoin/7/) 0x560b0b9f24d2 in node::MiniMiner::Linearize() src/node/mini_miner.cpp:320:5
16 [#8](/bitcoin-bitcoin/8/) 0x560b0c0fcecd in LinearizeTransactions(std::vector<RemovedMempoolTransactionInfo, std::allocator<RemovedMempoolTransactionInfo>> const&) src/policy/fees_util.cpp:66:61
17 [#9](/bitcoin-bitcoin/9/) 0x560b0ba50884 in CBlockPolicyEstimator::removeCPFPdParentTxs(std::vector<RemovedMempoolTransactionInfo, std::allocator<RemovedMempoolTransactionInfo>> const&) src/policy/fees.cpp:668:41
18 [#10](/bitcoin-bitcoin/10/) 0x560b0ba4d92b in CBlockPolicyEstimator::processBlock(std::vector<RemovedMempoolTransactionInfo, std::allocator<RemovedMempoolTransactionInfo>> const&, unsigned int) src/policy/fees.cpp:711:5
19 [#11](/bitcoin-bitcoin/11/) 0x560b0b29d292 in policy_estimator_fuzz_target(Span<unsigned char const>)::$_1::operator()() const src/test/fuzz/policy_estimator.cpp:78:40
20 [#12](/bitcoin-bitcoin/12/) 0x560b0b29d292 in unsigned long CallOneOf<policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3>(FuzzedDataProvider&, policy_estimator_fuzz_target(Span<unsigned char const>)::$_0, policy_estimator_fuzz_target(Span<unsigned char const>)::$_1, policy_estimator_fuzz_target(Span<unsigned char const>)::$_2, policy_estimator_fuzz_target(Span<unsigned char const>)::$_3) src/./test/fuzz/util.h:42:27
21 [#13](/bitcoin-bitcoin/13/) 0x560b0b29d292 in policy_estimator_fuzz_target(Span<unsigned char const>) src/test/fuzz/policy_estimator.cpp:38:9
22 [#14](/bitcoin-bitcoin/14/) 0x560b0b41be8d in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
23 [#15](/bitcoin-bitcoin/15/) 0x560b0b41be8d in LLVMFuzzerTestOneInput src/test/fuzz/fuzz.cpp:201:5
24 [#16](/bitcoin-bitcoin/16/) 0x560b0ad42044 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1abb044) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
25 [#17](/bitcoin-bitcoin/17/) 0x560b0ad41739 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1aba739) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
26 [#18](/bitcoin-bitcoin/18/) 0x560b0ad43356 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1abc356) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
27 [#19](/bitcoin-bitcoin/19/) 0x560b0ad43867 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1abc867) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
28 [#20](/bitcoin-bitcoin/20/) 0x560b0ad30d5f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1aa9d5f) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
29 [#21](/bitcoin-bitcoin/21/) 0x560b0ad5b3e6 in main (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1ad43e6) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
30 [#22](/bitcoin-bitcoin/22/) 0x7f432cb5e1c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
31 [#23](/bitcoin-bitcoin/23/) 0x7f432cb5e28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
32 [#24](/bitcoin-bitcoin/24/) 0x560b0ad25d44 in _start (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x1a9ed44) (BuildId: 38c5692016408502c695f631cd9bdad2a9744d28)
33SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow node/mini_miner.cpp:196:33
34MS: 0 ; base unit: 0000000000000000000000000000000000000000
35artifact_prefix='./'; Test unit written to ./crash-2315c89458602360eb93362328da5c0ef21f9864
36Traceback (most recent call last):
37 File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 411, in <module>
38 main()
39 File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 199, in main
40 run_once(
41 File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 376, in run_once
42 assert len(done_stat) == 1
43 ^^^^^^^^^^^^^^^^^^^
44AssertionError