Add test suite for address checksumming bug in RpcSource#930
Merged
Conversation
RPC fallback returns lowercase addresses instead of checksummed when lowercaseAddresses=false. The issue exists in RpcSource.res: - log.address is not checksummed (line 692) - block.miner is not checksummed (line 59) Tests demonstrate the bug by simulating the current behavior and showing that addresses are returned lowercase when they should be checksummed. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Updated reproduction tests to include: - Unit tests with simulated RPC response (work offline) - Integration tests with real RPC calls (require network) Both test suites demonstrate that when lowercaseAddresses=false: - log.address is not checksummed (RpcSource.res:689-693) - block.miner is not checksummed (RpcSource.res:46-60) https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Contributor
📝 WalkthroughWalkthroughAdds a test to assert contract-creation transactions parse with Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant RPC as Ethereum RPC
participant Schema as GetTransactionByHash Schema
participant RpcSrc as RpcSource
participant AddrUtil as Address.Utils
participant Consumer as Test Assertions
Test->>RPC: eth_getTransactionByHash(txHash)
RPC-->>Schema: raw tx (possible "to": null)
Schema->>RpcSrc: parsed tx object (to = None | address string)
alt tx.to is string
RpcSrc->>AddrUtil: transform string -> Address.t
AddrUtil-->>RpcSrc: Address.t
else tx.to is null
RpcSrc-->>RpcSrc: keep None
end
RpcSrc->>Consumer: provide transaction (tx.to None or Address.t)
Consumer-->>Test: assert tx.to is None and tx.from present
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
- Remove duplicate unit tests (keep only integration tests with real RPC) - Remove console.log statements - Use actual Rpc module instead of duplicating logic Tests verify Address.schema doesn't checksum, which is the root cause of the issue in RpcSource when lowercaseAddresses=false. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Test now calls the actual RpcSource.getItemsOrThrow method to verify that srcAddress in returned events is not checksummed when lowercaseAddresses=false. This demonstrates the real bug path rather than just testing the underlying Rpc module. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
![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
🤖 Fix all issues with AI agents
In `@scenarios/test_codegen/test/RpcSource_test.res`:
- Around line 440-551: The test uses a hard-coded live RPC token (rpcUrl /
testApiToken) and should be gated by an environment variable; change the failing
test "FAILING: srcAddress in events from RpcSource.getItemsOrThrow should be
checksummed" to read the token from an env var (e.g. TEST_RPC_TOKEN) via
Sys.getenv (or equivalent) and skip or no-op the Async.it when the env var is
missing/empty so CI won't hit mainnet or leak the token; update any other live
RPC uses in this file (the other occurrences of testApiToken) to use the same
env var gating.
Event ID must be in format sighash_topicCount (e.g., "0x...3ef_3") for EventRouter to match events correctly. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Contract creation transactions have `to: null` in RPC response. The schema uses S.option which expects string|undefined, but RPC returns null, causing parsing to fail with: "Expected string | undefined, received null" Test fetches USDT contract deployment transaction to reproduce. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scenarios/test_codegen/test/RpcSource_test.res`:
- Around line 496-500: The two schema lines blockSchema and transactionSchema
use Utils.magic without explicit type casts; update them to include explicit
input/output annotations like the paramsRawEventSchema line. Concretely, change
the tails to ->(Utils.magic: S.t<unit> => S.t<Internal.blockSchemaType>) for
blockSchema and ->(Utils.magic: S.t<unit> =>
S.t<Internal.transactionSchemaType>) for transactionSchema (replace
blockSchemaType/transactionSchemaType with the actual Internal types used in
this module) so each Utils.magic usage has the required input->output type
annotation.
Comment on lines
+496
to
+500
| paramsRawEventSchema: S.literal(%raw(`null`)) | ||
| ->S.shape(_ => ()) | ||
| ->(Utils.magic: S.t<unit> => S.t<Internal.eventParams>), | ||
| blockSchema: S.object(_ => ())->Utils.magic, | ||
| transactionSchema: S.object(_ => ())->Utils.magic, |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit Utils.magic type annotations for blockSchema/transactionSchema.
S.object(_ => ())->Utils.magic at Lines 499–500 lacks the required explicit type annotation. Please follow the pattern from Line 498 and specify the input/output types explicitly.
As per coding guidelines, “When using Utils.magic for type casting in ReScript, always add explicit type annotations with the format: value->(Utils.magic: inputType => outputType)”.
🤖 Prompt for AI Agents
In `@scenarios/test_codegen/test/RpcSource_test.res` around lines 496 - 500, The
two schema lines blockSchema and transactionSchema use Utils.magic without
explicit type casts; update them to include explicit input/output annotations
like the paramsRawEventSchema line. Concretely, change the tails to
->(Utils.magic: S.t<unit> => S.t<Internal.blockSchemaType>) for blockSchema and
->(Utils.magic: S.t<unit> => S.t<Internal.transactionSchemaType>) for
transactionSchema (replace blockSchemaType/transactionSchemaType with the actual
Internal types used in this module) so each Utils.magic usage has the required
input->output type annotation.
- Fix issue #931: Change S.option to S.null for transaction `to` field in Rpc.GetTransactionByHash schema. RPC returns null for contract creation transactions, not undefined. - Fix address checksumming: Apply Address.Evm.fromAddressOrThrow when lowercaseAddresses=false for both log.address and block.miner fields in RpcSource.res. - Remove checksumming reproduction test (now fixed) - Update issue #931 test description (now working) https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
S.nullable handles both null and undefined values from RPC responses, making the schema more robust across different RPC providers. https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@codegenerator/cli/npm/envio/src/sources/Rpc.res`:
- Around line 186-209: The transaction schema in Rpc.res uses S.nullable for
optional fields (e.g., the fields named "from", "to", "gas", "gasPrice",
"nonce", "transactionIndex", "value", "type_", "v", "r", "s", "yParity",
"maxPriorityFeePerGas", "maxFeePerGas", "maxFeePerBlobGas",
"blobVersionedHashes")—replace each S.nullable(...) with S.option(...) to match
the codebase convention and tests; preserve the inner schemas (e.g., keep
hexBigintSchema, hexIntSchema, S.string, S.array(S.string) and the Address
conversion/mapping used in the "from" and "to" fields) so only the
nullable->option wrapper changes.
Comment on lines
+186
to
+209
| // blockHash: ?s.field("blockHash", S.nullable(S.string)), | ||
| // blockNumber: ?s.field("blockNumber", S.nullable(hexIntSchema)), | ||
| // chainId: ?s.field("chainId", S.nullable(hexIntSchema)), | ||
| from: ?s.field("from", S.nullable(S.string->(Utils.magic: S.t<string> => S.t<Address.t>))), | ||
| to: ?s.field("to", S.nullable(S.string->(Utils.magic: S.t<string> => S.t<Address.t>))), | ||
| gas: ?s.field("gas", S.nullable(hexBigintSchema)), | ||
| gasPrice: ?s.field("gasPrice", S.nullable(hexBigintSchema)), | ||
| hash: ?s.field("hash", S.nullable(S.string)), | ||
| input: ?s.field("input", S.nullable(S.string)), | ||
| nonce: ?s.field("nonce", S.nullable(hexBigintSchema)), | ||
| transactionIndex: ?s.field("transactionIndex", S.nullable(hexIntSchema)), | ||
| value: ?s.field("value", S.nullable(hexBigintSchema)), | ||
| type_: ?s.field("type", S.nullable(hexIntSchema)), | ||
| // Signature fields - optional for ZKSync EIP-712 compatibility | ||
| v: ?s.field("v", S.option(S.string)), | ||
| r: ?s.field("r", S.option(S.string)), | ||
| s: ?s.field("s", S.option(S.string)), | ||
| yParity: ?s.field("yParity", S.option(S.string)), | ||
| v: ?s.field("v", S.nullable(S.string)), | ||
| r: ?s.field("r", S.nullable(S.string)), | ||
| s: ?s.field("s", S.nullable(S.string)), | ||
| yParity: ?s.field("yParity", S.nullable(S.string)), | ||
| // EIP-1559 fields | ||
| maxPriorityFeePerGas: ?s.field("maxPriorityFeePerGas", S.option(hexBigintSchema)), | ||
| maxFeePerGas: ?s.field("maxFeePerGas", S.option(hexBigintSchema)), | ||
| maxPriorityFeePerGas: ?s.field("maxPriorityFeePerGas", S.nullable(hexBigintSchema)), | ||
| maxFeePerGas: ?s.field("maxFeePerGas", S.nullable(hexBigintSchema)), | ||
| // EIP-4844 blob fields | ||
| maxFeePerBlobGas: ?s.field("maxFeePerBlobGas", S.option(hexBigintSchema)), | ||
| blobVersionedHashes: ?s.field("blobVersionedHashes", S.option(S.array(S.string))), | ||
| maxFeePerBlobGas: ?s.field("maxFeePerBlobGas", S.nullable(hexBigintSchema)), | ||
| blobVersionedHashes: ?s.field("blobVersionedHashes", S.nullable(S.array(S.string))), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate S.nullable definition
rg -n "let nullable|val nullable" --glob "*.res" --glob "*.resi" -A 5
# Inspect Internal.evmTransactionFields definition
rg -n "type evmTransactionFields" --glob "*.res" --glob "*.resi" -A 20Repository: enviodev/hyperindex
Length of output: 1570
🏁 Script executed:
#!/bin/bash
# Check imports in Rpc.res to find S module
head -50 codegenerator/cli/npm/envio/src/sources/Rpc.res | grep -E "^open|^import|^let.*=|^module"
# Search for nullable more broadly (might be in Zod or similar)
rg "nullable" --glob "*.res" --glob "*.resi" -B 2 -A 5 | head -100
# Check if S is an alias or module
rg "module S|let S|open.*S" codegenerator/cli/npm/envio/src/sources/Rpc.res -A 3Repository: enviodev/hyperindex
Length of output: 8123
🏁 Script executed:
#!/bin/bash
# Find complete imports and module definition at top of Rpc.res
head -100 codegenerator/cli/npm/envio/src/sources/Rpc.res
# Find S module/schema library
rg "module S =|open.*S|let S =|import.*S" codegenerator/cli/npm/envio/src/ -A 2 | head -50Repository: enviodev/hyperindex
Length of output: 6589
🏁 Script executed:
#!/bin/bash
# Search for S module definition or import across the codebase
rg "^open|^import|^module S|^let S" codegenerator/cli/npm/envio/src/ -B2 -A5 | head -100
# Look for where S might be imported/defined
fd "\.res$|\.resi$" codegenerator/cli/npm/envio/src/ | xargs rg -l "let nullable|module.*nullable" | head -10
# Check the full Encoders_schema_test to understand how nullable works
cat codegenerator/cli/npm/envio/src/EventProcessing_test.res 2>/dev/null || echo "File not found"
rg -B5 -A15 "S.nullable" scenarios/test_codegen/test/Encoders_schema_test.resRepository: enviodev/hyperindex
Length of output: 8487
🏁 Script executed:
#!/bin/bash
# Find S.field or S.object signature/definition
rg "let field|val field" --glob "*.res" --glob "*.resi" -B2 -A10 | head -100
# Look for Rest.route or S.object to understand field handling
rg "S\.object|Rest\.route" codegenerator/cli/npm/envio/src/sources/Rpc.res -B2 -A20 | head -150
# Check if there are other examples of ?s.field in codebase
rg "\?s\.field" --glob "*.res" -B2 -A2 | head -80Repository: enviodev/hyperindex
Length of output: 19930
🏁 Script executed:
#!/bin/bash
# Search for S.nullable vs S.option usage patterns
echo "=== S.nullable usage ==="
rg "S\.nullable" --glob "*.res" --glob "*.resi" -B1 -A1 | grep -v "^--$" | head -80
echo ""
echo "=== S.option usage ==="
rg "S\.option" --glob "*.res" --glob "*.resi" -B1 -A1 | grep -v "^--$" | head -80
echo ""
echo "=== S.null usage ==="
rg "S\.null\(" --glob "*.res" --glob "*.resi" -B1 -A1 | grep -v "^--$" | head -60Repository: enviodev/hyperindex
Length of output: 18122
🏁 Script executed:
#!/bin/bash
# Look at test case more closely to see what the schema actually expects
rg -B10 -A10 "Issue `#931`" scenarios/test_codegen/test/RpcSource_test.res
# Check if there's documentation or definition of S.nullable
rg "let nullable|type nullable" --glob "*.res" --glob "*.resi" -A 5
# Verify the actual test expectations for transaction schema
rg -B5 -A20 "from.*s.matches.*S.option.*Address" scenarios/test_codegen/test/RpcSource_test.res | head -40Repository: enviodev/hyperindex
Length of output: 2613
Replace S.nullable with S.option for optional transaction fields.
The schema uses S.nullable() instead of the standard S.option() pattern used throughout the codebase for optional fields. This creates inconsistency and may cause type mismatches with test expectations, which explicitly expect S.option(). Update all S.nullable() calls in this schema to use S.option() instead.
Example pattern to follow (from RpcSource_test.res):
"from": s.matches(S.option(Address.schema)),
"to": s.matches(S.option(Address.schema)),
"gasPrice": s.matches(S.option(BigInt.nativeSchema)),🤖 Prompt for AI Agents
In `@codegenerator/cli/npm/envio/src/sources/Rpc.res` around lines 186 - 209, The
transaction schema in Rpc.res uses S.nullable for optional fields (e.g., the
fields named "from", "to", "gas", "gasPrice", "nonce", "transactionIndex",
"value", "type_", "v", "r", "s", "yParity", "maxPriorityFeePerGas",
"maxFeePerGas", "maxFeePerBlobGas", "blobVersionedHashes")—replace each
S.nullable(...) with S.option(...) to match the codebase convention and tests;
preserve the inner schemas (e.g., keep hexBigintSchema, hexIntSchema, S.string,
S.array(S.string) and the Address conversion/mapping used in the "from" and "to"
fields) so only the nullable->option wrapper changes.
Member
Author
|
Fixes #931 |
DZakh
commented
Jan 28, 2026
| miner: if lowercaseAddresses { | ||
| result.miner->Address.Evm.fromAddressLowercaseOrThrow | ||
| } else { | ||
| result.miner->Address.Evm.fromAddressOrThrow |
Member
Author
There was a problem hiding this comment.
Some rpc providers don't return checksummed data - eg megaeth
Collaborator
There was a problem hiding this comment.
Is miner the only address field? Probably 🤔
DZakh
pushed a commit
that referenced
this pull request
Mar 10, 2026
Cherry-pick from PR #930. Apply Address.Evm.fromAddressOrThrow when lowercaseAddresses=false for both log.address and block.miner fields. Previously, addresses from RPC providers that returned unchecksummed data would not be properly checksummed. https://claude.ai/code/session_017jqSDS8homENyhbexMaoG4
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
3 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
This PR adds comprehensive test coverage for a bug in RpcSource where addresses from RPC responses are not being checksummed when
lowercaseAddresses=false. The tests demonstrate the issue and validate the expected fix.Changes
log.addressis not checksummed (currently failing)block.mineris not checksummed (currently failing)Address.Evm.fromAddressOrThrowblock.minerchecksumming against live Ethereum mainnet datalog.addresschecksumming against live USDT contract logsImplementation Details
Notes
These tests serve as regression tests once the checksumming logic is fixed in RpcSource.res. The fix should apply
Address.Evm.fromAddressOrThrowto addresses whenlowercaseAddresses=falseinstead of returning them as-is.https://claude.ai/code/session_01LYMzy1hbNjwPxQqKEacfKW
Summary by CodeRabbit
Tests
Bug Fixes
Notes
✏️ Tip: You can customize this high-level summary in your review settings.