-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(anvil): add anvil_getIntervalMining
API
#9290
feat(anvil): add anvil_getIntervalMining
API
#9290
Conversation
@grandizzy Followed your suggestion and created this new PR (old #9090). Please check if there is anything to improve. |
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, thank you! Just a small nit, IMO we should be consistent naming at par with anvil_setIntervalMining
e.g. anvil_getIntervalMining
(instead anvil_getIntervalMine
)
@grandizzy Yes, it's reasonable. The PR has been updated. |
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, thank you! Pending additional review before merging.
anvil_getIntervalMining
API
Thanks for your guidance. I'm so excited, this is my first PR for Foundry. Hope to contribute more in the future. |
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.
great, this is a lot simpler
/// Returns the value of mining interval, if set. | ||
/// | ||
/// Handler for ETH RPC call: `anvil_getIntervalMing` | ||
pub fn anvil_get_interval_ming(&self) -> Result<Option<u64>> { |
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.
should this be anvil_get_interval_mining
not anvil_get_interval_ming
? (and the comment above)
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.
thank you! yes indeed, missed the typo 😁 preparing an update in #9292
* feat: add anvil_get_interval_mine method * refactor: keep consistent naming --------- Co-authored-by: Your Name <you@example.com> Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com>
Motivation
Closes #8813
Solution