#36234 util: avoid redundant rolling Bloom insertions

full analysis

https://github.com/bitcoin/bitcoin/pull/36234 · JeremyRubin · +74/-3 in 3 files, 1 commits · labels: Utils/log/libs

Goal

  • Prevent repeated insertions of the same item from prematurely expiring older entries in rolling filters
  • Ensures filters retain tracked items up to their configured capacity even under duplicate inputs

Modifies `CRollingBloomFilter::insert` to check whether an item is already present in the active generation before advancing generation counters. If the item is already present in the current generation, it skips counter increments and generation rotation while reusing cached hash calculations.

Problem: Inserting the same item repeatedly into a `CRollingBloomFilter` advances its generation counters as if new items were being added. This can prematurely rotate generations and flush older, unrelated filter entries before configured capacity is reached.

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

P3 · bug fix

  • P3 because rolling Bloom filters are shared across networking to track seen items without early eviction
  • Prevents duplicate items from purging older data, though no caller vulnerability or benchmark was shown

CRollingBloomFilter is a shared utility used across networking to track seen items, where premature eviction can cause unnecessary re-requests. The author notes that 'repeating one value can therefore expire unrelated entries sooner than the configured capacity implies', but provides no specific caller vulnerability or performance numbers, making this a worthwhile correctness improvement without high urgency.

Membership: Changes the core `CRollingBloomFilter` utility in `src/common/bloom.cpp`.

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

Reviewability: Ready

  • Ready for review
  • Small patch with passing CI and dedicated unit tests

The patch is small, cleanly merges, passes CI, and includes dedicated unit test coverage.

Author status: silent since opening 4 days ago, with an open question from a reviewer for 2 days.

Open concerns:

  • Whether this addresses a known downstream usage issue or is motivated by general correctness (asked by instagibbs).

Agreement: Crickets

  • No concept or approach reviews yet
  • Asked whether this fixes a known caller problem or is just a general correctness improvement (instagibbs)

No concept or approach verdicts; instagibbs asked about motivation.

Only one question has been asked regarding whether any worrying usage patterns motivated the change, with no approvals or objections submitted yet.

  • instagibbs inquired if there are worrying usage patterns or if this is a general correctness fix.

Review verdicts (DrahtBot): 0

Files

52 lines under test/bench/ci.

  • src/test/bloom_tests.cpp +52/-0
  • src/common/bloom.cpp +20/-3
  • src/common/bloom.h +2/-0

Uncertainties

  • Whether any specific P2P message handler currently permits an external peer to trigger rapid duplicate insertions and flush recent inventory filters.

Card

This PR updates CRollingBloomFilter::insert to detect whether an item is already present in the current generation, avoiding premature generation rotations and eviction of older entries on repeated inputs. It also caches the computed hashes so that repeated hashing is avoided. This addresses an edge case in rolling bloom filter capacity tracking, which protects against unrelated items expiring too early. The change is small, includes unit tests, and is awaiting author feedback on a reviewer question about concrete usage patterns.

Data

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