bloom: avoid undefined sizing calculations #35818

l0rinc commented at 11:27 PM on July 26, 2026: contributor

Problem: The four-argument CBloomFilter constructor is used by unit tests and the fuzz target to exercise Bloom filter sizing directly. Zero elements make the hash-function calculation divide by zero. The size calculation converts its floating-point result to unsigned int before applying the protocol limit, which is undefined for zero or invalid false-positive rates and for valid parameters whose calculated size exceeds the integer range.

Fix: Require false-positive rates in [0, 1], construct the existing empty match-all state for zero elements, and use the largest permitted filter for a zero rate with positive elements. Clamp the calculated size before its integer conversion so large valid inputs remain within the protocol limit.

DrahtBot commented at 11:27 PM on July 26, 2026: contributor

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

Code Coverage & Benchmarks

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

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	rustaceanrob

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

test: characterize bloom filter sizing

Exercise ordinary sizing, the protocol size clamp, the match-all rate, tiny positive finite rates, and existing empty-filter behavior without relying on behavior changed by later commits.

3614b38e33

bloom: avoid undefined sizing calculations

The four-argument CBloomFilter constructor is currently used only by Bloom filter tests and the fuzz target, which need it to exercise the sizing calculations directly.

Zero elements make the hash-function formula divide by zero, and the size formula can convert NaN, infinity, a negative value, or a finite value outside the unsigned range before applying the protocol clamp.

Require false-positive rates in [0, 1], construct the existing empty-filter state for zero elements, use the maximum permitted size for a zero rate with positive elements, and clamp the size in floating-point space before converting it.

7705577116

l0rinc force-pushed on Jul 28, 2026

l0rinc renamed this:
~~bloom: support zero-element filters~~
bloom: avoid undefined sizing calculations
on Jul 28, 2026

DrahtBot added the label CI failed on Jul 28, 2026

DrahtBot commented at 2:19 AM on July 28, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/30319855016/job/90153267675</sub> <sub>LLM reason (✨ experimental): CI failed because the bloom_tests/bloom_create_invalid_false_positive_rate unit test crashed with a critical std::fetestexcept check failure (1 != 0).</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>

test: cover bloom sizing boundaries

Cover zero-element and zero-rate behavior, and clamping before unsigned conversion at the maximum element count.
Reject finite out-of-range values, infinities, quiet NaN, and signaling NaN.
Check that the covered cases other than signaling NaN do not raise floating-point exceptions.
Merely classifying a signaling NaN may raise FE_INVALID.

3acd9ae901

fuzz: cover bloom sizing boundaries

Exercise the full unsigned element-count range and generate ordinary valid false-positive rates across [0, 1] together with exact floating-point boundaries.

Generate the rate independently from the element count so zero-element filters exercise the same valid rate range instead of only zero, while empty input still exercises (0, 0).

c4302ddf80

l0rinc force-pushed on Jul 28, 2026

l0rinc closed this on Jul 28, 2026

l0rinc reopened this on Jul 28, 2026

DrahtBot removed the label CI failed on Jul 28, 2026

in src/common/bloom.cpp:35 in c4302ddf80

  30 | +    if (elements == 0) return 0; // Must precede the zero-rate case to preserve empty-filter semantics.
  31 | +    if (false_positive_rate == 0) return MAX_BLOOM_FILTER_SIZE; // Avoid log(0) raising FE_DIVBYZERO.
  32 | +    return std::min(-1 / LN2SQUARED * elements * log(false_positive_rate), MAX_BLOOM_FILTER_SIZE * 8.0) / 8;
  33 | +}
  34 | +
  35 |  CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweakIn, unsigned char nFlagsIn) :

rustaceanrob commented at 12:13 PM on August 4, 2026:

I think this suggestion is too large to do right now, but perhaps can be done as follow up. Given this is only used in test code, perhaps this can be defined in a test/util/bloom.h or similar. A free function could then be declared as a friend and defined in the test util, which could also contain the BloomFilterSize function. This makes it obvious the Assert in that function body is only reachable in test source.

namespace bloom_test {
CBloomFilter MakeBloomFilter(...)
}

class CBloomFilter {
private:
    friend CBloomFilter bloom_tests::MakeBloomFilter(...)
}

l0rinc commented at 10:55 PM on August 5, 2026:

perhaps can be done as follow up

Indeed, we can move construction and sizing behind a test-only factory in a follow-up so this PR stays focused on making the existing sizing calculations well-defined.

rustaceanrob commented at 12:32 PM on August 4, 2026: member

Concept ACK

rustaceanrob commented at 10:34 AM on August 14, 2026: member

Code review ACK c4302ddf806094fcc2b1e21593e25b2038191a07