-
Notifications
You must be signed in to change notification settings - Fork 106
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
refactor(ethereum_clis): move TransitionTool.verify_fixture()
to StateTest
and BlockTest
#935
base: main
Are you sure you want to change the base?
Conversation
shutil.copyfile(fixture_path, debug_fixture_path) | ||
|
||
|
||
class GethTransitionTool(GethEvm, TransitionTool): |
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.
I feel It is not necessarily GethEvm, it just happened to be inside gethEvm
for evmone it will be a standalone (nested from ethereum cli) class? it is a bit confusing
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.
Addressed this in the latest commit:
GethEvm
is a construct only valid for geth, since this sameevm
command contains both functions, and is inherited by bothGethTransitionTool
andGethFixtureConsumerTool
.- In other tools, there would be a class that inherits
EthereumCLI
with a binary forTransitionTool
and another class that also inheritsEthereumCLI
for a different binary forFixtureConsumerTool
.
…tatetest` & `Blocktest`
bf0f91b
to
1ee0336
Compare
Summary of the latest commit:
I'm thinking we can go all the way in this PR and accept multiple |
…re_format` a mandatory parameter
… generalize consume direct test
🗒️ Description
This PR refactors
ethereum_clis
and theconsume direct
interface so that it can be extended to other clients.Previously, the
GethTransitionTool
class implemented fixture consumption and referred to it as "fixture verification". The reason is historical: the first "consumer" was implemented infill
.This PR:
GethStatetest
andGethBlocktest
interface consumer classes to theevm statetest
, respectivelyevm_blocktest
commands.GethFixtureConsumer
class which applies the appropriate consumer class for the fixture to run against the client.Note, that as we now instantiate the
GethStatetest
andGethBlocktest
classes from withinGethFixtureConsumer
we don't currently use any of the binary detection/automatic subclass instantiation provided byEthereumCLI
that is used for theTransitionTool
.🔗 Related Issues
None.
✅ Checklist