-
Notifications
You must be signed in to change notification settings - Fork 169
src(eth_config): added eth_config simulator prototype #2054
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
Conversation
Thanks @felix314159 ! i create a specific issue for eip-7910, could you please link this PR to the issue also. Also, do we need a simulator for hive according to our last discussion? |
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 for the changes! I think we may need to adjust the approach here though, since it looks like some existing functionality would be lost with this implementation. Left some comments with suggestions on how to proceed.
please take a look at the most recent commit, this should come close to what we want? |
920803e
to
f5c6034
Compare
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.
This needs to use pytest_generate_tests
, it currently overloads the fixture eth_rpc
in a weird way, there's no need to do that.
Implemented your feedback, let me know if we still need to change sth |
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 much better now IMO.
There's a lot of comments but mostly because the change in src/ethereum_test_rpc/rpc.py
broke consume
and execute
sadly. It's a small change but we need to be careful with the RPC package because of the implications it can have.
075a67b
to
534c09b
Compare
Implemented your feedback and rebased on main. The only thing still unclear to me is the |
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 very good! I just tried this locally with my suggestions added and both modes are working great.
I came around and I now think --clients
it's a very convenient addition, thanks!
Thanks for all the improvements. The only thing I left out was the |
Not sure what's going on but |
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.
Hi, just a couple more comments, sadly the changes to src/ethereum_test_rpc/rpc.py
are breaking uv run execute
so it needs to be fixed.
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!
Let's get this merged! Thanks!
🗒️ Description
You can run it via
uv run execute eth-config --network=Mainnet --rpc-endpoint=https://<replace>@rpc.nimbus-besu-1.fusaka-devnet-3.ethpandaops.io --clients="besu,erigon,nethermind,nimbusel,reth" --network-config-file=./src/pytest_plugins/execute/eth_config/networks.yml -vv -s
What happens
In the
test_eth_config_majority()
it requests and compares theeth_config
responses of all provided--clients
. Certain cl+el combinations might not be available so it picks the first working one for each exec client. The test is only passed if each execution client from the clients flag was successfully queried and all responses are identical. Otherwise an overview of which endpoints provided which response (which with hash) is given, so that client teams can be contacted about misconfiguration.All other tests in
execute_eth_config
only contact the--rpc-endpoint
, so here no additional combinations are derived and you only end up requesting info from one execution client.🔗 Related Issues or PRs
N/A.
✅ Checklist
tox
checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlint
type(scope):
.mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_from
marker.