From 1429f8377017c0029cb87c4d355c37b796432611 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 16 Jan 2023 16:13:52 +0100 Subject: [PATCH 1/4] [block encodings] Make CheckBlock mockable for PartiallyDownloadedBlock --- src/blockencodings.cpp | 3 ++- src/blockencodings.h | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index bcb86d75cc0..c83e893c0ec 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -197,7 +197,8 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< return READ_STATUS_INVALID; BlockValidationState state; - if (!CheckBlock(block, state, Params().GetConsensus())) { + CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock; + if (!check_block(block, state, Params().GetConsensus(), /*fCheckPoW=*/true, /*fCheckMerkleRoot=*/true)) { // TODO: We really want to just check merkle tree manually here, // but that is expensive, and CheckBlock caches a block's // "checked-status" (in the CBlock?). CBlock should be able to diff --git a/src/blockencodings.h b/src/blockencodings.h index e60c1e3db42..7207ff1ae2d 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -7,8 +7,13 @@ #include +#include class CTxMemPool; +class BlockValidationState; +namespace Consensus { +struct Params; +}; // Transaction compression schemes for compact block relay can be introduced by writing // an actual formatter here. @@ -129,6 +134,11 @@ protected: const CTxMemPool* pool; public: CBlockHeader header; + + // Can be overriden for testing + using CheckBlockFn = std::function; + CheckBlockFn m_check_block_mock{nullptr}; + explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} // extra_txn is a list of extra transactions to look at, in form From 42bd4c746824e3b2adf2c696cf4a705fa43d1fa8 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 16 Jan 2023 16:14:16 +0100 Subject: [PATCH 2/4] [block encodings] Avoid fuzz blocking asserts in PartiallyDownloadedBlock --- src/blockencodings.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index c83e893c0ec..0d5575e5d5b 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -52,7 +52,8 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_WEIGHT / MIN_SERIALIZABLE_TRANSACTION_WEIGHT) return READ_STATUS_INVALID; - assert(header.IsNull() && txn_available.empty()); + if (!header.IsNull() || !txn_available.empty()) return READ_STATUS_INVALID; + header = cmpctblock.header; txn_available.resize(cmpctblock.BlockTxCount()); @@ -167,14 +168,18 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c return READ_STATUS_OK; } -bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const { - assert(!header.IsNull()); +bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const +{ + if (header.IsNull()) return false; + assert(index < txn_available.size()); return txn_available[index] != nullptr; } -ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector& vtx_missing) { - assert(!header.IsNull()); +ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector& vtx_missing) +{ + if (header.IsNull()) return READ_STATUS_INVALID; + uint256 hash = header.GetHash(); block = header; block.vtx.resize(txn_available.size()); From a8ac61ab5e1805df61f4dc94ded44a874725484c Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 16 Jan 2023 16:16:07 +0100 Subject: [PATCH 3/4] [fuzz] Add PartiallyDownloadedBlock target --- src/Makefile.test.include | 1 + src/test/fuzz/partially_downloaded_block.cpp | 136 +++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 src/test/fuzz/partially_downloaded_block.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 1a29e9a47ac..4d867fdc2f1 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -293,6 +293,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/parse_numbers.cpp \ test/fuzz/parse_script.cpp \ test/fuzz/parse_univalue.cpp \ + test/fuzz/partially_downloaded_block.cpp \ test/fuzz/policy_estimator.cpp \ test/fuzz/policy_estimator_io.cpp \ test/fuzz/pow.cpp \ diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp new file mode 100644 index 00000000000..fa9c18e52b6 --- /dev/null +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -0,0 +1,136 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace { +const TestingSetup* g_setup; +} // namespace + +void initialize_pdb() +{ + static const auto testing_setup = MakeNoLogFileContext(); + g_setup = testing_setup.get(); +} + +PartiallyDownloadedBlock::CheckBlockFn FuzzedCheckBlock(std::optional result) +{ + return [result](const CBlock&, BlockValidationState& state, const Consensus::Params&, bool, bool) { + if (result) { + return state.Invalid(*result); + } + + return true; + }; +} + +FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + + auto block{ConsumeDeserializable(fuzzed_data_provider)}; + if (!block || block->vtx.size() == 0 || + block->vtx.size() >= std::numeric_limits::max()) { + return; + } + + CBlockHeaderAndShortTxIDs cmpctblock{*block}; + + CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)}; + PartiallyDownloadedBlock pdb{&pool}; + + // Set of available transactions (mempool or extra_txn) + std::set available; + // The coinbase is always available + available.insert(0); + + std::vector> extra_txn; + for (size_t i = 1; i < block->vtx.size(); ++i) { + auto tx{block->vtx[i]}; + + bool add_to_extra_txn{fuzzed_data_provider.ConsumeBool()}; + bool add_to_mempool{fuzzed_data_provider.ConsumeBool()}; + + if (add_to_extra_txn) { + extra_txn.emplace_back(tx->GetWitnessHash(), tx); + available.insert(i); + } + + if (add_to_mempool) { + LOCK2(cs_main, pool.cs); + pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); + available.insert(i); + } + } + + auto init_status{pdb.InitData(cmpctblock, extra_txn)}; + + std::vector missing; + for (size_t i = 0; i < cmpctblock.BlockTxCount(); i++) { + // If init_status == READ_STATUS_OK then a available transaction in the + // compact block (i.e. IsTxAvailable(i) == true) implies that we marked + // that transaction as available above (i.e. available.count(i) > 0). + // The reverse is not true, due to possible compact block short id + // collisions (i.e. available.count(i) > 0 does not imply + // IsTxAvailable(i) == true). + if (init_status == READ_STATUS_OK) { + assert(!pdb.IsTxAvailable(i) || available.count(i) > 0); + } + + bool skip{fuzzed_data_provider.ConsumeBool()}; + if (!pdb.IsTxAvailable(i) && !skip) { + missing.push_back(block->vtx[i]); + } + } + + // Mock CheckBlock + bool fail_check_block{fuzzed_data_provider.ConsumeBool()}; + auto validation_result = + fuzzed_data_provider.PickValueInArray( + {BlockValidationResult::BLOCK_RESULT_UNSET, + BlockValidationResult::BLOCK_CONSENSUS, + BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE, + BlockValidationResult::BLOCK_CACHED_INVALID, + BlockValidationResult::BLOCK_INVALID_HEADER, + BlockValidationResult::BLOCK_MUTATED, + BlockValidationResult::BLOCK_MISSING_PREV, + BlockValidationResult::BLOCK_INVALID_PREV, + BlockValidationResult::BLOCK_TIME_FUTURE, + BlockValidationResult::BLOCK_CHECKPOINT, + BlockValidationResult::BLOCK_HEADER_LOW_WORK}); + pdb.m_check_block_mock = FuzzedCheckBlock( + fail_check_block ? + std::optional{validation_result} : + std::nullopt); + + CBlock reconstructed_block; + auto fill_status{pdb.FillBlock(reconstructed_block, missing)}; + switch (fill_status) { + case READ_STATUS_OK: + assert(!fail_check_block); + assert(block->GetHash() == reconstructed_block.GetHash()); + break; + case READ_STATUS_CHECKBLOCK_FAILED: [[fallthrough]]; + case READ_STATUS_FAILED: + assert(fail_check_block); + break; + case READ_STATUS_INVALID: + break; + } +} From a1c36275b5a27ae685f49ff02dabff0adbf51aa1 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Thu, 19 Jan 2023 18:06:24 +0100 Subject: [PATCH 4/4] [fuzz] Assert that omitting missing transactions always fails block reconstruction --- src/test/fuzz/partially_downloaded_block.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index fa9c18e52b6..f8ba4f08d9a 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -82,6 +82,9 @@ FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb) auto init_status{pdb.InitData(cmpctblock, extra_txn)}; std::vector missing; + // Whether we skipped a transaction that should be included in `missing`. + // FillBlock should never return READ_STATUS_OK if that is the case. + bool skipped_missing{false}; for (size_t i = 0; i < cmpctblock.BlockTxCount(); i++) { // If init_status == READ_STATUS_OK then a available transaction in the // compact block (i.e. IsTxAvailable(i) == true) implies that we marked @@ -97,6 +100,8 @@ FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb) if (!pdb.IsTxAvailable(i) && !skip) { missing.push_back(block->vtx[i]); } + + skipped_missing |= (!pdb.IsTxAvailable(i) && skip); } // Mock CheckBlock @@ -123,6 +128,7 @@ FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb) auto fill_status{pdb.FillBlock(reconstructed_block, missing)}; switch (fill_status) { case READ_STATUS_OK: + assert(!skipped_missing); assert(!fail_check_block); assert(block->GetHash() == reconstructed_block.GetHash()); break;