#35818 bloom: avoid undefined sizing calculations

full analysis

https://github.com/bitcoin/bitcoin/pull/35818 · l0rinc · +75/-4 in 4 files, 4 commits · labels: none

Goal

  • Avoid division by zero and undefined floating-point conversions during bloom filter sizing
  • Prevents crashes and undefined behavior when running unit tests and fuzz targets with edge-case inputs

This PR fixes undefined floating-point conversion and division by zero in the four-argument `CBloomFilter` constructor. It validates that the false-positive rate is within [0, 1], handles zero elements and zero false-positive rates explicitly, clamps the calculated filter size before converting from floating point to an unsigned integer, and expands unit and fuzz test coverage for boundary parameters.

Problem: The four-argument `CBloomFilter` constructor, used by unit tests and the fuzz target, divides by zero when given zero elements and causes undefined behavior when converting out-of-range floating-point results to `unsigned int`.

Category: Utilities (logging, arguments, libraries) (#36 of 66)

P4 · bug fix

  • P4 because the fix prevents undefined behavior and crashes during bloom filter sizing
  • Limited to edge cases in unit tests and fuzz targets rather than production node paths

Fixes floating-point undefined behavior and division by zero in CBloomFilter sizing, but the affected constructor is used exclusively in tests and fuzzing rather than production node paths.

Membership: Modifies CBloomFilter utility implementation and sizing calculations in src/common/bloom.cpp.

Factors: security/stability 1, bug 1, performance 0, user value 0, leverage 0

Reviewability: Ready

  • Ready for review with passing CI and no blocking requests

There are no blocking reviewer requests, CI is passing, and rustaceanrob has code-review ACKed the current commit.

Author status: silent since 2026-08-05 after addressing reviewer suggestions and agreeing to follow-up work

Resolved concerns:

  • rustaceanrob suggested moving the test-only constructor behind a test utility factory, but agreed that it should be deferred to a follow-up PR.

Agreement: Positive

  • Full code review approval with no remaining objections (rustaceanrob)
  • Agreed to defer moving test construction logic into a helper to a follow-up (rustaceanrob)

Positive: code review ACK from rustaceanrob with no objections

rustaceanrob reviewed and ACKed the commit after agreeing to defer architectural refactoring to a follow-up.

  • rustaceanrob: Concept ACK and later 'Code review ACK c4302ddf80'
  • rustaceanrob agreed with l0rinc that moving test construction into test utilities can happen in a follow-up

Review verdicts (DrahtBot): 1

Files

64 lines under test/bench/ci.

  • src/test/bloom_tests.cpp +51/-0
  • src/common/bloom.cpp +11/-2
  • src/test/fuzz/bloom_filter.cpp +11/-2
  • src/common/bloom.h +2/-0

Card

This PR fixes undefined floating-point conversion and division by zero in the four-argument CBloomFilter constructor by validating false-positive rates and clamping size calculations before conversion. The affected constructor is only invoked in unit tests and fuzzing harnesses, meaning production bitcoind operations are unaffected. The PR also expands fuzz and unit test coverage across boundary conditions. Review is complete with a Code Review ACK from rustaceanrob and no outstanding objections.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:21, confidence high, input hash 6af686a05225522c