-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(rpc): add eth_multicallV1
#5596
feat(rpc): add eth_multicallV1
#5596
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.
this looks pretty good already.
we also want this in the EthApi
traits in the rpc-api crate
ty lot |
@mattsse does this look better? |
multicall_bundle: Bundle, | ||
state_context: Option<StateContext>, | ||
mut state_override: Option<StateOverride>, | ||
) -> EthResult<Vec<EthCallResponse>> { |
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.
this is just the same as call_many?
so atm we don't need this function and can reuse call_many instead
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.
this is just the same as call_many?
so atm we don't need this function and can reuse call_many instead
hey ty for your review, you means i should directly call the function call_many inside this function ?
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.
yes, at least for now
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.
yes, at least for now
ty so much, very greatful from your part all these review. should be fine now
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.
the PR does not implement eth_multicallV1
according to the spec, please, re-read the spec or let smb else pick up the issue
/// Executes complex RPC calls to Ethereum nodes | ||
|
||
pub async fn eth_multicall_v1( |
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.
/// Executes complex RPC calls to Ethereum nodes | |
pub async fn eth_multicall_v1( | |
/// Executes complex RPC calls to Ethereum nodes. | |
/// Ref: <https://github.com/ethereum/execution-apis/pull/484> | |
pub async fn eth_multicall_v1( |
@@ -18,6 +18,15 @@ pub trait EthApi { | |||
#[method(name = "protocolVersion")] | |||
async fn protocol_version(&self) -> RpcResult<U64>; | |||
|
|||
/// Executes complex RPC calls to Ethereum nodes | |||
#[method(name = "ethMulticallV1")] |
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.
jsonrpc method names are case sensitive and eth prefix is already specified on the trait
#[method(name = "ethMulticallV1")] | |
#[method(name = "multicallV1")] |
@@ -51,6 +51,17 @@ where | |||
EthApiSpec::protocol_version(self).await.to_rpc_result() | |||
} | |||
|
|||
/// Handler for ! `eth_MultiCallV1` |
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.
/// Handler for ! `eth_MultiCallV1` | |
/// Handler for ! `eth_multicallV1` |
eth_multicallv1
eth_multicallv1
eth_multicallV1
ref : #5586