parser: make Parser2 network-generic (unblock raindex AnyNetwork)#521
Conversation
`eth_call` and the `Parser2`/`ParserV2` methods were bound to `P: Provider`, which defaults to `Provider<Ethereum>`. Callers using a `Provider<AnyNetwork>` (e.g. raindex, which is multi-chain and builds AnyNetwork providers) could not pass them to `parse_text`/`parse`/`parse_pragma` — the bound required `Provider<Ethereum>`. Parameterise over `N: Network` (`P: Provider<N>`) and build the call via `TransactionBuilder` so `N::TransactionRequest` is used. Parsing is a plain `eth_call` with no network-specific fields, so this is purely a relaxation of the bound; existing Ethereum-provider callers are unaffected (N is inferred). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR generalizes the ChangesNetwork-generic parser API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
crates/parser/src/v2.rs (1)
26-57: ⚡ Quick winAdd one regression test for
Provider<AnyNetwork>.This API change is specifically meant to unblock
Provider<AnyNetwork>, but the tests in this file still only cover the default mocked provider path. A small test that type-checks and callsparse_text/parse_pragma_textwith an actualProvider<AnyNetwork>would lock in the contract this PR is introducing and catch future regressions early.🤖 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 `@crates/parser/src/v2.rs` around lines 26 - 57, Add a small regression test that type-checks and invokes the new API with a real Provider<AnyNetwork>: create a test that constructs a Provider<AnyNetwork> (or uses the existing provider test helper configured for AnyNetwork), calls Parser::parse_text and Parser::parse_pragma_text (verifying returned types parse2Return and Vec<Address>/parsePragma1Return as appropriate), and asserts the results or that calls complete successfully; ensure the test references the parse_text and parse_pragma_text methods and uses Provider<AnyNetwork> so the compiler will catch regressions to the generic Provider<AnyNetwork> API.
🤖 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.
Nitpick comments:
In `@crates/parser/src/v2.rs`:
- Around line 26-57: Add a small regression test that type-checks and invokes
the new API with a real Provider<AnyNetwork>: create a test that constructs a
Provider<AnyNetwork> (or uses the existing provider test helper configured for
AnyNetwork), calls Parser::parse_text and Parser::parse_pragma_text (verifying
returned types parse2Return and Vec<Address>/parsePragma1Return as appropriate),
and asserts the results or that calls complete successfully; ensure the test
references the parse_text and parse_pragma_text methods and uses
Provider<AnyNetwork> so the compiler will catch regressions to the generic
Provider<AnyNetwork> API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0dc1362-fb76-430f-b3b8-a4fb8ff1a3f7
📒 Files selected for processing (1)
crates/parser/src/v2.rs
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=S |
1 participant
Why
Parser2/ParserV2and the internaleth_callwere bound toP: Provider, which defaults toProvider<Ethereum>. raindex is multi-chain and buildsProvider<AnyNetwork>providers; it cannot pass them toparse_text/parse/parse_pragma— the compiler reportsProvider<Ethereum>is required but onlyProvider<AnyNetwork>is implemented. This blocks raindex's crates.io cutover (raindex_common::add_ordercallsparse_text(.., &provider)with an AnyNetwork provider).What
Parameterise over
N: Network(P: Provider<N>) and build the request viaalloy::network::TransactionBuildersoN::TransactionRequestis used instead of the hardcoded EthereumTransactionRequest. Parsing is a plaineth_callwith no network-specific fields, so this only relaxes the bound — existing Ethereum-provider callers are unaffected (Nis inferred).Verification
cargo test -p rainlang_parser: 4 pass (Ethereum-provider mocks, unchanged).cargo check --workspace(native): clean.test-wasm-build: clean (both#[cfg]trait variants).Once merged, Package Release republishes the parser; raindex pins the new version to finish its cutover. Relates to rainlang#519/#520.
Summary by CodeRabbit