Add eth_getLogs support to EvmRpcClient with event decoding - #1360
Conversation
eth_getLogs now runs through the Rust EvmRpcClient and decodes event params with the shared DecoderCore, returning logs already paired with their decoded params. RpcSource no longer converts logs to the HyperSync shape or runs the JS decoder; block/transaction/receipt enrichment and the block-interval/error-classification logic stay on the ReScript side. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRemoves the standalone EvmDecoder and Rpc.getLogs paths, adds get_logs to EvmRpcClient with embedded decoding, updates ReScript bindings and RpcSource to use decoded rpcEventItem results, and rewires tests through MockRpcServer and NativeDecoder helpers. ChangesABI decoding consolidated into EvmRpcClient
Sequence Diagram(s)sequenceDiagram
participant RpcSource
participant EvmRpcClient
participant JsonRpcClient
participant DecoderCore
RpcSource->>EvmRpcClient: getLogs(GetLogsParams)
EvmRpcClient->>JsonRpcClient: eth_getLogs(filter)
JsonRpcClient-->>EvmRpcClient: RawLog list
EvmRpcClient->>DecoderCore: spawn_blocking decode each log
DecoderCore-->>EvmRpcClient: decoded params
EvmRpcClient-->>RpcSource: RpcEventItem list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
eth_getLogs now runs through the Rust client, so drop the unused ReScript route, request/result schemas, and Rpc.getLogs, along with the live tests that exercised them. The ReScript↔Rust boundary is covered by EvmRpcClient_test's getLogs cases (decode, no-match, error round-trip). Kept the GetLogs.log type and topic-query helpers, still used by the source's field parsing and selection config. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
Both data sources now decode internally (HyperSync via get_event_items, RPC via get_logs), so the EvmDecoder napi class and its ReScript binding no longer have a production caller. Drop it, along with the now-orphaned Event type and Log::from_simple converter on the Rust side and the ResponseTypes.log/event shapes on the ReScript side; DecoderCore stays. The decoder unit tests (renamed events #1285, same-signature contracts, tuple/struct/empty params, the #1353 raw_events regression) now decode through EvmRpcClient.getLogs against a mock server via a shared NativeDecoder helper, preserving their coverage. The per-file mock servers are unified into a MockRpcServer test helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scenarios/test_codegen/test/lib_tests/EvmRpcClient_test.res (1)
108-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the outgoing
eth_getLogspayload in this happy-path test.Right now this only proves response parsing/decoding. Because the mock ignores the request body, a regression in filter serialization (
fromBlock/toBlockhex encoding ortopicsnesting) would still pass. Addingmock.requeststo the expectation would lock down the new Rust request contract too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scenarios/test_codegen/test/lib_tests/EvmRpcClient_test.res` around lines 108 - 161, This test only verifies response decoding and does not assert the outgoing eth_getLogs request shape, so a regression in filter serialization could slip through. Update EvmRpcClient_test.res in the Async.it("Decodes event params and parses hex log fields") case to also inspect mock.requests after client.getLogs, and assert the serialized request body includes the correct fromBlock/toBlock hex values and topics nesting. Use the existing EvmRpcClient.make and MockRpcServer.makeRaw setup to verify the request contract alongside the current response assertions.scenarios/test_codegen/test/helpers/MockRpcServer.res (1)
58-76: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEcho the incoming JSON-RPC
idhere.
make()always returns"id": 1, so this helper won't catch regressions if the client starts varying ids or pipelines multiple requests. Reflecting the request id back keeps the mock aligned with the actual JSON-RPC contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scenarios/test_codegen/test/helpers/MockRpcServer.res` around lines 58 - 76, The mock response in MockRpcServer.start currently hardcodes the JSON-RPC id, so it should echo the incoming request id instead. Update the request handler to parse the "id" from requestBody alongside "method", then pass that parsed id into the JSON.Object built for the response in start so the mock reflects the actual JSON-RPC contract. Keep getResult(method) unchanged and use the existing start/~handler flow to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/evm_rpc_source/mod.rs`:
- Around line 151-160: The `RpcSource` event decoding path is hiding real ABI
decode failures by collapsing `decoder.decode_napi(...)` with `.ok().flatten()`,
so malformed topics/data or stale metadata look the same as an unmatched event.
Update the logic in `evm_rpc_source::mod` inside the `spawn_blocking` mapping so
`decode_napi` errors are preserved and surfaced separately from the “no match”
case, using the `params` construction around `RpcEventItem` to distinguish
successful decode, no decode result, and actual decode failure.
- Around line 135-143: The get_logs method is hex-encoding from_block and
to_block before validating them, which allows negative i64 values to turn into
unintended two’s-complement block numbers. In get_logs, reject any negative
params.from_block or params.to_block up front and return an error before
constructing the JSON filter, so only valid non-negative block bounds reach
eth_getLogs.
---
Nitpick comments:
In `@scenarios/test_codegen/test/helpers/MockRpcServer.res`:
- Around line 58-76: The mock response in MockRpcServer.start currently
hardcodes the JSON-RPC id, so it should echo the incoming request id instead.
Update the request handler to parse the "id" from requestBody alongside
"method", then pass that parsed id into the JSON.Object built for the response
in start so the mock reflects the actual JSON-RPC contract. Keep
getResult(method) unchanged and use the existing start/~handler flow to locate
the fix.
In `@scenarios/test_codegen/test/lib_tests/EvmRpcClient_test.res`:
- Around line 108-161: This test only verifies response decoding and does not
assert the outgoing eth_getLogs request shape, so a regression in filter
serialization could slip through. Update EvmRpcClient_test.res in the
Async.it("Decodes event params and parses hex log fields") case to also inspect
mock.requests after client.getLogs, and assert the serialized request body
includes the correct fromBlock/toBlock hex values and topics nesting. Use the
existing EvmRpcClient.make and MockRpcServer.makeRaw setup to verify the request
contract alongside the current response assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2431ba46-8f0e-4512-aa47-2de0c98bc728
📒 Files selected for processing (17)
packages/cli/src/evm_hypersync_source/decode.rspackages/cli/src/evm_hypersync_source/mod.rspackages/cli/src/evm_hypersync_source/types.rspackages/cli/src/evm_rpc_source/mod.rspackages/envio/src/Core.respackages/envio/src/sources/EvmRpcClient.respackages/envio/src/sources/HyperSyncClient.respackages/envio/src/sources/Rpc.respackages/envio/src/sources/RpcSource.resscenarios/test_codegen/test/RpcSource_test.resscenarios/test_codegen/test/helpers/MockRpcServer.resscenarios/test_codegen/test/helpers/NativeDecoder.resscenarios/test_codegen/test/lib_tests/EvmRpcClient_test.resscenarios/test_codegen/test/lib_tests/HyperSyncDecoder_test.resscenarios/test_codegen/test/lib_tests/RenamedEventDecode_test.resscenarios/test_codegen/test/lib_tests/Rpc_Test.resscenarios/test_codegen/test/lib_tests/SameSignatureEventDecode_test.res
💤 Files with no reviewable changes (5)
- scenarios/test_codegen/test/lib_tests/Rpc_Test.res
- packages/cli/src/evm_hypersync_source/types.rs
- packages/envio/src/Core.res
- packages/envio/src/sources/HyperSyncClient.res
- packages/envio/src/sources/Rpc.res
| let decoder = self.decoder.clone(); | ||
| // Decoding is CPU-bound ABI work; keep it off the libuv async thread. | ||
| tokio::task::spawn_blocking(move || { | ||
| raw_logs | ||
| .into_iter() | ||
| .map(|raw| { | ||
| let params = decoder.decode_napi(&raw.to_decoder_log()).ok().flatten(); | ||
| Ok(RpcEventItem { | ||
| log: raw.into_rpc_log()?, | ||
| params, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don’t collapse decode errors into “no match.”
Line 157 drops every decode_napi failure with .ok().flatten(). For a registered event with malformed topics/data or stale ABI metadata, params becomes None, and downstream RpcSource will treat that the same as an unmatched signature and skip the event. Please surface decode failures separately instead of silently discarding them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/evm_rpc_source/mod.rs` around lines 151 - 160, The
`RpcSource` event decoding path is hiding real ABI decode failures by collapsing
`decoder.decode_napi(...)` with `.ok().flatten()`, so malformed topics/data or
stale metadata look the same as an unmatched event. Update the logic in
`evm_rpc_source::mod` inside the `spawn_blocking` mapping so `decode_napi`
errors are preserved and surfaced separately from the “no match” case, using the
`params` construction around `RpcEventItem` to distinguish successful decode, no
decode result, and actual decode failure.
There was a problem hiding this comment.
This is currently intentional: it mirrors the HyperSync decoder's existing behavior (its decode_logs also used .ok().flatten()), so RPC and HyperSync now treat a genuine decode failure the same way — drop the event rather than fail the batch. The prior RPC-only path raised a non-retryable ImpossibleForTheQuery ("check your ABI"). Whether to restore that louder signal (and reintroduce the RPC/HyperSync divergence) is a deliberate design call I've flagged to the maintainer; leaving this thread open pending that decision.
Generated by Claude Code
There was a problem hiding this comment.
@DZakh Thanks for the clarification — that makes sense.
Keeping decode failures collapsed to None in packages/cli/src/evm_rpc_source/mod.rs for parity with the existing HyperSync path is a reasonable intentional tradeoff, and I agree this is better treated as a maintainer/product decision than a must-fix within this PR. Happy to leave the thread open for that call rather than push further here.
(ᵔᴥᵔ)੭ 🐇
✏️ Learnings added
Learnt from: DZakh
Repo: enviodev/hyperindex PR: 1360
File: packages/cli/src/evm_rpc_source/mod.rs:160-169
Timestamp: 2026-06-29T14:54:40.926Z
Learning: In the Rust/ReScript EVM log decoding flow for this repository, `packages/cli/src/evm_rpc_source/mod.rs` `EvmRpcClient::get_logs` intentionally collapses `DecoderCore::decode_napi` failures to `None` to mirror the existing HyperSync decoder behavior, so both RPC and HyperSync drop undecodable events instead of failing the batch. The older RPC-only path surfaced these as a non-retryable `ImpossibleForTheQuery` error, and changing back would be an explicit maintainer decision because it reintroduces RPC/HyperSync behavioral divergence.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- get_logs rejects negative from_block/to_block before hex-encoding, so a bad bound can't become a two's-complement quantity that widens the range. - The getLogs decode test now also asserts the outgoing eth_getLogs request body (hex block bounds + topic nesting), locking the request contract. - MockRpcServer.make echoes the request's JSON-RPC id instead of hardcoding 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
- RpcLog no longer carries `data` (consumed only by the Rust decoder) or `removed` (unused on the ReScript side), so neither crosses the napi boundary. RawLog keeps `data` for decoding and drops `removed`. - EvmRpcClient::new takes checksum_addresses as a required bool instead of Option<bool>; the ReScript make/binding now require ~checksumAddresses so callers pick decode-time address casing explicitly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
2 participants
Summary
Adds
eth_getLogsRPC method support to the Rust-basedEvmRpcClient, with integrated event parameter decoding. This replaces the previous JavaScript-basedRpc.getLogsapproach with a native implementation that decodes event logs using the sameDecoderCoreinfrastructure as HyperSync.Key Changes
Rust EvmRpcClient (
packages/cli/src/evm_rpc_source/mod.rs):GetLogsParamsstruct to define filter parameters (block range, addresses, topics)RpcLogstruct to represent decoded log fields (hex quantities converted to i64)RpcEventItemstruct pairing logs with their decoded paramsget_logs()async method that:eth_getLogsvia JSON-RPCDecoderCoreEvmRpcClient::new()to acceptevent_paramsandchecksum_addressesfor decoder initializationReScript bindings (
packages/envio/src/sources/EvmRpcClient.res):getLogsParamstype matching the Rust structrpcEventItemtype with log and decoded paramsttype to includegetLogsmethodclassNewbinding to pass event params and checksum flag to Rust constructorgetLogscallsRpcSource integration (
packages/envio/src/sources/RpcSource.res):Rpc.getLogscall withrpcClient.getLogs()RpcEventItemgetNextPageto useEvmRpcClient.tinstead of genericRpc.clientTest infrastructure:
fetchstub withMockJsonRpcServermodule that runs a real local HTTP server (required because Rust client uses its own HTTP stack)EvmRpcClient.getLogs()covering:Implementation Details
parse_hex_u64utility and validates against i64::MAXhttps://claude.ai/code/session_014HcYHxx8g1htYJS1G8Wq4g
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
getLogs, decoding correctness, and retry/error behavior using a dedicated mock RPC server.