-
Notifications
You must be signed in to change notification settings - Fork 130
feat(l2): add helper functions for l2 eth client #5433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a helper function get_batch_number() to retrieve the current batch number from the L2 RPC endpoint ethrex_batchNumber, which will be useful for rex and future components.
- Adds
get_batch_number()function to query theethrex_batchNumberRPC endpoint - Introduces
GetBatchNumberErrorenum with variants for JSON serialization, RPC errors, and integer parsing - Integrates the new error type into the main
EthClientErrorenum
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/networking/rpc/clients/eth/errors.rs | Adds GetBatchNumberError enum with SerdeJSONError, RPCError, and ParseIntError variants, and integrates it into EthClientError |
| crates/l2/networking/rpc/clients.rs | Implements get_batch_number() function that calls ethrex_batchNumber RPC endpoint and parses the hex-encoded batch number response |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lines of code reportTotal lines added: Detailed view |
| #[derive(Debug, thiserror::Error)] | ||
| pub enum GetBatchNumberError { | ||
| #[error("{0}")] | ||
| SerdeJSONError(#[from] serde_json::Error), | ||
| #[error("{0}")] | ||
| RPCError(String), | ||
| #[error("{0}")] | ||
| ParseIntError(#[from] std::num::ParseIntError), | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the enum above this has a TODO that says Move to L2, do you know if there's an issue created for that? Would it be easy to achieve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't find any. The closes was this
JereSalo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, idk if it was tested but it would be good if it was
It tested it manually |
Motivation
We want this new functions that will be useful for
rexand future use.Description
Add
get_batch_number()function to callethrex_batchNumberAdd
send_ethrex_transaction()function to callethrex_sendTransaction