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
Implement MainnetTesterChain #92
Conversation
This is just going to track my progress. I'm developing this concurrently with this branch in the ethereum-tester library: ethereum/eth-tester#14 |
a3aa9cb
to
51e7faf
Compare
51e7faf
to
8b76f13
Compare
evm/vm/flavors/tester/__init__.py
Outdated
parent_header, | ||
**assoc(header_params, 'gas_limit', parent_header.gas_limit) | ||
) | ||
return create_tester_header_from_parent |
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.
Wouldn't it make more sense to wrap it with classmethod() here?
evm/vm/flavors/tester/__init__.py
Outdated
Creates and initializes a new block header from the provided | ||
`parent_header`. | ||
""" | ||
return parent_class.create_header_from_parent( |
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 don't understand why we want to ignore the class on which the method was called (vm_class) and instead always use parent_class? Maybe it deserves a comment?
evm/vm/flavors/tester/__init__.py
Outdated
|
||
|
||
@reversed_return | ||
def _generate_vm_configuration(homestead=None, dao=None, anti_dos=None): |
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.
It took me a while to realize what values the arguments here could take. Maybe add a _start_block suffix to them, or a docstring?
evm/vm/flavors/tester/__init__.py
Outdated
""" | ||
pass | ||
|
||
def configure_forks(self, homestead=None, dao=None, anti_dos=0): |
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.
Same here about the args names
@gsalgado I believe I've addressed all of your comments. Merging when green unless I hear otherwise. |
evm/vm/flavors/tester/__init__.py
Outdated
|
||
class MaintainGasLimitMixin(object): | ||
@classmethod | ||
def create_tester_header_from_parent(cls, parent_header, **header_params): |
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.
Shouldn't this be create_header_from_parent
?
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.
YES! good catch.
fixing test failure here: https://github.com/pipermerriam/py-evm/pull/105 |
minor formatting updates, remove additional docs to separate pr
Blocker for: ethereum/eth-tester#14
What was wrong?
Py-EVM needs a tester chain that can be used with testing. Primary features needed are:
How was it fixed?
Added a new
Chain
subclass underevm.vm.flavors.tester
with these properties.Cute Animal Picture