-
Notifications
You must be signed in to change notification settings - Fork 245
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
feature(rpc): add remaining anvil rpc methods to provider #831
feature(rpc): add remaining anvil rpc methods to provider #831
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.
couple nits
@@ -77,5 +78,11 @@ ipc = ["pubsub", "alloy-rpc-client/ipc", "alloy-transport-ipc"] | |||
reqwest-default-tls = ["alloy-transport-http?/reqwest-default-tls"] | |||
reqwest-rustls-tls = ["alloy-transport-http?/reqwest-rustls-tls"] | |||
reqwest-native-tls = ["alloy-transport-http?/reqwest-native-tls"] | |||
anvil = ["reqwest", "dep:alloy-node-bindings", "dep:alloy-signer-wallet"] | |||
anvil-api = ["dep:alloy-rpc-types-anvil"] |
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.
unsure if newlines are from your formatter, or if other formatters would also produce them.
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.
its my formatter (Even Better TOML
extension) with the default configuration
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.
nits
} | ||
|
||
async fn anvil_set_next_block_timestamp(&self, seconds: u64) -> TransportResult<()> { | ||
self.client().request("evm_setNextBlockTimestamp", (seconds,)).await |
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.
some of these are declared as fn anvil but are evm_ ?
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.
yeah, these were originally introduced by ganache which is deprecated now, imo we can keep the anvil prefix because these are also aliased in anvil:
but these also exist on hardhat iirc
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.
Yeah I've used the anvil prefix if it is available in Foundry as such. This is the case for most methods but unfortunately not all.
Co-authored-by: DaniPopes <[email protected]>
…ectness to be handled in #838
Motivation
Closes #271
Solution
Implements Anvil API mirroring implementation in Foundry: https://github.com/foundry-rs/foundry/blob/00854b602ef0e67379a2027ccc5d0aad553e5333/crates/anvil/src/eth/api.rs#L1492-L2025
Found a couple of issues in Anvil, very likely unrelated to this API implementation that will requires follow-ups in Foundry:
evm_mine
,evm_mine_detailed
only mines a single block regardless of the configuration defined by MineOptions. Works OK withNone
.anvil_setBlock
logic is implemented in Anvil but not available on the API (dead code).anvil_enable_traces
is disabled in Anvil.anvil_reset
does not update thechain_id
correctly.Goal of the tests are mainly as a smoke test to make sure the API is correct.
PR Checklist