-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 hardhat config RPC methods #4596
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Thanks LGTM overall. I left some questions to double check some things, approving assuming none of those need changes.
@@ -118,7 +119,7 @@ pub enum Request { | |||
serialize_with = "single_to_sequence", | |||
deserialize_with = "sequence_to_single" | |||
)] | |||
SetPrevRandao(ZeroXPrefixedBytes), | |||
SetPrevRandao(B256), |
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.
Just double checking: does B256
serializes into 0x...
string?
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, it does.
) -> Result<bool, ProviderError> { | ||
let spec_id = data.spec_id(); | ||
if spec_id < SpecId::LONDON { | ||
return Err(ProviderError::UnmetHardfork { |
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.
Wondering if this is going to match error returned by Hardhat. Is that a concern at all @alcuadrado @fvictorio ?
/// The number of the block that the network forked from. | ||
pub fork_block_number: u64, | ||
/// The hash of the block that the network forked from. | ||
pub fork_block_hash: B256, |
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.
Noticed that the Hardhat type doesn't explicit say that this is a 0x-prefixed string. Are we sure it is?
// The hash of the block that the network forked from.
forkBlockHash: string;
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.
const latestBlockHashHex = latestBlock.header.hash().toString("hex");
const latestBlockHash = `0x${latestBlockHashHex}`;
a88a7d2
to
d07025d
Compare
d07025d
to
3fa4a72
Compare
As part of this change, I made the
network_id
au64
as there was no need for it to support up toU256
. It's represented as a string decimal in the JSON-RPC. TheU64
allows deserialization of these values too, making it a convenient helper.