[RPC][filter] Add more block range checks#1824
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds synchronous-to-async conversions for several filter-creation RPC methods and enforces a max block-range check for Log-type filters during creation, changes polling, and direct log queries; returns an error when the effective from/to span exceeds the configured maximum. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(0,128,0,0.5)
participant Client
end
rect rgba(0,0,128,0.5)
participant RPC as EthFilter RPC
end
rect rgba(128,0,0,0.5)
participant Chain as Chain/State
end
Client->>RPC: eth_newFilter / eth_getLogs / getFilterChanges
RPC->>Chain: query best block / resolve from/to defaults
Chain-->>RPC: current block number
RPC->>RPC: compute effective from/to and block_range
alt block_range <= max_block_range
RPC->>RPC: create/store filter or fetch logs
RPC-->>Client: success (filter id or logs)
else block_range > max_block_range
RPC-->>Client: error (block range exceeded)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/rpc/src/eth/filter.rs`:
- Around line 387-393: The current check that rejects oversize ranges (comparing
block_range derived from current_number and from_number against
self.max_block_range) runs after last_poll has already been advanced, which can
move the cursor even on error; move the validation so it occurs before updating
last_poll (or if you prefer, roll back last_poll on error) — locate where
last_poll is updated in the polling/lock section and perform the block_range
calculation and the comparison (using current_number, from_number,
self.max_block_range) before setting last_poll, returning the internal_err if
too wide.
librelois
left a comment
There was a problem hiding this comment.
eth_newFilter range validation uses chain best block, not latest indexed block, so behavior diverges from eth_getLogs under indexing lag.
File: filter.rs (line 111)
Details: the new check in create_filter computes range with self.client.info().best_number, while eth_getLogs/other read paths cap at latest indexed. If best is ahead of indexed, eth_newFilter can reject a filter that eth_getLogs semantics would allow at that moment. This is a real behavior mismatch with the stated “same limit as eth_getLogs” goal.
* Add more block range checks. * Coderabbit review + prettier. * Use latest_indexed_block_number() for FilterType::Log
* Add more block range checks. * Coderabbit review + prettier. * Use latest_indexed_block_number() for FilterType::Log
…ntier#1824 (#3677) * update frontier pin * Configure AllowUnprotectedTxs to false * Fix compile error * Temporary allow unprotected txs to not break tests * fix dev tests * fix tracing tests * fix coderabbit suggestion
…ntier#1824 (#3677) * update frontier pin * Configure AllowUnprotectedTxs to false * Fix compile error * Temporary allow unprotected txs to not break tests * fix dev tests * fix tracing tests * fix coderabbit suggestion
This PR applies the same
max_block_rangelimit (used byeth_getLogs) to all other log retrieval paths:eth_newFilter– Reject log filters whose block range exceeds the limit at creation.eth_getFilterLogs– Enforce the limit before running the query.eth_getFilterChanges– Enforce the limit before querying logs (Log filter branch).The main goal is to mitigate DoS on these methods.