Skip to content

perf(l1): precompute eth_getLogs bloom inputs once instead of per-block#6895

Open
ElFantasma wants to merge 2 commits into
mainfrom
perf/getlogs-bloom-precompute
Open

perf(l1): precompute eth_getLogs bloom inputs once instead of per-block#6895
ElFantasma wants to merge 2 commits into
mainfrom
perf/getlogs-bloom-precompute

Conversation

@ElFantasma

Copy link
Copy Markdown
Contributor

Motivation

In the eth_getLogs header-bloom prefilter, the per-block check re-derived each requested address's and topic's bloom bits on every block: Bloom::contains_input(BloomInput::Raw(..)) internally does Bloom::from(input) (a keccak + bit extraction) before the subset test. Since the filter is constant for the whole query, that derivation was recomputed once per block — and for wide-range queries that skip most blocks, the bloom check is the only per-block work, so a ~1M-block sparse query did ~1M redundant keccaks.

Raised by @iovoid in review of #6813. Closes #6893.

Description

  • Introduce BloomFilterMatcher, built once before the block loop, holding each address and each constrained topic position pre-derived into Blooms (a topic position is a wildcard, or an OR over its concrete alternatives — preserving the None/empty-list wildcard semantics).
  • The per-block check is now block_bloom.contains_bloom(&precomputed) — a cheap bit-subset test with no hashing.
  • Behaviour-identical: the existing bloom_match_* unit tests pass unchanged (rewired through a bloom_matches helper that builds the matcher).

Checklist

  • Precompute address/topic blooms once before the block loop
  • Per-block check uses contains_bloom
  • bloom_match_* tests pass unchanged

@ElFantasma ElFantasma requested a review from a team as a code owner June 19, 2026 19:21
@github-actions github-actions Bot added L1 Ethereum client performance Block execution throughput and performance in general labels Jun 19, 2026
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

This is a well-crafted performance optimization. The change correctly moves bloom filter derivation from a per-block hot path to a one-time upfront computation, significantly reducing hashing overhead for wide block ranges.

Correctness & Logic

  1. Logic Preservation (Lines 264–280): The wildcard handling for TopicFilter::Topics correctly identifies both empty lists and lists containing None as unconstrained positions, matching the original behavior. The translation from contains_input (hash-then-check) to contains_bloom (bitwise subset check) is valid because BloomFilterMatcher::new() pre-computes the exact bloom bits that contains_input would have derived.

  2. Bloom Semantics (Line 310, 316): Using contains_bloom performs a cheap bitwise & operation instead of recomputing keccak256 hashes for every block. This is the core optimization and is correctly implemented.

Code Quality

  1. Documentation (Lines 249–258, 289–304): The module-level and struct-level documentation clearly explains the optimization strategy and the necessary-condition nature of bloom filtering. This is excellent for maintainers.

  2. Test Updates (Lines 406–409): The introduction of the bloom_matches helper function in tests cleanly adapts existing test cases to the new API without changing assertions, ensuring behavioral continuity.

Minor Suggestions

  1. Closure Allocation (Line 252): The to_bloom closure captures nothing and could be a static function or inlined, but the compiler will likely inline it anyway. This is purely stylistic.

  2. Memory Pre-allocation (Lines 253–254, 281–283): You could pre-size the Vec<Bloom> collections using .with_capacity(address_filter.len()) and similar for topics to avoid reallocations during the mapping, though this is a minor optimization given typical filter sizes.

Verdict: The PR is correct, secure (read-only optimization with no side effects), and follows Rust idioms. The performance improvement for RPC log filtering on wide ranges will be substantial.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

PR #6895perf(l1): precompute eth_getLogs bloom inputs once instead of per-block

Overview

This PR optimizes eth_getLogs by introducing BloomFilterMatcher, a struct that pre-derives the filter's addresses and topic positions into Bloom values before the block loop. The per-block check becomes a cheap bit-subset test (contains_bloom) instead of re-hashing every address and topic via Bloom::from(BloomInput::Raw(...)) on every block. The change is scoped to one file (crates/networking/rpc/eth/logs.rs) with 99 additions and 58 deletions.


Correctness

The refactor is behavior-identical to the old block_bloom_matches function:

  • Address filtering: bloom.contains_input(BloomInput::Raw(addr)) ≡ building Bloom::from(BloomInput::Raw(addr)) once and calling bloom.contains_bloom(&precomputed). Both check that the same bits are set.
  • Topic wildcards: TopicFilter::Topics(sub_topics) where is_empty() or any element is None correctly becomes None in topic_positions, which returns true in matches(). This matches the original || sub_topics.is_empty() || sub_topics.iter().any(Option::is_none) guard.
  • Topic OR / position AND: Some(blooms).iter().any(...) preserves the within-position OR, and topic_positions.iter().all(...) preserves the across-positions AND.
  • Empty filter (no addresses, no topics): addresses is empty → first guard is skipped; topic_positions is empty → all() is vacuously true. Returns true. Correct.
  • The flatten() call in the TopicFilter::Topics arm is guarded by the !any(Option::is_none) check in the match arm above it, so it only runs on all-Some iterators. No silent data loss.

All existing unit tests pass unchanged (wired through the new bloom_matches helper), which provides good regression coverage.


Potential Issues

Minor: non-deterministic iteration order in addresses

address_filter is a HashSet<&H160>. Its iteration order is non-deterministic, so self.addresses is in unspecified order. This is harmless since any() doesn't care about order, but it's worth keeping in mind if this struct ever gains ordering-sensitive logic.

Minor: memory footprint for large address sets

