-
Notifications
You must be signed in to change notification settings - Fork 256
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
feat: add rpc namespace #994
Conversation
8bebeca
to
7ce869b
Compare
7ce869b
to
17a5ddf
Compare
@mattsse hi! |
@DaniPopes, maybe you want to take a look until it's gone stale? |
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.
sorry about the delay.
let's just move this type to the rpc-types crate, not worth having a standalone crate just for this.
crates/rpc-types-rpc/src/rpc.rs
Outdated
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 think this crate is a bit too much,
this only consists of 1 type which we can just move to the rpc-types crate, it's only one type.
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.
Changed!
One comment is that I didn't hide mod rpc behind the feature flag, because it will cause serde crate to be unused. If the feature flag is needed I could dig deeper into it.
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.
lgtm
* Implement Rpc namespace * Review comment * Dependency fix --------- Co-authored-by: Mikhail Sozin <[email protected]>
* Implement Rpc namespace * Review comment * Dependency fix --------- Co-authored-by: Mikhail Sozin <[email protected]>
Motivation
Adds RPC namespace for #926
Considerations
I took the type from reth rpc-types, so it could be migrated to alloy crate later.
The name of the crate came up a little bit silly
rpc-types-rpc
, so maybe better to rename it to rpc-types-namespaces, or something like thatBoth anvil and get do not support rpc_methods, so I did not implement a test to cover the call itself, but the structure is tested on the mock data.
Solution
Usage example. Replace RETH_ENDPOINT with endpoint
PR Checklist