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

feature: add RPC eth_getBlockReceipts #3247

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Feb 23, 2024

What was wrong?

Related to Issue #2004

How was it fixed?

eth_getBlockReceipts was introduced in the standard RPC ethereum/execution-apis#438, we can simply implement it

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

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.

Thank you for this PR! Let me know if any of my comments don't make sense or need clarification.

@@ -642,11 +642,32 @@ def test_eth_getBlockByNumber_finalized(
) -> None:
super().test_eth_getBlockByNumber_finalized(w3, empty_block)

def test_eth_fee_history(self, w3: "Web3") -> None:
super().test_eth_fee_history(w3)
def test_eth_getBlockReceipts_hash(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we don't have eth_getBlockReceipts implemented in eth-tester, we'll need to add these tests to the not_implemented section that's closer to the beginning of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excuse me, may I ask if this is the change? It seems that even after the change here, it has not taken effect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kclowes /ping

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the ping. Now that you are listing the tests as not_implemented above, you can delete them here and the tests should pass.

web3/_utils/module_testing/eth_module.py Outdated Show resolved Hide resolved
web3/_utils/module_testing/eth_module.py Outdated Show resolved Hide resolved
web3/_utils/method_formatters.py Outdated Show resolved Hide resolved
@kclowes
Copy link
Collaborator

kclowes commented Mar 4, 2024

I don't think this is failing due to anything you added, @jsvisa. I'll see if I can straighten it out. Thanks!

Signed-off-by: jsvisa <delweng@gmail.com>
Signed-off-by: jsvisa <delweng@gmail.com>
@kclowes kclowes marked this pull request as ready for review March 12, 2024 04:41
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 @jsvisa!

@kclowes kclowes merged commit 4cbb780 into ethereum:main Mar 12, 2024
81 checks passed
@jsvisa jsvisa deleted the eth_getBlockReceipts branch March 12, 2024 05:12
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.

None yet

2 participants