Each Bloom is 256 bytes. For a filter with N addresses, addresses now holds 256 * N bytes on the heap (previously zero). For typical RPC filters this is negligible, but for pathological inputs (e.g., thousands of addresses), the struct could balloon. Not a blocker, just worth documenting if callers can pass unbounded address lists.

Observation: to_bloom closure visibility

let to_bloom = |bytes: &[u8]| Bloom::from(BloomInput::Raw(bytes));

This is defined as a local closure inside new(). It's used in two places within that function. This is idiomatic and fine; no issue here.


Code Quality

The code is well-structured and the comments are accurate and informative. The separation of construction (new) from query (matches) is clean. Making BloomFilterMatcher private to the module is appropriate.

The test helper bloom_matches is a sensible shim that keeps the test surface unchanged while routing through the new struct — good for regression safety.


Performance

The optimization is sound and the motivation is well-justified. For a query spanning M blocks with F addresses and T topics, the old code did O(M × (F + T)) keccak hashes; the new code does O(F + T) hashes total. For sparse wide-range queries (the common hot path for eth_getLogs), this is a meaningful improvement.


Summary

This is a clean, correct, and well-motivated performance improvement. No behavioral changes, no correctness issues found, and the existing tests give strong coverage. The two minor notes above (iteration order documentation, memory footnote for unbounded address sets) are not blockers — the PR is in good shape to merge.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces BloomFilterMatcher, a struct that precomputes each filter address and topic into a Bloom once before the block loop, replacing per-block contains_input calls (which internally re-hash via keccak) with cheap contains_bloom bit-subset tests. The optimization is behaviour-identical and targets the dominant per-block cost in wide-range sparse eth_getLogs queries.

  • BloomFilterMatcher::new is constructed once before the for block_num in from..=to loop; wildcard semantics (TopicFilter::Topic(None), empty or None-containing Topics) are preserved identically through the None branch of topic_positions.
  • Per-block work is reduced from O(addresses + topics) keccak invocations to O(addresses + topics) bitwise-AND checks, a meaningful speedup for million-block queries that skip almost every block on the bloom-filter step.
  • All existing bloom_match_* unit tests are rewired through a thin bloom_matches helper that delegates to the new struct, confirming behaviour parity.

Confidence Score: 5/5

Safe to merge — the change is a pure performance refactor with no observable behaviour difference.

The substitution of contains_input with precomputed contains_bloom is mathematically equivalent: Bloom::from(BloomInput::Raw(bytes)) produces exactly the bits that contains_input tests, so the subset check is identical. Wildcard semantics map to the same None branch in topic_positions. All existing unit tests pass unchanged.

No files require special attention.

Important Files Changed

Filename Overview
crates/networking/rpc/eth/logs.rs Replaces block_bloom_matches with BloomFilterMatcher; logic is behaviour-identical, wildcard edge cases are handled correctly, and unit tests cover all branches.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[eth_getLogs request] --> B[Resolve block range from/to]
    B --> C[Build address_filter HashSet]
    C --> D["BloomFilterMatcher::new()\nPrecompute address & topic Blooms once\n(keccak x n_addresses + n_topics)"]
    D --> E[for block_num in from..=to]
    E --> F[Fetch block header]
    F --> G{"bloom_matcher.matches(header.logs_bloom)\ncontains_bloom — bitwise AND only"}
    G -- false --> H[continue — skip block]
    G -- true --> I[Fetch block body + receipts]
    I --> J[Collect logs matching address_filter]
    J --> K{More blocks?}
    K -- yes --> E
    K -- no --> L[Second pass: exact topic filtering]
    L --> M[Return filtered RpcLogs]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[eth_getLogs request] --> B[Resolve block range from/to]
    B --> C[Build address_filter HashSet]
    C --> D["BloomFilterMatcher::new()\nPrecompute address & topic Blooms once\n(keccak x n_addresses + n_topics)"]
    D --> E[for block_num in from..=to]
    E --> F[Fetch block header]
    F --> G{"bloom_matcher.matches(header.logs_bloom)\ncontains_bloom — bitwise AND only"}
    G -- false --> H[continue — skip block]
    G -- true --> I[Fetch block body + receipts]
    I --> J[Collect logs matching address_filter]
    J --> K{More blocks?}
    K -- yes --> E
    K -- no --> L[Second pass: exact topic filtering]
    L --> M[Return filtered RpcLogs]
Loading

Reviews (1): Last reviewed commit: "docs(l1): changelog entry for eth_getLog..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

No findings.

This looks correct on review. The change stays in the RPC log prefilter path and preserves the exact filtering logic later in crates/networking/rpc/eth/logs.rs, so the new matcher is only an optimization of the header-bloom necessary-condition check introduced at logs.rs:145 and implemented at logs.rs:249. The wildcard handling for topics: [[null, T]] in logs.rs:277 also matches Ethereum log-filter semantics and avoids false negatives.

The added unit coverage is good, especially the regression test at logs.rs:462 for the mixed wildcard/alternative topic case.

I could not run cargo test in this environment because rustup attempts to write under a read-only home directory, so this review is based on diff and source inspection.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 23
Total lines removed: 0
Total lines changed: 23

Detailed view
+------------------------------------------+-------+------+
| File                                     | Lines | Diff |
+------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/logs.rs | 403   | +23  |
+------------------------------------------+-------+------+

@github-actions

Copy link
Copy Markdown

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 58.005 ± 0.150 57.756 58.320 1.00
head 58.266 ± 0.388 57.871 59.066 1.00 ± 0.01

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

Labels

L1 Ethereum client performance Block execution throughput and performance in general

Projects

Status: No status
Status: Todo

Development

Successfully merging this pull request may close these issues.

perf(l1,rpc): precompute eth_getLogs bloom inputs once instead of per-block

1 participant