Skip to content

Commit bfeacc1

Browse files
committed
Merge bitcoin/bitcoin#32154: fuzz: Avoid integer sanitizer warnings in policy_estimator target
fa6a007 fuzz: Avoid integer sanitizer warnings in policy_estimator target (MarcoFalke) Pull request description: It seems odd to write a fuzz target to trigger integer sanitizer warnings in `CBlockPolicyEstimator::processBlockTx` and then suppress them. If the scenario can happen in reality, the code should be properly fixed to handle the cases. If not, it seems better to fix the fuzz target to not trigger meaningless traces. Do that here by keeping track of the current height and limiting mempool entries to at most this entry height. ACKs for top commit: brunoerg: ACK fa6a007 dergoegge: utACK fa6a007 Tree-SHA512: 2092017dc309fb095fe5d43cfb76efb691795f303d567ee919be2b5cac19a944293636229903dc4d1e8b9fe5daf9dc3058544321eff1735f91f804c3baa36cd0
2 parents 06f9ead + fa6a007 commit bfeacc1

File tree

4 files changed

+16
-11
lines changed

4 files changed

+16
-11
lines changed

src/test/fuzz/policy_estimator.cpp

+11-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2020-2022 The Bitcoin Core developers
1+
// Copyright (c) 2020-present The Bitcoin Core developers
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

@@ -33,6 +33,12 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
3333
bool good_data{true};
3434

3535
CBlockPolicyEstimator block_policy_estimator{FeeestPath(*g_setup->m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES};
36+
37+
uint32_t current_height{0};
38+
const auto advance_height{
39+
[&] { current_height = fuzzed_data_provider.ConsumeIntegralInRange<decltype(current_height)>(current_height, 1 << 30); },
40+
};
41+
advance_height();
3642
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
3743
{
3844
CallOneOf(
@@ -44,7 +50,7 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
4450
return;
4551
}
4652
const CTransaction tx{*mtx};
47-
const CTxMemPoolEntry& entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx);
53+
const auto entry{ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height)};
4854
const auto tx_submitted_in_package = fuzzed_data_provider.ConsumeBool();
4955
const auto tx_has_mempool_parents = fuzzed_data_provider.ConsumeBool();
5056
const auto tx_info = NewMempoolTransactionInfo(entry.GetSharedTx(), entry.GetFee(),
@@ -68,14 +74,15 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
6874
break;
6975
}
7076
const CTransaction tx{*mtx};
71-
mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
77+
mempool_entries.emplace_back(CTxMemPoolEntry::ExplicitCopy, ConsumeTxMemPoolEntry(fuzzed_data_provider, tx, current_height));
7278
}
7379
std::vector<RemovedMempoolTransactionInfo> txs;
7480
txs.reserve(mempool_entries.size());
7581
for (const CTxMemPoolEntry& mempool_entry : mempool_entries) {
7682
txs.emplace_back(mempool_entry);
7783
}
78-
block_policy_estimator.processBlock(txs, fuzzed_data_provider.ConsumeIntegral<unsigned int>());
84+
advance_height();
85+
block_policy_estimator.processBlock(txs, current_height);
7986
},
8087
[&] {
8188
(void)block_policy_estimator.removeTx(ConsumeUInt256(fuzzed_data_provider));

src/test/fuzz/util/mempool.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2022 The Bitcoin Core developers
1+
// Copyright (c) 2022-present The Bitcoin Core developers
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

@@ -14,7 +14,7 @@
1414
#include <cstdint>
1515
#include <limits>
1616

17-
CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept
17+
CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx, uint32_t max_height) noexcept
1818
{
1919
// Avoid:
2020
// policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long'
@@ -24,7 +24,7 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider,
2424
assert(MoneyRange(fee));
2525
const int64_t time = fuzzed_data_provider.ConsumeIntegral<int64_t>();
2626
const uint64_t entry_sequence{fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
27-
const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
27+
const auto entry_height{fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, max_height)};
2828
const bool spends_coinbase = fuzzed_data_provider.ConsumeBool();
2929
const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, MAX_BLOCK_SIGOPS_COST);
3030
return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}};

src/test/fuzz/util/mempool.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2022 The Bitcoin Core developers
1+
// Copyright (c) 2022-present The Bitcoin Core developers
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

@@ -21,6 +21,6 @@ class DummyChainState final : public Chainstate
2121
}
2222
};
2323

24-
[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx) noexcept;
24+
[[nodiscard]] CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CTransaction& tx, uint32_t max_height=std::numeric_limits<uint32_t>::max()) noexcept;
2525

2626
#endif // BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H

test/sanitizer_suppressions/ubsan

-2
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,11 @@ unsigned-integer-overflow:CompressAmount
5353
unsigned-integer-overflow:DecompressAmount
5454
unsigned-integer-overflow:crypto/
5555
unsigned-integer-overflow:MurmurHash3
56-
unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx
5756
unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal
5857
unsigned-integer-overflow:prevector.h
5958
unsigned-integer-overflow:InsecureRandomContext::rand64
6059
unsigned-integer-overflow:InsecureRandomContext::SplitMix64
6160
unsigned-integer-overflow:bitset_detail::PopCount
62-
implicit-integer-sign-change:CBlockPolicyEstimator::processBlockTx
6361
implicit-integer-sign-change:SetStdinEcho
6462
implicit-integer-sign-change:compressor.h
6563
implicit-integer-sign-change:crypto/

0 commit comments

Comments
 (0)