Skip to content
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

Add eth_feeHistory RPC endpoint #2094

Merged
merged 3 commits into from
Aug 4, 2021
Merged

Conversation

mike6649
Copy link
Contributor

@mike6649 mike6649 commented Jul 31, 2021

What was wrong?

Post-london RPC endpoint eth_feeHistory is not currently supported. This PR adds support for it with fee_history in the eth module.
Related to Issue #2038

How was it fixed?

Following the eth1.0-specs, fee_history function was added to the eth module.

Todo:

  • Add support to async eth module
  • Testing
  • Add entry to the release notes

Cute Animal Picture

@mike6649 mike6649 marked this pull request as ready for review August 1, 2021 10:40
@mike6649
Copy link
Contributor Author

mike6649 commented Aug 1, 2021

Hi! Please let me know if there is any feedback to the implementation so far. I'm just roughly following the implementations of similar RPC methods, but I'm not sure whether the tests are written properly. Thanks all!

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @mike6649! This looks like what we were going for! I added some nitpicky comments around the use of apply_list_to_array_formatter vs. apply_formatter_to_array that I'm curious to get your feedback on, but looks good other than that!

Also if you don't mind adding a newsfragment, that would be helpful. If you don't get to it, one of the maintainers can before merge. Thanks again!

web3/_utils/method_formatters.py Outdated Show resolved Hide resolved
web3/_utils/method_formatters.py Outdated Show resolved Hide resolved
web3/_utils/method_formatters.py Outdated Show resolved Hide resolved
@mike6649
Copy link
Contributor Author

mike6649 commented Aug 3, 2021

Hi @kclowes , I made the suggested changes above and added a newsfragment. Let me know if there is anything else! 👍

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks @mike6649! I'll get it merged soon!

@kclowes kclowes changed the title [WIP] Add eth_feeHistory RPC endpoint Add eth_feeHistory RPC endpoint Aug 4, 2021
@kclowes kclowes merged commit d722f31 into ethereum:master Aug 4, 2021
@mike6649 mike6649 deleted the feehistory branch August 14, 2021 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants