fix: error when block range exceeds in eth filter and logs#6856
Merged
akaladarshi merged 6 commits intomainfrom Apr 7, 2026
Merged
fix: error when block range exceeds in eth filter and logs#6856akaladarshi merged 6 commits intomainfrom
akaladarshi merged 6 commits intomainfrom
Conversation
akaladarshi
requested review from
hanabi1224 and
sudo-shashank
and removed request for
a team
Contributor
WalkthroughIntroduces Ethereum-compatible block-range error handling: adds Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RPC as "RPC Handler\n(EthGetLogs / EthTraceFilter)"
participant Parser as "Filter Parser\n(parse_block_range)"
participant Events as "Events Store\n(get_events_for_parsed_filter)"
Client->>RPC: eth_getLogs / trace_filter request
RPC->>Parser: parse_eth_filter_spec / parse_block_range
alt range > max
Parser-->>RPC: EthErrors::BlockRangeExceeded (error)
RPC-->>Client: JSON-RPC ServerError (-32005) with message
else parsed
Parser-->>RPC: ParsedFilter
RPC->>Events: get_events_for_parsed_filter(parsed)
Events-->>RPC: events
RPC-->>Client: JSON-RPC response (events)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
akaladarshi
force-pushed
the
akaladarshi/add-block-range-err
branch
from
7f8bac9 to
df123ff
Compare
hanabi1224
reviewed
Apr 6, 2026
hanabi1224
reviewed
Apr 6, 2026
hanabi1224
reviewed
Apr 6, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/rpc/methods/eth.rs (1)
3164-3172: Preserve all structured ETH errors and keep the source chain.Line 3171 replaces the original parse error with a fresh
anyhow!, so non-range failures lose their cause chain, and any futureEthErrorsvariant returned here will also lose its RPC metadata. Fast-path anyEthErrorsunchanged and usee.context("failed to parse events for filter")for everything else.As per coding guidelines, "Use
anyhow::Result<T>for most operations and add context with.context()when errors occur".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/eth.rs` around lines 3164 - 3172, The current map_err replaces non-range errors with a new anyhow! which drops the source; instead, in the closure for .parse_eth_filter_spec(&ctx, ð_filter).map_err(|e| { ... }), fast-path and return the original error unchanged if it is any EthErrors (not just BlockRangeExceeded) by detecting downcast_ref::<EthErrors>(), otherwise return e.context("failed to parse events for filter") so the original source chain is preserved and non-EthErrors gain context; reference parse_eth_filter_spec, ctx, eth_filter and EthErrors when locating the code to update.src/rpc/methods/eth/errors.rs (1)
21-26: Tighten the public construction API forBlockRangeExceeded.
limit_exceeded()is now public but undocumented, and the publicmessagefield lets callers bypass the helper and build inconsistent{ max, given, message }combinations. I’d add a doc comment to the helper and derive the display/message frommax/giveninstead of storing it separately.As per coding guidelines, "Document public functions and structs with doc comments".
Also applies to: 46-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/eth/errors.rs` around lines 21 - 26, BlockRangeExceeded currently stores a free-form public message and exposes an undocumented public constructor limit_exceeded(), which allows inconsistent instances; remove the public message field (make it private/omit it), change the #[error("{message}")] to derive the display from the numeric fields (e.g., #[error("block range exceeded: max = {max}, given = {given}")]) so the error text is computed from max and given, and then either restrict the helper constructor limit_exceeded() to a narrower visibility (pub(crate) or private) or add a doc comment to it explaining its use if it must remain public; apply the same change to the other similar error variant that follows.CHANGELOG.md (1)
50-50: Polish the changelog sentence for readability and consistency.The phrase “when block range exceeds in the eth filter and logs API” is a bit awkward. Consider a cleaner wording:
Suggested wording tweak
-- [`#6856`](https://github.com/ChainSafe/forest/pull/6856): Return ethereum compatible error `BlockRangeExceeded` with code `-32005` when block range exceeds in the eth filter and logs API. +- [`#6856`](https://github.com/ChainSafe/forest/pull/6856): Return Ethereum-compatible `BlockRangeExceeded` (`-32005`) when the requested block range exceeds the configured limit in `eth_filter` and `eth_getLogs`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 50, Edit the changelog entry that mentions PR `#6856` to improve readability: rephrase the sentence about returning the ethereum-compatible error BlockRangeExceeded (code -32005) so it clearly states that this error is returned when the requested block range exceeds limits in the eth_filter and logs APIs; keep the error name `BlockRangeExceeded`, the code `-32005`, and the PR reference `#6856` intact while making the sentence grammatically clear and consistent with other changelog entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rpc/methods/eth.rs`:
- Around line 3870-3879: The config allows max_filter_height_range = 0 but
trace_filter treats 0 as "unlimited" while parse_block_range enforces a hard
zero cap; fix by validating the deserialized config to reject 0 (require
positive integer) or make trace_filter apply the same sentinel handling as
parse_block_range; specifically update the config validation code where
max_filter_height_range is deserialized (and any initialization of
ctx.eth_event_handler.max_filter_height_range) to error on 0, or alternatively
change the check in trace_filter (the block using
ctx.eth_event_handler.max_filter_height_range and the comparison with
to_block/from_block) to treat 0 the same as parse_block_range’s behavior so both
eth_getLogs/eth_newFilter and trace_filter are consistent.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 50: Edit the changelog entry that mentions PR `#6856` to improve
readability: rephrase the sentence about returning the ethereum-compatible error
BlockRangeExceeded (code -32005) so it clearly states that this error is
returned when the requested block range exceeds limits in the eth_filter and
logs APIs; keep the error name `BlockRangeExceeded`, the code `-32005`, and the
PR reference `#6856` intact while making the sentence grammatically clear and
consistent with other changelog entries.
In `@src/rpc/methods/eth.rs`:
- Around line 3164-3172: The current map_err replaces non-range errors with a
new anyhow! which drops the source; instead, in the closure for
.parse_eth_filter_spec(&ctx, ð_filter).map_err(|e| { ... }), fast-path and
return the original error unchanged if it is any EthErrors (not just
BlockRangeExceeded) by detecting downcast_ref::<EthErrors>(), otherwise return
e.context("failed to parse events for filter") so the original source chain is
preserved and non-EthErrors gain context; reference parse_eth_filter_spec, ctx,
eth_filter and EthErrors when locating the code to update.
In `@src/rpc/methods/eth/errors.rs`:
- Around line 21-26: BlockRangeExceeded currently stores a free-form public
message and exposes an undocumented public constructor limit_exceeded(), which
allows inconsistent instances; remove the public message field (make it
private/omit it), change the #[error("{message}")] to derive the display from
the numeric fields (e.g., #[error("block range exceeded: max = {max}, given =
{given}")]) so the error text is computed from max and given, and then either
restrict the helper constructor limit_exceeded() to a narrower visibility
(pub(crate) or private) or add a doc comment to it explaining its use if it must
remain public; apply the same change to the other similar error variant that
follows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b1186afe-3b75-44cf-b3f3-81a90db02674
📒 Files selected for processing (4)
CHANGELOG.mdsrc/rpc/methods/eth.rssrc/rpc/methods/eth/errors.rssrc/rpc/methods/eth/filter/mod.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 17 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
akaladarshi
force-pushed
the
akaladarshi/add-block-range-err
branch
from
dbb5cd4 to
544c3c7
Compare
hanabi1224
approved these changes
Apr 6, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/rpc/methods/eth/errors.rs (2)
21-26: Variant structure looks good, butmaxandgivenfields are unused outside construction.The
maxandgivenfields are stored in the enum but only used duringlimit_exceeded()to build themessage. TheRpcErrorDatamethods only accessmessage. This is fine if the fields are retained for debugging/logging or potential future use (e.g., structured error data), but if they serve no other purpose, consider simplifying to just store the message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/eth/errors.rs` around lines 21 - 26, The BlockRangeExceeded enum variant stores max and given but they're never accessed outside construction; simplify by removing those unused fields and only storing the generated message (or, alternatively, use them in RpcErrorData or limit_exceeded()); update the BlockRangeExceeded definition in src/rpc/methods/eth/errors.rs to only include message (or keep fields but expose them via RpcErrorData accessors), and adjust any construction sites and the limit_exceeded() helper to build the message from provided values before constructing the variant so RpcErrorData and callers continue to read message correctly.
46-52: Consider adding a doc comment for the public constructor.Per coding guidelines, public functions should be documented. A brief doc comment would help clarify the purpose and parameters.
📝 Suggested doc comment
+ /// Create a new BlockRangeExceeded error indicating the requested block range + /// exceeds the configured maximum. pub fn limit_exceeded(max_block_range: u64, given: u64) -> Self {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rpc/methods/eth/errors.rs` around lines 46 - 52, The public constructor limit_exceeded (which builds the BlockRangeExceeded variant) lacks a doc comment; add a brief /// doc comment above pub fn limit_exceeded explaining its purpose (constructs a BlockRangeExceeded error for when a requested block range is larger than allowed), document the parameters max_block_range and given and describe the resulting error message format, and mention that it returns Self::BlockRangeExceeded so callers know what error is produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/rpc/methods/eth/errors.rs`:
- Around line 21-26: The BlockRangeExceeded enum variant stores max and given
but they're never accessed outside construction; simplify by removing those
unused fields and only storing the generated message (or, alternatively, use
them in RpcErrorData or limit_exceeded()); update the BlockRangeExceeded
definition in src/rpc/methods/eth/errors.rs to only include message (or keep
fields but expose them via RpcErrorData accessors), and adjust any construction
sites and the limit_exceeded() helper to build the message from provided values
before constructing the variant so RpcErrorData and callers continue to read
message correctly.
- Around line 46-52: The public constructor limit_exceeded (which builds the
BlockRangeExceeded variant) lacks a doc comment; add a brief /// doc comment
above pub fn limit_exceeded explaining its purpose (constructs a
BlockRangeExceeded error for when a requested block range is larger than
allowed), document the parameters max_block_range and given and describe the
resulting error message format, and mention that it returns
Self::BlockRangeExceeded so callers know what error is produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 993b3ee7-8201-4269-b43b-9cae6642c358
📒 Files selected for processing (4)
CHANGELOG.mdsrc/rpc/methods/eth.rssrc/rpc/methods/eth/errors.rssrc/rpc/methods/eth/filter/mod.rs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/rpc/methods/eth.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
BlockRangeExceedederror which shows the block range provided in the filter and logs API is more the configuredReference issue to close (if applicable)
Closes #6826
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Bug Fixes
Documentation