feat: add eth1wrap#87
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces the eth1wrap module, providing an Ethereum execution layer RPC client interface that wraps Alloy's Provider. Similar to how the Go implementation wraps go-ethereum's ethclient.Client, this implementation adds Charon-specific functionality like ERC-1271 smart contract signature verification with built-in retry support for rate limiting.
Key Changes
- New
EthClientstruct wrapping Alloy'sDynProviderwith automatic retry/backoff for rate limiting - Integration of ERC-1271 contract signature verification via
verify_smart_contract_based_signaturemethod - Addition of Alloy, thiserror, and url dependencies to the workspace
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
crates/charon/src/eth1wrap/mod.rs |
Core implementation of EthClient with error types, retry configuration, and ERC-1271 signature verification |
crates/charon/src/eth1wrap/build/IERC1271.abi |
ABI definition for ERC-1271 isValidSignature function |
crates/charon/src/lib.rs |
Module export for eth1wrap |
crates/charon/Cargo.toml |
Dependency additions for alloy, thiserror, and url |
Cargo.toml |
Workspace-level dependency definitions |
flake.nix |
Added protobuf to nix development dependencies |
Cargo.lock |
Dependency lock file updates for Alloy ecosystem and transitive dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Coverage (base → head): |
varex83
left a comment
varex83
left a comment
There was a problem hiding this comment.
LGTM!
One question: do you plan to add tests in the future for this?
|
Also, can you rename the PR so it will have the same style as other PRs |
I think this was discussed during our dailies but for the time being there are no tests since the original Go tests are pretty much testing mocks. The only relevant tests that they have is to ensure that the magic value used in If you have any suggestion for particular tests then I'm happy to add them in a later PR. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let instance = IERC1271::new(address, &self.0); | ||
|
|
||
| let call = instance | ||
| .isValidSignature(hash.into(), sig.to_vec().into()) |
There was a problem hiding this comment.
The sig.to_vec() call creates an unnecessary allocation by copying the slice into a new Vec. Since Alloy types typically accept references or slices, consider checking if the generated binding accepts Cow<[u8]> or a slice reference directly to avoid the allocation.
| .isValidSignature(hash.into(), sig.to_vec().into()) | |
| .isValidSignature(hash.into(), sig.into()) |
Solves #40
Solves #27
The existing Go implementation adds an interface that wraps a
ethclient.Clientfromgo-ethereum. Here, we do something similar by wrapping analloy::providers::Providerfrom the Alloy family of crates, while adding methods required specifically by Charon (ex.VerifySmartContractBasedSignature).Alloy uses reqwest as the underlying HTTP client. Adding retries in a general fashion to this client is tricky since "standard" solutions like reqwest-middleware are incompatible with Alloy.
The proposed solution for now is to use the retry mechanism provided directly by Alloy: https://docs.rs/alloy-transport/latest/alloy_transport/layers/struct.RetryBackoffLayer.html . An other option (and I think the most suitable) is to use tower's Exponential Backoff, which is the underlying mechanism used by Alloy itself.
I'd like to separate PRs into one for
eth1wrap(this one) and other for expbackoff and retry modules as defined in the original Go implementation, and then make the required changes to use the appropriate retry mechanism.