Merge bitcoin/bitcoin#30980: fuzz: fix bug in p2p_headers_presync harness
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run

a7498cc7e2 Fix bug in p2p_headers_presync harness (marcofleon)

Pull request description:

  The calculation for the test chain's work (`total_work`) should be outside of the loop. Previously, `total_work` was being miscalculated due to multiple additions of work from the same headers. Now, each header's work is only counted once, providing an accurate total.

  https://github.com/bitcoin/bitcoin/pull/30918 followup

ACKs for top commit:
  dergoegge:
    utACK a7498cc7e2
  instagibbs:
    ACK a7498cc7e2
  glozow:
    makes sense, utACK a7498cc7e2
  mzumsande:
    ACK a7498cc7e2

Tree-SHA512: b95f25dcf7ace220e30f1d72f50d85ee18777467927c0cc1ed8582b390cb7185ffc0e2f127309eb083044fb41f5a13fce5ebb15b7952718a899bafff26921be8
This commit is contained in:
glozow 2024-10-02 20:58:05 -04:00
commit cfb59da4b3
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4

View file

@ -197,16 +197,16 @@ FUZZ_TARGET(p2p_headers_presync, .init = initialize)
auto headers_msg = NetMsg::Make(NetMsgType::BLOCK, TX_WITH_WITNESS(block));
g_testing_setup->SendMessage(fuzzed_data_provider, std::move(headers_msg));
});
// This is a conservative overestimate, as base is only moved forward when sending headers. In theory,
// the longest chain generated by this test is 1600 (FUZZ_MAX_HEADERS_RESULTS * 100) headers. In that case,
// this variable will accurately reflect the chain's total work.
total_work += CalculateClaimedHeadersWork(all_headers);
// This test should never create a chain with more work than MinimumChainWork.
assert(total_work < chainman.MinimumChainWork());
}
// This is a conservative overestimate, as base is only moved forward when sending headers. In theory,
// the longest chain generated by this test is 1600 (FUZZ_MAX_HEADERS_RESULTS * 100) headers. In that case,
// this variable will accurately reflect the chain's total work.
total_work += CalculateClaimedHeadersWork(all_headers);
// This test should never create a chain with more work than MinimumChainWork.
assert(total_work < chainman.MinimumChainWork());
// The headers/blocks sent in this test should never be stored, as the chains don't have the work required
// to meet the anti-DoS work threshold. So, if at any point the block index grew in size, then there's a bug
// in the headers pre-sync logic.