Skip to content

Commit 837e9ed

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#26898: fuzz: Add PartiallyDownloadedBlock target
a1c3627 [fuzz] Assert that omitting missing transactions always fails block reconstruction (dergoegge) a8ac61a [fuzz] Add PartiallyDownloadedBlock target (dergoegge) 42bd4c7 [block encodings] Avoid fuzz blocking asserts in PartiallyDownloadedBlock (dergoegge) 1429f83 [block encodings] Make CheckBlock mockable for PartiallyDownloadedBlock (dergoegge) Pull request description: This PR adds a fuzz target for `PartiallyDownloadedBlock`, which we currently do not have any coverage for. ACKs for top commit: mzumsande: Code Review ACK a1c3627 MarcoFalke: re-ACK a1c3627 🎼 Tree-SHA512: 01ae452fe457da0c8f2b28c72091d40807c56a9e5d0f80b55f166b67be50baf80a02f53d4cbe9736bb22424cca1758b87e4e471b8a24e756c22563a2640e9a5f
2 parents 75e752f + a1c3627 commit 837e9ed

4 files changed

+165
-6
lines changed

src/Makefile.test.include

+1
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ test_fuzz_fuzz_SOURCES = \
293293
test/fuzz/parse_numbers.cpp \
294294
test/fuzz/parse_script.cpp \
295295
test/fuzz/parse_univalue.cpp \
296+
test/fuzz/partially_downloaded_block.cpp \
296297
test/fuzz/policy_estimator.cpp \
297298
test/fuzz/policy_estimator_io.cpp \
298299
test/fuzz/pow.cpp \

src/blockencodings.cpp

+12-6
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
5252
if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_WEIGHT / MIN_SERIALIZABLE_TRANSACTION_WEIGHT)
5353
return READ_STATUS_INVALID;
5454

55-
assert(header.IsNull() && txn_available.empty());
55+
if (!header.IsNull() || !txn_available.empty()) return READ_STATUS_INVALID;
56+
5657
header = cmpctblock.header;
5758
txn_available.resize(cmpctblock.BlockTxCount());
5859

@@ -167,14 +168,18 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
167168
return READ_STATUS_OK;
168169
}
169170

