Skip to content

Conversation

@ZocoLini
Copy link
Collaborator

@ZocoLini ZocoLini commented Dec 17, 2025

This PR removed the bloom module since it is not being used in the codebase. The only reference to BloomFitlers in the crate is the following:

/// Strategy for handling mempool (unconfirmed) transactions.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum MempoolStrategy {
    /// Fetch all announced transactions (high bandwidth, sees all transactions).
    FetchAll,
    /// Use BIP37 bloom filters (moderate privacy, good efficiency).
    BloomFilter,
}

That is being use here:

    /// Check if we should fetch a transaction based on its txid.
    pub async fn should_fetch_transaction(&self, _txid: &Txid) -> bool {
        match self.strategy {
            MempoolStrategy::FetchAll => {
                // Check if we're at capacity
                let state = self.mempool_state.read().await;
                state.transactions.len() < self.max_transactions
            }
            MempoolStrategy::BloomFilter => {
                // For bloom filter strategy, we would check the bloom filter
                // This is handled by the network layer
                true
            }
        }
    }

It sais that the network is handling the filtering. Couldn't find the Network really handling this, maybe is the dashcore crate.

Would like confirmation if this can be removed or if we need to implement it. If we can remove it I would like to add to this PR a commit removing the should_fetch_transaction method and the MempoolStrategy if it is not being used anywhere else

Summary by CodeRabbit

  • Chores
    • Removed bloom filter functionality including filter creation, lifecycle management, statistics tracking, and utility APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Walkthrough

The entire bloom filter module is removed from the dash-spv crate, including the builder, manager, statistics tracking, utility functions, and all associated tests. No bloom filter functionality remains publicly exported.

Changes

Cohort / File(s) Change Summary
Core bloom module infrastructure
dash-spv/src/bloom/builder.rs, dash-spv/src/bloom/manager.rs, dash-spv/src/bloom/utils.rs
Removed BloomFilterBuilder struct and fluent builder API; removed BloomFilterManager, BloomFilterConfig, and BloomFilterStats structures with associated async methods (create_filter, add_address, add_outpoint, add_data, contains, process_transaction, needs_recreation, get_stats, clear); removed utility functions extract_pubkey_hash and outpoint_to_bytes.
Statistics and tracking
dash-spv/src/bloom/stats.rs
Removed BloomStatsTracker struct and all statistics tracking infrastructure including DetailedBloomStats, QueryPerformance, FilterHealth, and NetworkImpact structures with associated lifecycle methods (record_query, record_addition, record_recreation, record_transaction, summary_report).
Module declarations
dash-spv/src/bloom/mod.rs, dash-spv/src/lib.rs
Removed all submodule declarations (builder, manager, stats, utils, tests) and public re-exports (BloomFilterBuilder, BloomFilterConfig, BloomFilterManager, BloomFilterStats, BloomStatsTracker, DetailedBloomStats) from mod.rs; removed pub mod bloom from lib.rs.
Tests
dash-spv/src/bloom/tests.rs
Removed comprehensive test suite covering BloomFilterBuilder, BloomFilterManager, BloomStatsTracker, utility functions, and transaction processing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Dependency verification: Confirm no remaining codebase references or imports from the removed bloom module exist elsewhere in the project
  • Breaking changes: Verify that external consumers (if any) are aware of the removal, or that this is an internal-only module with no public API surface
  • Module completeness: Ensure all bloom-related files have been identified and removed, with no orphaned references or imports

Poem

A rabbit hops through forests dense,
But blooms no longer make defense—
We've swept the filter clean away,
The burrow's lighter every day! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removal of the bloom module from the codebase. It is concise, clear, and directly reflects the primary changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-bloom-module

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9234fd and 8372bde.

📒 Files selected for processing (7)
  • dash-spv/src/bloom/builder.rs (0 hunks)
  • dash-spv/src/bloom/manager.rs (0 hunks)
  • dash-spv/src/bloom/mod.rs (0 hunks)
  • dash-spv/src/bloom/stats.rs (0 hunks)
  • dash-spv/src/bloom/tests.rs (0 hunks)
  • dash-spv/src/bloom/utils.rs (0 hunks)
  • dash-spv/src/lib.rs (0 hunks)
💤 Files with no reviewable changes (7)
  • dash-spv/src/lib.rs
  • dash-spv/src/bloom/utils.rs
  • dash-spv/src/bloom/tests.rs
  • dash-spv/src/bloom/mod.rs
  • dash-spv/src/bloom/manager.rs
  • dash-spv/src/bloom/builder.rs
  • dash-spv/src/bloom/stats.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: Core Components Tests
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: SPV Components Tests
  • GitHub Check: Key Wallet Components Tests
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: Pre-commit (macos-latest)
  • GitHub Check: Pre-commit (ubuntu-latest)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants