Add option to merge calls to eth_getLogs #200
Conversation
📝 WalkthroughWalkthroughAdds an option to merge eth_getLogs calls and perform client-side filtering; updates log-fetching internals to support merged vs. standard call strategies, adds tests and Vitest config, and includes a prerelease release note. Two public API changes: new EvmRpcStreamOptions field and added mergeGetLogs parameter on two fetch functions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant StreamConfig
participant LogFetcher
participant MergeHelper as mergedGetLogsCalls
participant StandardHelper as standardGetLogsCalls
participant RPC as eth_getLogs
rect rgb(220,235,255)
Note over Client,LogFetcher: Merged flow (mergeGetLogs: true)
Client->>StreamConfig: request logs (mergeGetLogsFilter)
StreamConfig->>LogFetcher: fetchLogs(..., mergeGetLogs: true)
LogFetcher->>MergeHelper: group & split filters (addr/no-addr)
MergeHelper->>RPC: batched eth_getLogs calls
RPC-->>MergeHelper: aggregated logs
MergeHelper->>LogFetcher: {logFilters, logs}
LogFetcher->>LogFetcher: refine per filter, dedupe, attach filterIds
LogFetcher-->>Client: merged, deduped results
end
rect rgb(235,255,235)
Note over Client,LogFetcher: Standard flow (mergeGetLogs: false)
Client->>StreamConfig: request logs (mergeGetLogsFilter)
StreamConfig->>LogFetcher: fetchLogs(..., mergeGetLogs: false)
LogFetcher->>StandardHelper: issue per-filter requests
StandardHelper->>RPC: parallel eth_getLogs per filter
RPC-->>StandardHelper: logs per filter
StandardHelper->>LogFetcher: [{logFilter, logs}...]
LogFetcher->>Client: per-filter results (dedup per call)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/evm-rpc/test/log-fetcher.test.ts (1)
22-31: Consider extracting shared client setup.The
beforeAllclient setup is duplicated across both describe blocks. Consider extracting to a shared helper or top-level setup if the tests grow.🔎 Proposed refactor
// At top of file, after imports function createTestClient() { const rpcUrl = process.env.TEST_EVM_RPC_URL; if (!rpcUrl) { throw new Error("TEST_EVM_RPC_URL environment variable is not set"); } return createPublicClient({ transport: http(rpcUrl), }); } // Then in each describe block: let client: ReturnType<typeof createPublicClient>; beforeAll(() => { client = createTestClient(); });Also applies to: 152-161
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/evm-rpc/test/__snapshots__/log-fetcher.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
change/@apibara-evm-rpc-2585c9c9-f236-4627-a25a-8e7b3c572a71.jsonpackages/evm-rpc/package.jsonpackages/evm-rpc/src/log-fetcher.tspackages/evm-rpc/src/stream-config.tspackages/evm-rpc/test/log-fetcher.test.tspackages/evm-rpc/vitest.config.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/evm-rpc/test/log-fetcher.test.ts (1)
packages/evm-rpc/src/log-fetcher.ts (2)
fetchLogsByBlockHash(17-74)fetchLogsForRange(76-160)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (14)
change/@apibara-evm-rpc-2585c9c9-f236-4627-a25a-8e7b3c572a71.json (1)
1-7: LGTM!Changeset is correctly formatted and appropriately describes the new feature for merging
eth_getLogscalls.packages/evm-rpc/package.json (1)
26-27: LGTM!Test scripts and Vitest dependency are appropriately configured for the new test suite.
Also applies to: 32-32
packages/evm-rpc/src/stream-config.ts (3)
48-49: LGTM!The new
mergeGetLogsFilteroption is well-documented and provides clear semantics for controlling merge behavior.
281-281: LGTM!Range-based fetching correctly enables merging only when
mergeGetLogsFilter === "always".
301-303: LGTM!Block-hash fetching appropriately enables merging for both
"always"and"accepted"modes, which makes sense since this path handles accepted (potentially reorg-able) blocks where reducing RPC calls is beneficial.packages/evm-rpc/vitest.config.ts (1)
1-7: LGTM!Vitest configuration with a 10-second timeout is appropriate for integration tests that make external RPC calls.
packages/evm-rpc/test/log-fetcher.test.ts (2)
33-53: Empty filter test verifies correct behavior.Good test case - validates that both merged and non-merged paths return empty results for empty filters, ensuring short-circuit logic works correctly.
82-120: Comprehensive filter coverage.Testing strict empty topics combined with Transfer event topic provides good coverage for edge cases in the
refineLogfunction.packages/evm-rpc/src/log-fetcher.ts (6)
44-71: Deduplication logic is correct.The approach of tracking seen logs by
logIndexand accumulatingfilterIdsfor duplicate matches correctly handles multiple filters matching the same log entry.
111-149: Block-level deduplication is well-implemented.The nested
seenLogsByBlockNumberAndIndexcorrectly tracks logs per block and handles multiple filters matching the same log within each block.
167-169: Client-side address filtering enables merged log fetching.Using
isAddressEqualfor case-insensitive address comparison is the correct approach for client-side filtering when logs are fetched in bulk.
232-247: Merged call trades RPC count for bandwidth.The merged approach fetches logs for all addresses without topic filtering, relying on
refineLogfor client-side filtering. This effectively reduces RPC calls but may increase data transfer when filters have different topics. This is the expected trade-off for the feature.
250-265: Address-less filters fetch all logs.When a filter has no address specified, the merged path fetches all logs from the block/range. This is semantically correct but can be expensive on high-activity blocks. Users should be aware that address-less filters with
mergeGetLogs: truemay result in significant data transfer.
271-303: Standard path preserves RPC-level filtering.The non-merged path correctly passes address and topics to each
eth_getLogscall, leveraging server-side filtering for efficiency when reducing RPC count isn't the priority.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
⏰ 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). (1)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/evm-rpc/test/log-fetcher.test.ts (2)
7-8: Consider adding comments to document contract addresses.Adding brief comments explaining what these contract addresses represent (e.g., token contracts, protocol contracts) would improve test readability and maintainability.
79-117: Consider extracting the Transfer event signature.The Transfer event topic signature (
0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef) is duplicated multiple times. Extracting it as a named constant would improve maintainability and make the test intent clearer.🔎 Suggested refactor
Add near the top of the file with other constants:
+const TRANSFER_EVENT_SIGNATURE = + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef" as const;Then replace the hardcoded values in the tests.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/evm-rpc/test/log-fetcher.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/evm-rpc/test/log-fetcher.test.ts (1)
packages/evm-rpc/src/log-fetcher.ts (2)
fetchLogsByBlockHash(17-74)fetchLogsForRange(76-160)
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (7)
packages/evm-rpc/test/log-fetcher.test.ts (7)
30-50: Good test coverage for empty filter edge case.The test correctly validates that both standard and merged modes produce identical results for an empty filter, ensuring behavioral parity between the two code paths.
52-77: LGTM: Multi-address filtering test validates key functionality.The test appropriately covers filtering by multiple contract addresses and ensures parity between standard and merged implementations.
125-147: Well-designed sortResult helper for range queries.The helper correctly handles the more complex return structure from
fetchLogsForRange, ensuring deterministic ordering of both block numbers and logs within each block.
157-179: Consistent test pattern ensures thorough validation.The test suite maintains a consistent pattern across both functions being tested, which aids maintainability and ensures comprehensive coverage of the mergeGetLogs feature.
181-208: LGTM: Range query filtering test validates expected behavior.The test appropriately validates multi-address filtering for range queries and ensures behavioral consistency between standard and merged modes.
210-250: Comprehensive test coverage for complex range filtering.The test validates the most complex filtering scenario (strict empty topics + Transfer events) across a block range, ensuring the mergeGetLogs feature works correctly in all scenarios.
22-28: Timeout configuration is already in place at 10 seconds inpackages/evm-rpc/vitest.config.ts. However, monitor test reliability with external RPC endpoints—if tests become flaky on slower networks, consider increasing the timeout or using a private RPC endpoint via theTEST_EVM_RPC_URLenvironment variable.
1 participant
This PR helps reduce the number of RPC calls made.