Adds a fuzz target that directly calls ConnectBlock with fJustCheck set to true, so it hits block/transaction validation without writing undo data or updating the chainstate.
This PR is essentially #34651 with some minor tweaks and style cleanups. Additional validation harnesses (e.g. #34895) could build on this test's setup.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35850.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline and AI policy for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | dergoegge, nervana21 |
| Concept ACK | Crypt-iQ, ismaelsadeeq, brunoerg |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
No conflicts as of last run.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Co-authored-by: marcofleon <marleo23@proton.me>
utACK 020bede7c2d3e7a764ed51344925fcfe2300b3cf
A follow up could be to investigate if we can make this harness find https://bitcoincore.org/en/2026/05/05/disclose-cve-2024-52911/ (perhaps the only thing missing here is to actually use the multi-threaded checkqueue).
Concept ACK, will review
Concept ACK, thanks for picking it up.
Concept ACK
374 | + 375 | + if (!coinbase) { 376 | + // Create spending scripts for all CTxOuts so they can be spent in later 377 | + // transactions. Do it here as the transaction hash is definitive. 378 | + for (int i = 0; i < num_outputs; i++) { 379 | + additional_txins.emplace_back(GetSpendingScript(*res, i));
In LoadCurrentBlock we filter before adding to g_spend_candidate_txins. Could we use the same idea here? Otherwise ConsumeTransaction can pollute additional_txins with scripts that are not spendable.
Also, the existing LoadCurrentBlock skip only checks the OP_RETURN prefix, so oversized unspendable scripts can be added there. Prefer scriptPubKey.IsUnspendable() at both insert sites so spend candidates match what CCoinsViewCache::AddCoin would keep.
<details><summary>suggested patch</summary>
diff --git a/src/test/fuzz/connect_block.cpp b/src/test/fuzz/connect_block.cpp
index 86eccde4f1..08ab22550b 100644
--- a/src/test/fuzz/connect_block.cpp
+++ b/src/test/fuzz/connect_block.cpp
@@ -42,7 +42,7 @@ TestingSetup* g_setup;
static std::vector<std::shared_ptr<CBlock>> g_blocks;
/** Set of block hashes in g_blocks */
static std::set<uint256> g_existing_block_hashes;
-/** CTxIns for spending outputs (excluding OP_RETURN), which can be unspent, already spent, or an immature coinbase. */
+/** CTxIns for spending outputs (excluding unspendable outputs), which can be unspent, already spent, or an immature coinbase. */
static std::vector<CTxIn> g_spend_candidate_txins;
/** Static P2SH_OP_TRUE script */
static const CScript P2SH_OP_TRUE = CScript() << OP_HASH160 << ToByteVector(ScriptHash(CScript() << OP_TRUE)) << OP_EQUAL;
@@ -71,18 +71,17 @@ static void InitTaprootScript()
}
/**
- * Given a transaction and an output index, create a CTxIn that can be used to
- * spend it (if possible).
+ * Given a transaction and a spendable output index, create a CTxIn that can be
+ * used to spend it.
*/
static CTxIn GetSpendingScript(const CTransaction& tx, uint32_t vout_index)
{
Assert(vout_index < tx.vout.size());
const CTxOut& output = tx.vout[vout_index];
- CTxIn res{COutPoint(tx.GetHash(), vout_index)};
- if (output.scriptPubKey.size() >= 1 && output.scriptPubKey[0] == OP_RETURN)
- return res;
+ Assert(!output.scriptPubKey.IsUnspendable());
+ CTxIn res{COutPoint(tx.GetHash(), vout_index)};
if (output.scriptPubKey == P2WSH_OP_TRUE) {
res.scriptSig = CScript();
res.scriptWitness.stack.push_back(WITNESS_STACK_ELEM_OP_TRUE);
@@ -98,6 +97,13 @@ static CTxIn GetSpendingScript(const CTransaction& tx, uint32_t vout_index)
return res;
}
+/** Add a spend-candidate CTxIn unless the output is unspendable. */
+static void MaybeAddSpendCandidate(std::vector<CTxIn>& pool, const CTransaction& tx, uint32_t vout_index)
+{
+ Assert(vout_index < tx.vout.size());
+ if (tx.vout[vout_index].scriptPubKey.IsUnspendable()) return;
+ pool.push_back(GetSpendingScript(tx, vout_index));
+}
/**
* Read the block from the BlockManager and add it to g_blocks and g_existing_block_hashes.
@@ -119,11 +125,7 @@ static void LoadCurrentBlock(Chainstate& chainstate, CBlockIndex* current_block)
// Iterate all transaction outputs.
for (const auto& tx : g_blocks[current_block->nHeight]->vtx) {
for (uint32_t vout_index{0}; vout_index < tx->vout.size(); ++vout_index) {
- auto& vout = tx->vout[vout_index];
- // Do not keep OP_RETURN outputs as they are not spendable.
- if (vout.scriptPubKey.size() >= 1 && vout.scriptPubKey[0] == OP_RETURN) continue;
- // Create the CTxIn that can be used to spend this output.
- g_spend_candidate_txins.push_back(GetSpendingScript(*tx, vout_index));
+ MaybeAddSpendCandidate(g_spend_candidate_txins, *tx, vout_index);
}
}
}
@@ -373,10 +375,10 @@ CTransactionRef ConsumeTransaction(FuzzedDataProvider& fuzzed_data_provider,
auto res = MakeTransactionRef(tx);
if (!coinbase) {
- // Create spending scripts for all CTxOuts so they can be spent in later
+ // Create spending scripts for spendable CTxOuts so they can be spent in later
// transactions. Do it here as the transaction hash is definitive.
for (int i = 0; i < num_outputs; i++) {
- additional_txins.emplace_back(GetSpendingScript(*res, i));
+ MaybeAddSpendCandidate(additional_txins, *res, i);
}
}
</details>
tACK 020bede7c2d3e7a764ed51344925fcfe2300b3cf
Left a non-blocking suggestion that can also be considered as a follow-up.
343 | + // Read CAmount to spend. 344 | + tx.vout[i].nValue = fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(-10, 50 * COIN + 10); 345 | + 346 | + // Read scriptPubKey type into one of the valid types. 347 | + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 4)) { 348 | + case 0:
CallOneOf should be shorter and clearer, no?