170-
bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const {
171-
assert(!header.IsNull());
171+
bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const
172+
{
173+
if (header.IsNull()) return false;
174+
172175
assert(index < txn_available.size());
173176
return txn_available[index] != nullptr;
174177
}
175178

176-
ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) {
177-
assert(!header.IsNull());
179+
ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing)
180+
{
181+
if (header.IsNull()) return READ_STATUS_INVALID;
182+
178183
uint256 hash = header.GetHash();
179184
block = header;
180185
block.vtx.resize(txn_available.size());
@@ -197,7 +202,8 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
197202
return READ_STATUS_INVALID;
198203

199204
BlockValidationState state;
200-
if (!CheckBlock(block, state, Params().GetConsensus())) {
205+
CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock;
206+
if (!check_block(block, state, Params().GetConsensus(), /*fCheckPoW=*/true, /*fCheckMerkleRoot=*/true)) {
201207
// TODO: We really want to just check merkle tree manually here,
202208
// but that is expensive, and CheckBlock caches a block's
203209
// "checked-status" (in the CBlock?). CBlock should be able to

src/blockencodings.h

+10
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,13 @@
77

88
#include <primitives/block.h>
99

10+
#include <functional>
1011

1112
class CTxMemPool;
13+
class BlockValidationState;
14+
namespace Consensus {
15+
struct Params;
16+
};
1217

1318
// Transaction compression schemes for compact block relay can be introduced by writing
1419
// an actual formatter here.
@@ -129,6 +134,11 @@ class PartiallyDownloadedBlock {
129134
const CTxMemPool* pool;
130135
public:
131136
CBlockHeader header;
137+
138+
// Can be overriden for testing
139+
using CheckBlockFn = std::function<bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool)>;
140+
CheckBlockFn m_check_block_mock{nullptr};
141+
132142
explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {}
133143

134144
// extra_txn is a list of extra transactions to look at, in <witness hash, reference> form
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
#include <blockencodings.h>
2+
#include <consensus/merkle.h>
3+
#include <consensus/validation.h>
4+
#include <primitives/block.h>
5+
#include <primitives/transaction.h>
6+
#include <test/fuzz/FuzzedDataProvider.h>
7+
#include <test/fuzz/fuzz.h>
8+
#include <test/fuzz/util.h>
9+
#include <test/fuzz/util/mempool.h>
10+
#include <test/util/setup_common.h>
11+
#include <test/util/txmempool.h>
12+
#include <txmempool.h>
13+
14+
#include <cstddef>
15+
#include <cstdint>
16+
#include <limits>
17+
#include <memory>
18+
#include <optional>
19+
#include <set>
20+
#include <vector>
21+
22+
namespace {
23+
const TestingSetup* g_setup;
24+
} // namespace
25+
26+
void initialize_pdb()
27+
{
28+
static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
29+
g_setup = testing_setup.get();
30+
}
31+
32+
PartiallyDownloadedBlock::CheckBlockFn FuzzedCheckBlock(std::optional<BlockValidationResult> result)
33+
{
34+
return [result](const CBlock&, BlockValidationState& state, const Consensus::Params&, bool, bool) {
35+
if (result) {
36+
return state.Invalid(*result);
37+
}
38+
39+
return true;
40+
};
41+
}
42+
43+
FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb)
44+
{
45+
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
46+
47+
auto block{ConsumeDeserializable<CBlock>(fuzzed_data_provider)};
48+
if (!block || block->vtx.size() == 0 ||
49+
block->vtx.size() >= std::numeric_limits<uint16_t>::max()) {
50+
return;
51+
}
52+
53+
CBlockHeaderAndShortTxIDs cmpctblock{*block};
54+
55+
CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)};
56+
PartiallyDownloadedBlock pdb{&pool};
57+
58+
// Set of available transactions (mempool or extra_txn)
59+
std::set<uint16_t> available;
60+
// The coinbase is always available
61+
available.insert(0);
62+
63+
std::vector<std::pair<uint256, CTransactionRef>> extra_txn;
64+
for (size_t i = 1; i < block->vtx.size(); ++i) {
65+
auto tx{block->vtx[i]};
66+
67+
bool add_to_extra_txn{fuzzed_data_provider.ConsumeBool()};
68+
bool add_to_mempool{fuzzed_data_provider.ConsumeBool()};
69+
70+
if (add_to_extra_txn) {
71+
extra_txn.emplace_back(tx->GetWitnessHash(), tx);
72+
available.insert(i);
73+
}
74+
75+
if (add_to_mempool) {
76+
LOCK2(cs_main, pool.cs);
77+
pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx));
78+
available.insert(i);
79+
}
80+
}
81+
82+
auto init_status{pdb.InitData(cmpctblock, extra_txn)};
83+
84+
std::vector<CTransactionRef> missing;
85+
// Whether we skipped a transaction that should be included in `missing`.
86+
// FillBlock should never return READ_STATUS_OK if that is the case.
87+
bool skipped_missing{false};
88+
for (size_t i = 0; i < cmpctblock.BlockTxCount(); i++) {
89+
// If init_status == READ_STATUS_OK then a available transaction in the
90+
// compact block (i.e. IsTxAvailable(i) == true) implies that we marked
91+
// that transaction as available above (i.e. available.count(i) > 0).
92+
// The reverse is not true, due to possible compact block short id
93+
// collisions (i.e. available.count(i) > 0 does not imply
94+
// IsTxAvailable(i) == true).
95+
if (init_status == READ_STATUS_OK) {
96+
assert(!pdb.IsTxAvailable(i) || available.count(i) > 0);
97+
}
98+
99+
bool skip{fuzzed_data_provider.ConsumeBool()};
100+
if (!pdb.IsTxAvailable(i) && !skip) {
101+
missing.push_back(block->vtx[i]);
102+
}
103+
104+
skipped_missing |= (!pdb.IsTxAvailable(i) && skip);
105+
}
106+
107+
// Mock CheckBlock
108+
bool fail_check_block{fuzzed_data_provider.ConsumeBool()};
109+
auto validation_result =
110+
fuzzed_data_provider.PickValueInArray(
111+
{BlockValidationResult::BLOCK_RESULT_UNSET,
112+
BlockValidationResult::BLOCK_CONSENSUS,
113+
BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE,
114+
BlockValidationResult::BLOCK_CACHED_INVALID,
115+
BlockValidationResult::BLOCK_INVALID_HEADER,
116+
BlockValidationResult::BLOCK_MUTATED,
117+
BlockValidationResult::BLOCK_MISSING_PREV,
118+
BlockValidationResult::BLOCK_INVALID_PREV,
119+
BlockValidationResult::BLOCK_TIME_FUTURE,
120+
BlockValidationResult::BLOCK_CHECKPOINT,
121+
BlockValidationResult::BLOCK_HEADER_LOW_WORK});
122+
pdb.m_check_block_mock = FuzzedCheckBlock(
123+
fail_check_block ?
124+
std::optional<BlockValidationResult>{validation_result} :
125+
std::nullopt);
126+
127+
CBlock reconstructed_block;
128+
auto fill_status{pdb.FillBlock(reconstructed_block, missing)};
129+
switch (fill_status) {
130+
case READ_STATUS_OK:
131+
assert(!skipped_missing);
132+
assert(!fail_check_block);
133+
assert(block->GetHash() == reconstructed_block.GetHash());
134+
break;
135+
case READ_STATUS_CHECKBLOCK_FAILED: [[fallthrough]];
136+
case READ_STATUS_FAILED:
137+
assert(fail_check_block);
138+
break;
139+
case READ_STATUS_INVALID:
140+
break;
141+
}
142+
}

0 commit comments

Comments
 (0)