Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31213: fuzz: Fix difficulty target generation i…
Browse files Browse the repository at this point in the history
…n `p2p_headers_presync`

a6ca8f3 fuzz: Fix difficulty target generation in p2p_headers_presync (marcofleon)
fa327c7 util: Add ConsumeArithUInt256InRange fuzzing helper (marcofleon)

Pull request description:

  In the `p2p_headers_presync` fuzz target, this assertion failed:
  ```
   assert(total_work < chainman.MinimumChainWork());
   ```
  Input that triggered the failure: [p2ppresync_crash.txt](https://github.com/user-attachments/files/17620203/p2ppresync_crash.txt)

  The test previously used `ConsumeIntegralInRange` to generate header difficulty targets within a hardcoded range. The fuzzer found specific values in that range that correspond to very low thresholds due to how [`SetCompact`][setcompact-link] works. The total work of a long enough test chain ended up exceeding `MinimumChainWork`.

  Fix this by adding a new `ConsumeArithUInt256InRange` helper function and use it in the fuzz test to generate target values within the originally intended range. The target is then converted to an `nBits` value using `GetCompact()`.

  For some more context, see bitcoin/bitcoin#30918.

  [setcompact-link]: https://github.com/bitcoin/bitcoin/blob/6463117a29294f6ddc9fafecfd1e9023956cc41b/src/arith_uint256.h#L251-L271

ACKs for top commit:
  instagibbs:
    ACK bitcoin/bitcoin@a6ca8f3
  dergoegge:
    Code review ACK a6ca8f3
  brunoerg:
    code review ACK a6ca8f3

Tree-SHA512: 92013d9d37bd3f11992ee678ba9745196efbdc4d773fd14994116629260bea46ffc9fa3923d443af7b623d39c6211900ce98a349c62ad1976e12312c37ef9df0
  • Loading branch information
fanquake committed Nov 20, 2024
2 parents 15c1f47 + a6ca8f3 commit 116b8c5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
22 changes: 18 additions & 4 deletions src/test/fuzz/p2p_headers_presync.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include <arith_uint256.h>
#include <blockencodings.h>
#include <net.h>
#include <net_processing.h>
Expand Down Expand Up @@ -89,10 +90,23 @@ CBlockHeader ConsumeHeader(FuzzedDataProvider& fuzzed_data_provider, const uint2
{
CBlockHeader header;
header.nNonce = 0;
// Either use the previous difficulty or let the fuzzer choose
header.nBits = fuzzed_data_provider.ConsumeBool() ?
prev_nbits :
fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0x17058EBE, 0x1D00FFFF);
// Either use the previous difficulty or let the fuzzer choose. The upper target in the
// range comes from the bits value of the genesis block, which is 0x1d00ffff. The lower
// target comes from the bits value of mainnet block 840000, which is 0x17034219.
// Calling lower_target.SetCompact(0x17034219) and upper_target.SetCompact(0x1d00ffff)
// should return the values below.
//
// RPC commands to verify:
// getblockheader 000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f
// getblockheader 0000000000000000000320283a032748cef8227873ff4872689bf23f1cda83a5
if (fuzzed_data_provider.ConsumeBool()) {
header.nBits = prev_nbits;
} else {
arith_uint256 lower_target = UintToArith256(uint256{"0000000000000000000342190000000000000000000000000000000000000000"});
arith_uint256 upper_target = UintToArith256(uint256{"00000000ffff0000000000000000000000000000000000000000000000000000"});
arith_uint256 target = ConsumeArithUInt256InRange(fuzzed_data_provider, lower_target, upper_target);
header.nBits = target.GetCompact();
}
header.nTime = ConsumeTime(fuzzed_data_provider);
header.hashPrevBlock = prev_hash;
header.nVersion = fuzzed_data_provider.ConsumeIntegral<int32_t>();
Expand Down
16 changes: 16 additions & 0 deletions src/test/fuzz/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,22 @@ template <typename WeakEnumType, size_t size>
return UintToArith256(ConsumeUInt256(fuzzed_data_provider));
}

[[nodiscard]] inline arith_uint256 ConsumeArithUInt256InRange(FuzzedDataProvider& fuzzed_data_provider, const arith_uint256& min, const arith_uint256& max) noexcept
{
assert(min <= max);
const arith_uint256 range = max - min;
const arith_uint256 value = ConsumeArithUInt256(fuzzed_data_provider);
arith_uint256 result = value;
// Avoid division by 0, in case range + 1 results in overflow.
if (range != ~arith_uint256(0)) {
const arith_uint256 quotient = value / (range + 1);
result = value - (quotient * (range + 1));
}
result += min;
assert(result >= min && result <= max);
return result;
}

[[nodiscard]] std::map<COutPoint, Coin> ConsumeCoins(FuzzedDataProvider& fuzzed_data_provider) noexcept;

[[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept;
Expand Down

0 comments on commit 116b8c5

Please sign in to comment.