fuzz: Block connection and chain reorganization fuzzing harnesses #34651

pull RobinDavid wants to merge 1 commits into bitcoin:master from RobinDavid:new-fuzz-harness-block-connection changing 2 files +499 −0
  1. RobinDavid commented at 3:25 PM on February 22, 2026: contributor

    Hi Bitcoin Core maintainers.

    This PR is a remastered version of fuzzing harnesses developed during a Bitcoin Core audit. It includes 1 fuzzing harnesses:

    • connect_block: Test the ConnectBlock() function responsible of validating the block and all its transaction with consensus rules. Thanks to justCheck parameter no side effect is performed on the internal state enabling relatively fast fuzzing.

    Besides pre-mining blocks, the harness initialization intends to bring a bit more of diversity by introducing additional transactions in blocks (otherwise only coinbases), and put some transactions in the mempool so that they can be 'picked' and put in a block by means of input mutation.

    Note: Some additional harnesses are added in another PR: #34895

    Authored by @RobinDavid and @nsurbay

  2. DrahtBot added the label Fuzzing on Feb 22, 2026
  3. DrahtBot commented at 3:26 PM on February 22, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34651.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK marcofleon
    Concept ACK dergoegge
    Stale ACK ismaelsadeeq

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34895 (fuzz: Fuzzing harnesses for ActivateBestChainStep and ActivateBestChain by RobinDavid)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • keep a references on blocks -> keep references on blocks [“a references” is grammatically incorrect; “references” should not be preceded by “a”.]
    • It intend to leave more space to craft complex transactions and especially with various scripts types -> It is intended to leave more space to craft complex transactions and especially with various script types [“It intend” is a grammatical error (“intend” should be conjugated), and “scripts types” should be “script types” for clarity.]
    • It is exclusively used by ConsumeBlock to read transaction inside a block. -> It is exclusively used by ConsumeBlock to read transactions inside a block. [“transaction” should be plural to match “various scripts types” / general-purpose fuzz input.]
    • Create additional transaction in the mempool that are spending coins from mature blocks. -> Create additional transactions in the mempool that are spending coins from mature blocks. [Singular/plural mismatch (“transaction” should be “transactions” given “additional” and the loop that adds multiple txs).]

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • active_chainstate.ConnectBlock(block, state, &new_index, active_coins, true) in src/test/fuzz/connect_block.cpp

    <sup>2026-04-08 20:58:41</sup>

  4. DrahtBot added the label CI failed on Feb 22, 2026
  5. DrahtBot commented at 4:12 PM on February 22, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/22279929273/job/64448849605</sub> <sub>LLM reason (✨ experimental): Compilation failed due to missing system header sys/sdt.h (not found).</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  6. in src/test/fuzz/connect_block.cpp:1 in 98ad4d8063 
       0 | @@ -0,0 +1,941 @@
   1 | +// Copyright (c) 2019-2021 The Bitcoin Core developers
    brunoerg commented at 7:55 PM on February 23, 2026:

    2021?

  7. RobinDavid force-pushed on Feb 23, 2026
  8. in ci/test/00_setup_env_arm.sh:11 in 28afc49ca0 
       7 | @@ -8,7 +8,7 @@ export LC_ALL=C.UTF-8
   8 |  
   9 |  export HOST=arm-linux-gnueabihf
  10 |  export DPKG_ADD_ARCH="armhf"
  11 | -export PACKAGES="python3-zmq g++-arm-linux-gnueabihf libc6:armhf libstdc++6:armhf libfontconfig1:armhf libxcb1:armhf"
  12 | +export PACKAGES="systemtap-sdt-dev python3-zmq g++-arm-linux-gnueabihf libc6:armhf libstdc++6:armhf libfontconfig1:armhf libxcb1:armhf"
    maflcko commented at 10:29 AM on February 24, 2026:

    This won't fix the ci failure. The CI fails because the build system does not add the usdt headers to the fuzz binary. So possible fixes are:

    • Add the headers in the build system to the fuzz binary
    • Copy-paste the connect-trace struct manually into the fuzz target code
    • Something else?
    RobinDavid commented at 8:40 AM on March 17, 2026:

    Yes I fixed it, but duplicating the definition in connect_block.cpp. It was a ConnectTrace beforhand, now it is a ConnectedBlock struct.

  9. DrahtBot commented at 4:12 PM on March 2, 2026: contributor

    Could turn into draft while CI is red?

  10. RobinDavid marked this as a draft on Mar 2, 2026
  11. RobinDavid force-pushed on Mar 16, 2026
  12. RobinDavid force-pushed on Mar 16, 2026
  13. RobinDavid force-pushed on Mar 16, 2026
  14. RobinDavid force-pushed on Mar 16, 2026
  15. RobinDavid force-pushed on Mar 16, 2026
  16. DrahtBot removed the label CI failed on Mar 16, 2026
  17. RobinDavid marked this as ready for review on Mar 17, 2026
  18. RobinDavid commented at 8:43 AM on March 17, 2026: contributor

    I fixed all the CI issues. Its ready for review now. cc @dergoegge

  19. sedited requested review from marcofleon on Mar 17, 2026
  20. in src/test/fuzz/connect_block.cpp:696 in 87c6d12d58 outdated 
     691 | +                                                  /* justCheck*/ true);
 692 | +
 693 | +    if (success) {
 694 | +        DEBUGOUTPUT(std::cout << "Block connected successfully: " << curr_header.GetHash().ToString() << std::endl);
 695 | +        DEBUGOUTPUT(std::cout << "State: " << state.ToString() << std::endl);
 696 | +        //Assert(active_chainstate.DisconnectBlock(block, &new_index, active_coins) == DISCONNECT_OK);
    marcofleon commented at 2:22 PM on March 17, 2026:

    I like this thought a lot, a connect and disconnect roundtrip test. I think they are inverses for UTXO set operations, so it would be a great correctness check. It's a shame we can't test this statelessly without refactoring. DisconnectBlock needs undo data that ConnectBlock only writes to disk when justCheck is false, so a roundtrip here would require either making the harness stateful or refactoring to keep the undo data in memory.

    RobinDavid commented at 9:15 AM on March 22, 2026:

    Hi. Ok I will create two separate PRs. As they share common functions, I will still put them in the same source file. Is it okay for you ?

    In a previous version the harness was doing the full round-trip, but lately, I encountered issues and though it would be better to keep it fully stateless. We keep it that way right ?

  21. marcofleon commented at 2:38 PM on March 17, 2026: contributor

    @dergoegge and I took a look at the connect_block harness and we think it'd be good to split that into its own PR. It's relatively straightforward, valuable on its own, and would likely get through review quicker. The other two harnesses could remain open in this PR as a draft on top of the new connect_block PR.

  22. in src/test/fuzz/connect_block.cpp:137 in 87c6d12d58 
     132 | +            /** Get the CTxOut output object  */
 133 | +            auto& vout = tx->vout[voutIndex];
 134 | +            /** Do not keep OP_RETURN outputs as they are not spendable */
 135 | +            if (vout.scriptPubKey.size() >= 1 && vout.scriptPubKey[0] == OP_RETURN) continue;
 136 | +            /** Create the CTxIn that can be used to spend this output */
 137 | +            allUTXOCTxIn[currentBlock->nHeight] = getSpendingScript(*tx, voutIndex);
    brunoerg commented at 1:36 PM on March 18, 2026:

    Not sure if it's expected, but it looks only one CTxIn per block is retained because it's being overwritten since you're using the nHeight as index.

    RobinDavid commented at 2:24 PM on March 22, 2026:

    It works has pre-mined blocks only contains the coinbase transaction that only contains one output. We did it that way to keep the vector sorted by height (as block were iterated in from tip to genesis). But to make it more generic I changed it to use push_back and reverse it at the end.

  23. in src/test/fuzz/connect_block.cpp:301 in 87c6d12d58 outdated 
     296 | +
 297 | +    /** Some harnesses want to explicitely read coinbase transactions in input */
 298 | +    if (coinbase) {
 299 | +        tx.vin.resize(1);                                   /** Size of vin hardcoded */
 300 | +        tx.vin[0].prevout.SetNull();
 301 | +        tx.vin[0].nSequence = CTxIn::MAX_SEQUENCE_NONFINAL; /** Make sure timelock is enforced. */
    brunoerg commented at 1:39 PM on March 18, 2026:

    nit: This comment looks wrong, timelock is not enforced, especially because it's a coinbase.

  24. in src/test/fuzz/connect_block.cpp:579 in 87c6d12d58 outdated 
     574 | +    /** If it does not exists write it into the ChainstateManager. */
 575 | +    if (blockIndex == nullptr) {
 576 | +        BlockValidationState state;
 577 | +
 578 | +        CBlockIndex* bestBlock = nullptr;
 579 | +        blockIndex = csm.m_blockman.AddToBlockIndex(block, bestBlock);
    marcofleon commented at 4:15 PM on March 18, 2026:

    I think pass in the chainstate manager's m_best_header instead of the local bestBlock here. That way the headers get updated along with the blocks you add to the chain. This should fix the assertion failure.

    RobinDavid commented at 2:56 PM on March 22, 2026:

    Thas weird that it did not triggered earlier. Maybe the underlying behavior of AddBlockIndex had slightly changed. Writing it as an Assert was maybe a bit to Arbitrary. I changed it to:

            CBlockIndex* bestBlock = csm.m_best_header;
        blockIndex = csm.m_blockman.AddToBlockIndex(block, bestBlock);
        /* bestBlock equals blockIndex meaning it has properly been added
        *  and the block had accumulated more proof of work than the current best header.
        *  Thus early reject block otherwise.
        */
        if(bestBlock != blockIndex){
            DEBUGOUTPUT(std::cout << "Fail to add block to BlockManager or insufficient PoW" << std::endl);
            return nullptr;
        }
  25. marcofleon commented at 4:15 PM on March 18, 2026: contributor

    Base64: BAkAAAQJdwKGAAAAgBTpAACFBAk=

    crash_activate_best_chain_step.txt

    <details> <summary>stack trace</summary>

    ===== stack trace ===== 
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 3223231957
INFO: Loaded 1 modules   (580898 inline 8-bit counters): 580898 [0x556d05d1e360, 0x556d05dac082), 
INFO: Loaded 1 PC tables (580898 PCs): 580898 [0x556d05dac088,0x556d066892a8), 
/workdir/out/libfuzzer_msan/fuzz: Running 1 inputs 1 time(s) each.
Running: /workdir/workspace/solutions/crash-aa5b43f99124f0e080b2b68ac94024c65fc8381b
validation.cpp:5502 double ChainstateManager::GuessVerificationProgress(const CBlockIndex *) const: Assertion `m_best_header->nHeight >= pindex->nHeight' failed.
==10489== ERROR: libFuzzer: deadly signal
    [#0](/bitcoin-bitcoin/0/) 0x556d01ef1db9 in __sanitizer_print_stack_trace /llvm-project/compiler-rt/lib/msan/msan.cpp:790:3
    [#1](/bitcoin-bitcoin/1/) 0x556d01e618d8 in fuzzer::PrintStackTrace() /llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5
    [#2](/bitcoin-bitcoin/2/) 0x556d01e442f3 in fuzzer::Fuzzer::CrashCallback() /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:231:3
    [#3](/bitcoin-bitcoin/3/) 0x556d01f50c3d in SignalAction(int, void*, void*) /llvm-project/compiler-rt/lib/msan/msan_interceptors.cpp:1166:3
    [#4](/bitcoin-bitcoin/4/) 0x7f65ae046def  (/lib/x86_64-linux-gnu/libc.so.6+0x3fdef) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#5](/bitcoin-bitcoin/5/) 0x7f65ae09b95b  (/lib/x86_64-linux-gnu/libc.so.6+0x9495b) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#6](/bitcoin-bitcoin/6/) 0x7f65ae046cc1 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x3fcc1) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#7](/bitcoin-bitcoin/7/) 0x7f65ae02f4ab in abort (/lib/x86_64-linux-gnu/libc.so.6+0x284ab) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#8](/bitcoin-bitcoin/8/) 0x556d03409b63 in assertion_fail(std::__1::source_location const&, std::__1::basic_string_view<char, std::__1::char_traits<char>>) /workdir/bitcoin/src/util/check.cpp:41:5
    [#9](/bitcoin-bitcoin/9/) 0x556d05435257 in bool&& inline_assertion_check<false, bool>(bool&&, std::__1::source_location const&, std::__1::basic_string_view<char, std::__1::char_traits<char>>) /workdir/bitcoin/src/util/check.h:90:13
    [#10](/bitcoin-bitcoin/10/) 0x556d05435257 in ChainstateManager::GuessVerificationProgress(CBlockIndex const*) const /workdir/bitcoin/src/validation.cpp:5502:10
    [#11](/bitcoin-bitcoin/11/) 0x556d05424776 in UpdateTipLog(ChainstateManager const&, CCoinsViewCache const&, CBlockIndex const*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) /workdir/bitcoin/src/validation.cpp:2867:5
    [#12](/bitcoin-bitcoin/12/) 0x556d05422f9e in Chainstate::UpdateTip(CBlockIndex const*) /workdir/bitcoin/src/validation.cpp:2911:5
    [#13](/bitcoin-bitcoin/13/) 0x556d0542a016 in Chainstate::ConnectTip(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const>, std::__1::vector<ConnectedBlock, std::__1::allocator<ConnectedBlock>>&, DisconnectedBlockTransactions&) /workdir/bitcoin/src/validation.cpp:3076:5
    [#14](/bitcoin-bitcoin/14/) 0x556d0543004c in Chainstate::ActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, std::__1::vector<ConnectedBlock, std::__1::allocator<ConnectedBlock>>&) /workdir/bitcoin/src/validation.cpp:3232:18
    [#15](/bitcoin-bitcoin/15/) 0x556d02312811 in (anonymous namespace)::TestChainstate::CallActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, std::__1::vector<ConnectedBlock, std::__1::allocator<ConnectedBlock>>&) /workdir/bitcoin/src/test/fuzz/connect_block.cpp:40:16
    [#16](/bitcoin-bitcoin/16/) 0x556d02312811 in activate_best_chain_step_fuzz_target(std::__1::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/src/test/fuzz/connect_block.cpp:755:32
    [#17](/bitcoin-bitcoin/17/) 0x556d01f6a365 in std::__1::__invoke_result_impl<void, void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>::type std::__1::__invoke[abi:de210108]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:87:27
    [#18](/bitcoin-bitcoin/18/) 0x556d01f6a365 in void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:de210108]<void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:342:5
    [#19](/bitcoin-bitcoin/19/) 0x556d01f6a365 in void std::__1::__invoke_r[abi:de210108]<void, void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>>(void (*&)(std::__1::span<unsigned char const, 18446744073709551615ul>), std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__type_traits/invoke.h:348:10
    [#20](/bitcoin-bitcoin/20/) 0x556d01f6a365 in std::__1::__function::__func<void (*)(std::__1::span<unsigned char const, 18446744073709551615ul>), void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>&&) /libcxx_msan/include/c++/v1/__functional/function.h:174:12
    [#21](/bitcoin-bitcoin/21/) 0x556d02d8b8c1 in std::__1::__function::__value_func<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()[abi:de210108](std::__1::span<unsigned char const, 18446744073709551615ul>&&) const /libcxx_msan/include/c++/v1/__functional/function.h:274:12
    [#22](/bitcoin-bitcoin/22/) 0x556d02d8b8c1 in std::__1::function<void (std::__1::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::__1::span<unsigned char const, 18446744073709551615ul>) const /libcxx_msan/include/c++/v1/__functional/function.h:772:10
    [#23](/bitcoin-bitcoin/23/) 0x556d02d8b8c1 in test_one_input(std::__1::span<unsigned char const, 18446744073709551615ul>) /workdir/bitcoin/src/test/fuzz/fuzz.cpp:88:5
    [#24](/bitcoin-bitcoin/24/) 0x556d02d8b8c1 in LLVMFuzzerTestOneInput /workdir/bitcoin/src/test/fuzz/fuzz.cpp:216:5
    [#25](/bitcoin-bitcoin/25/) 0x556d01e45a4b in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:619:13
    [#26](/bitcoin-bitcoin/26/) 0x556d01e2f6f2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:329:6
    [#27](/bitcoin-bitcoin/27/) 0x556d01e35590 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:864:9
    [#28](/bitcoin-bitcoin/28/) 0x556d01e62262 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    [#29](/bitcoin-bitcoin/29/) 0x7f65ae030ca7  (/lib/x86_64-linux-gnu/libc.so.6+0x29ca7) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#30](/bitcoin-bitcoin/30/) 0x7f65ae030d64 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29d64) (BuildId: fce446c9d4ad48e2b0c90cce1a11722897805281)
    [#31](/bitcoin-bitcoin/31/) 0x556d01e286e0 in _start (/workdir/out/libfuzzer_msan/fuzz+0x13696e0)

NOTE: libFuzzer has rudimentary signal handlers.
      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal

    </details>

  26. RobinDavid force-pushed on Mar 22, 2026
  27. RobinDavid force-pushed on Mar 22, 2026
  28. DrahtBot added the label CI failed on Mar 22, 2026
  29. RobinDavid force-pushed on Mar 22, 2026
  30. RobinDavid force-pushed on Mar 22, 2026
  31. DrahtBot removed the label CI failed on Mar 22, 2026
  32. RobinDavid requested review from marcofleon on Mar 24, 2026
  33. in src/test/fuzz/connect_block.cpp:490 in 673cfd3b44 outdated 
     485 | +
 486 | +    /** Try to connect the block */
 487 | +    bool success = active_chainstate.ConnectBlock(block,
 488 | +                                                  state,
 489 | +                                                  &new_index,
 490 | +                                                  active_coins,
    marcofleon commented at 4:53 PM on April 7, 2026:

    The UTXO set is updated in ConnectBlock() before the fJustCheck check later on. Those spend and add operations will persist across iterations. I think we want to pass in a dummy view (CCoinsViewCache on top of CoinsTip()) here instead of the actual thing, similar to TestBlockValidity().

    RobinDavid commented at 8:59 PM on April 8, 2026:

    Used a similar structure than in TestBlockValidity.

  34. in src/test/fuzz/connect_block.cpp:474 in 673cfd3b44 
     469 | +    CBlockHeader curr_header = static_cast<const CBlockHeader&>(block);
 470 | +
 471 | +    BlockValidationState state;
 472 | +    /** Perform basic checks on the block to early reject it */
 473 | +    const auto& consensus = test_setup->m_node.chainman->GetConsensus();
 474 | +    if (!CheckBlock(block, state, consensus)) {
    marcofleon commented at 5:05 PM on April 7, 2026:

    Do we need this check here? CheckBlock is called fairly soon in ConnectBlock(). And with fJustCheck=true it skips the merkle root and PoW checks, so this call in the harness rejects blocks that ConnectBlock() would accept.

    RobinDavid commented at 8:58 PM on April 8, 2026:

    I removed the check.

  35. Implement connect_block fuzzing harness 6fceab38b4
  36. RobinDavid force-pushed on Apr 8, 2026
  37. ismaelsadeeq commented at 3:30 PM on April 10, 2026: member

    Concept ACK

  38. in src/test/fuzz/connect_block.cpp:480 in 6fceab38b4 
     475 | +    new_index.nHeight = active_tip->nHeight + 1;
 476 | +    new_index.phashBlock = &currentHash;
 477 | +
 478 | +    /** Try to connect the block */
 479 | +    BlockValidationState state;
 480 | +    bool success = active_chainstate.ConnectBlock(block,
    ismaelsadeeq commented at 3:32 PM on April 10, 2026:

    Should we first pass the block to TestBlockValidity before connect Block? See rationale #35000 (comment)

    RobinDavid commented at 8:10 PM on April 10, 2026:

    TestBlockValidity already calls ConnectBlock so if we do call it, we do not need to call ConnectBlock in the harness anymore. If it is to leverage the fact that it calls ContextualCheckBlockHeader and ContextualCheckBlock, I am calling these functions in the other harness #34895 that connects above in the call chain on ActivateBestChainStep / ActivateBestChain. As such, I would keep it as-is to keep it simple, but I eager to hear other opinions on this.

    ismaelsadeeq commented at 2:16 PM on April 13, 2026:

    We should just call TestBlockValidity here and not call connectBlock since it is implicitly called and has the same setup as your connectBlock in this harness, including the creation of a dummy index. This gives us a harness for block validity, and we should also assert that the tip has not changed after calling TestBlockValidity.

    marcofleon commented at 1:52 PM on April 14, 2026:

    @ismaelsadeeq and I discussed offline a bit, and we decided to keep the harness in this PR as is. Fuzzing TestBlockValidity on its own would be useful with the right oracles, which we can figure out in #35000.

    RobinDavid commented at 8:10 PM on April 14, 2026:

    Ok great! Fine for me.

    Is the PR ready to be merged then ?

    marcofleon commented at 5:54 PM on April 20, 2026:

    @RobinDavid, you can see more about the review process here.

    This is an example of a recent fuzz test that went through review and was merged.

  39. ismaelsadeeq commented at 2:19 PM on April 13, 2026: member

    s/fuzz: Block connection and chain reorganization fuzzing harnesses/fuzz: Block connection

  40. in src/test/fuzz/connect_block.cpp:5 in 6fceab38b4 
       0 | @@ -0,0 +1,498 @@
   1 | +// Copyright (c) 2025-present The Bitcoin Core developers
   2 | +// Distributed under the MIT software license, see the accompanying
   3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +
   5 | +#include <algorithm>
    ismaelsadeeq commented at 9:57 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    nit: Standard library headers usually aren't mixed with project headers. They come after project header, with some line separation between the

  41. in src/test/fuzz/connect_block.cpp:1 in 6fceab38b4 
       0 | @@ -0,0 +1,498 @@
   1 | +// Copyright (c) 2025-present The Bitcoin Core developers
    ismaelsadeeq commented at 9:58 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    // Copyright (c) The Bitcoin Core developers
  42. in src/test/fuzz/connect_block.cpp:26 in 6fceab38b4 
      21 | +
  22 | +
  23 | +namespace {
  24 | +
  25 | +
  26 | +/** Testing setup used by the three harnesses */
    ismaelsadeeq commented at 9:59 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    This is incorrect; there is only one harness here.

  43. in src/test/fuzz/connect_block.cpp:20 in 6fceab38b4 
      15 | +#include <test/util/setup_common.h>
  16 | +#include <validationinterface.h>
  17 | +#include <validation.h>
  18 | +
  19 | +
  20 | +#define DEBUGOUTPUT(x) //(x);
    ismaelsadeeq commented at 10:00 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    Why is this needed? And later the std::cout? I guess it's used for debugging; not sure whether it is ideal to merge with this.

    marcofleon commented at 3:38 PM on April 20, 2026:

    nit: We don't usually keep debug logs around in tests afaik, so this should be removed.

  44. in src/test/fuzz/connect_block.cpp:7 in 6fceab38b4 
       0 | @@ -0,0 +1,498 @@
   1 | +// Copyright (c) 2025-present The Bitcoin Core developers
   2 | +// Distributed under the MIT software license, see the accompanying
   3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +
   5 | +#include <algorithm>
   6 | +#include <consensus/merkle.h>
   7 | +#include <kernel/disconnected_transactions.h>
    ismaelsadeeq commented at 10:01 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    This include is not needed.

  45. in src/test/fuzz/connect_block.cpp:495 in 6fceab38b4 
     490 | +    }
 491 | +    else {
 492 | +        DEBUGOUTPUT(std::cout << "Block connection failed: " << state.GetRejectReason() << std::endl);
 493 | +        // If the connection failed, we can still try to disconnect the block
 494 | +        // to ensure that the disconnect logic is robust.
 495 | +    }
    ismaelsadeeq commented at 10:08 AM on April 17, 2026:

    In 6fceab38b4be167d2014796eecc962270b82892c "Implement connect_block fuzzing harness"

    This is good for debugging, but it won't help us. If you'd like to document that the goal is to test a connect/disconnect block round trip, please proceed. A comment is ideal; why can't we do that now? Maybe link to @marcofleon's comment permalink.

    Also, we can test some invariants; when valid, no reject reason should be added vis-à-vis

    marcofleon commented at 6:01 PM on April 20, 2026:

    I agree it's better to drop this whole chunk, unless there is some truly useful assertion we can make upon success/failure. Doing the connect-disconnect block roundtrip would require a refactor in validation, which I may tackle at some point.

  46. in src/test/fuzz/connect_block.cpp:32 in 6fceab38b4 
      27 | +TestingSetup* test_setup{nullptr};
  28 | +/** Vector of blocks to keep a references on blocks (to enable fuzzing input to pick one to build upon)  */
  29 | +static std::vector<std::shared_ptr<CBlock>> listBlocks;
  30 | +/** Set of block hashes in listBlocks */
  31 | +static std::set<uint256> existingBlockHash;
  32 | +/** Spending script for all UTXOs (excluding OP_RETURN) including not mature CoinBase and already spend one */
    marcofleon commented at 3:40 PM on April 20, 2026:

    nit: spent instead of spend

  47. ismaelsadeeq commented at 4:09 PM on April 20, 2026: member

    Code review 392fac5d0c459cd4eede0f4b9cc7c868b1f0dc29

    I have spent some time reviewing this PR and the related #34651, which includes additional fuzz harnesses. After several iterations of review, I think I have a good understanding of what this PR is doing and what it aims to achieve.

    The PR can be simplified significantly. I started inline comments but switched rather than leaving many inline comments, I think it would be cleaner to share a few fixup commits that demonstrate my suggestions.

    <details> <summary>footnotes</summary>

    </details>

  48. in src/test/fuzz/connect_block.cpp:218 in 6fceab38b4 
     213 | +
 214 | +/**
 215 | + * Initialize the chain for the three harnesses.
 216 | + */
 217 | +static void initialize_connect_block() {
 218 | +    Assert(EnableFuzzDeterminism());
    marcofleon commented at 5:23 PM on April 20, 2026:

    nit: This is redundant, so it should be removed. And it would prevent (crash on) the use of the nondeterminism env that we have in our fuzzing framework

  49. in src/test/fuzz/connect_block.cpp:284 in 6fceab38b4 
     279 | +        if (fuzzed_data_provider.ConsumeBool()) {           /** 1/2 probability to get a valid vin */
 280 | +            tx.vin[0].scriptSig = CScript() << targetHeight << OP_0;
 281 | +        } else {
 282 | +            /** Read arbitrary data in input as scriptSig */
 283 | +            auto scriptSig = ConsumeRandomLengthByteVector<unsigned char>(fuzzed_data_provider, 100);
 284 | +            tx.vin[0].scriptSig << scriptSig;
    marcofleon commented at 5:33 PM on April 20, 2026:

    nit: Could make sense to replace these two lines with the existing ConsumeScript helper.

    Something like tx.vin[0].scriptSig = ConsumeScript(fuzzed_data_provider) or appending its output if that was the intent.

    Same goes for the scriptSig in the else branch below and the scriptPubKey in the switch statement below.

  50. in src/test/fuzz/connect_block.cpp:319 in 6fceab38b4 
     314 | +            }
 315 | +            if (fuzzed_data_provider.ConsumeBool()) {
 316 | +                tx.vin[i].scriptWitness.stack.clear();
 317 | +                int numWit = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 10);
 318 | +                for (int j = 0; j < numWit; j++) {
 319 | +                    tx.vin[i].scriptWitness.stack.push_back(ConsumeRandomLengthByteVector<unsigned char>(fuzzed_data_provider, 100));
    marcofleon commented at 5:42 PM on April 20, 2026:

    nit: Could replace this with ConsumeScriptWitness.

  51. in src/test/fuzz/connect_block.cpp:303 in 6fceab38b4 
     298 | +                tx.vin[i] = additionalUTXO[targetUTXO - allUTXOCTxIn.size()];
 299 | +            }
 300 | +
 301 | +            /** Enable fuzzer to mutate every CTxIn fields after it being taken from existing UTXOs */
 302 | +            if (fuzzed_data_provider.ConsumeBool()) {
 303 | +                tx.vin[i].nSequence = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
    marcofleon commented at 5:43 PM on April 20, 2026:

    nit: Could use ConsumeSequence instead.

  52. marcofleon commented at 5:59 PM on April 20, 2026: contributor

    ACK 6fceab38b4be167d2014796eecc962270b82892c

    I think this test accomplishes its goal and could be merged, despite it being a bit messy. There are several clean up nits which could be addressed in a followup PR or in #34895.

    style nit: Some variables throughout the test use camelCase, where snake_case would be preferred (dev notes).

  53. DrahtBot requested review from ismaelsadeeq on Apr 20, 2026
  54. ismaelsadeeq commented at 9:31 AM on April 21, 2026: member

    I think this test accomplishes its goal and could be merged, despite it being a bit messy. There are several clean up nits which could be addressed in a followup PR or in #34895.

    Why the rush? I would like to see my suggestions fixed or addressed.

    https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#decision-making-process

    In general, all pull requests must: Be well peer-reviewed; Follow code style guidelines (C++, functional tests);

    I tend to disagree here; I think merging a messy change lowers the bar.

  55. marcofleon commented at 2:54 PM on April 21, 2026: contributor

    Why the rush?

    The fuzz harnesses in this PR and in #34895 were part of an external security audit and were made without all the specifics of this project in mind. I figure if the test is logically sound, adds good coverage, and does the job it's meant to do, then there isn't a need for us to stall it with style nits and refactors. A member of the project could likely more quickly and easily get those done and merged in a followup.

    Looking at it again, I do think, at the minimum, the DEBUGOUTPUT(x) and all related code should be dropped before this PR is merged, so my ack was premature.

    I'm not looking to rush, more just trying not to stall a test-only addition of fuzz targets that overall look effective to me. Ultimately, I agree with this project's high bar and of course, am willing to go through the necessary rounds of review.

  56. ismaelsadeeq commented at 3:55 PM on April 21, 2026: member

    That makes sense, though it wasn't clear from your ACK or the PR description. You mentioned merging despite the messiness being okay, which is what made me push back a bit. Glad we're on the same page now. I'd note that "fix in a follow-up" doesn't always happen.

    Also, thanks for the audit and this work @RobinDavid. Given that this originated from external audit work, I'd like to suggest a potentially better approach that might be more efficient for everyone:

    1. Have a project member take over and apply the suggested changes, as they're familiar with the codebase and conventions, which would save both the researchers and reviewers bandwidth.

    2. Open a follow-up PR with a committed author who will see it through. In past cases, when the author opens the follow-up and directly references the outstanding comments, it signals continuity e.g #33189, #33591, #29424 and I'd be happy to ACK that while we work through the remaining suggestions in that branch.

  57. sedited commented at 7:09 PM on April 23, 2026: contributor

    Have a project member take over and apply the suggested changes, as they're familiar with the codebase and conventions, which would save both the researchers and reviewers bandwidth.

    I think I would prefer this, given that we have made slow progress here. @RobinDavid is that good with you?

  58. RobinDavid commented at 9:34 PM on April 23, 2026: contributor

    Yes I only work on this PR on my free time so I can't make it progress as fast as I'd want. So yes @sedited and @ismaelsadeeq I am fine to hand over applying these changes, I agree with all of them.

    I would be even better if you could hand over the second PR #34895 which would requires more significant refactoring than this one.

  59. ismaelsadeeq commented at 7:06 PM on April 28, 2026: member

    Yes I only work on this PR on my free time so I can't make it progress as fast as I'd want. So yes @sedited and @ismaelsadeeq I am fine to hand over applying these changes, I agree with all of them.

    I would be even better if you could hand over the second PR #34895 which would requires more significant refactoring than this one. @RobinDavid cool.

    I talked with @marcofleon a few days ago, and we concluded that I will incorporate the suggestions here and open a new PR. I expected it to be up by today, but did not because I got sucked into parallel work :(. I will focus on this for the rest of the week and have a PR up soon.

  60. dergoegge commented at 1:01 PM on April 29, 2026: member

    Why the rush?

    I agree that there is no rush, so I don't see the need for this to be handed over (unless Robin has no interest in maintaining this, which I think he has, given that he is doing this in his spare time). These types of PRs take months or years on a regular basis (regardless of who the author is). The bottleneck is primarily review.

    This PR was opened in February but review started later on, and splitting the connect_block harness into it's own PR (i.e. what we now have in this PR) was only suggested a little more than a month ago (https://github.com/bitcoin/bitcoin/pull/34651#pullrequestreview-3961182288).

    Have a project member take over and apply the suggested changes, as they're familiar with the codebase and conventions, which would save both the researchers and reviewers bandwidth

    Open a follow-up PR with a committed author who will see it through

    I don't know if an LLM wrote this or if it's your opinion, but I don't think this is a fair thing to say, given that Robin has made an obviously genuine attempt at contributing and has been responsive throughout.

    Contributing to this project is not limited to members (GitHub org membership means nothing beyond being able to edit the wiki and restart CI jobs), and should be possible without getting paid to do so full time. Someone showing up to contribute in their spare time is great, and also how a lot of us started out (thank you Robin for contributing your time!).

    From what I've seen, the harness here works as intended. It should be cleaned up before merge (style, comments, etc.) but there is no need to completely re-write it (as suggested here #34651#pullrequestreview-4127784272). @RobinDavid if you're still interested to work on this when you have the time, I'd be happy to ACK this once the nits have been addressed.

  61. ismaelsadeeq commented at 1:17 PM on April 29, 2026: member

    I don't know if an LLM wrote this or if it's your opinion, but I don't think this is a fair thing to say, given that Robin has made an obviously genuine attempt at contributing and has been responsive throughout.

    I wrote this to suggest moving forward because of the eagerness to merge I see here #34651 (review) and #34651#pullrequestreview-4141384168, for the reason mentioned #34651 (comment), which I believe other contributors agree with me on this.

    Contributing to this project is not limited to members (GitHub org membership means nothing beyond being able to edit the wiki and restart CI jobs), and should be possible without getting paid to do so full time. Someone showing up to contribute in their spare time is great, and also how a lot of us started out (thank you Robin for contributing your time!

    I am happy to have this continue review as well and will ACK as well, even without a rewrite, as long as the suggestions are addressed. I am not in any rush to take over someone's work; I did almost similar work in parallel, and and @marcofleon and you suggested we close and focus on this, which is what I am doing now. I am here in good faith, wanting to move this work forward and have this harness up so that it can be used beyond this #34651#pullrequestreview-4127784272.

  62. ismaelsadeeq commented at 3:31 PM on April 29, 2026: member

    Re #34651 (comment) see https://github.com/bitcoin/bitcoin/compare/master...ismaelsadeeq:bitcoin:block-chain-validation-harness branch with all comments addressed and an added harness using #34651#pullrequestreview-4127784272.

  63. RobinDavid commented at 10:15 PM on April 29, 2026: contributor

    @ismaelsadeeq the branch https://github.com/ismaelsadeeq/bitcoin/tree/block-chain-validation-harness do not seems to just address the aforementioned remarks. It is significantly different than this current PR (that only contains one fuzz harness). Do we want to restore all 3 harnesses (connect_block, activate_best_chain_step, activate_best_chain) and a new one into a single PR ? I though we wanted to keep them split. I would happy to take @marcofleon and @dergoegge opinion on this.

  64. marcofleon commented at 2:00 PM on May 12, 2026: contributor

    Do we want to restore all 3 harnesses (connect_block, activate_best_chain_step, activate_best_chain) and a new one into a single PR ?

    I think we should keep it split up, as we decided earlier. Makes it easier to review. This PR would map to the first commit of Sadiq's branch: https://github.com/bitcoin/bitcoin/commit/1a34a13449542b9715eccab8fe04f764cddfdcda

    I've taken a better look at https://github.com/ismaelsadeeq/bitcoin/tree/block-chain-validation-harness and I think the refactoring of shared setup and helpers into util files makes sense. We can reuse them for future validation harnesses.

    If this PR were generally kept as is (all in one file) and cleaned up style-wise as reviewers have suggested above, then I would ack it and also review a refactor followup. @RobinDavid another option could be to take https://github.com/bitcoin/bitcoin/commit/1a34a13449542b9715eccab8fe04f764cddfdcda for this PR (or re-work parts of it if something stands out to you), essentially accomplishing the refactor here, which would then be used for #34895 later on. It's up to you, either option would work fine for me.

  65. in src/test/fuzz/connect_block.cpp:395 in 6fceab38b4 
     390 | +
 391 | +    /** Give the fuzzer input the ability to mutate block header fields */
 392 | +    if (fuzzed_data_provider.ConsumeBool()) {
 393 | +        block.nVersion = fuzzed_data_provider.ConsumeIntegral<int32_t>();
 394 | +    }
 395 | +    if (fuzzed_data_provider.ConsumeBool() && !forceValidBlock) {
    marcofleon commented at 2:18 PM on May 12, 2026:

    What was the intention here with forceValidBlock? Is it to create a fully valid header that would pass the contextual checks in validation.cpp? If that's the case, I think it should be applied to the other three header fields as well (nVersion, nBits, nTime).

    Also, I think force_valid_header would be a clearer name.

Contributors
RobinDavidDrahtBotbrunoergmaflckomarcofleonismaelsadeeqsediteddergoegge


ismaelsadeeq

Labels
Fuzzing
Linked (view graph)
#34895 fuzz: Fuzzing harnesses for ActivateBestChainStep and ActivateBestChain
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-18 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me