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

Unify optimized implementations #811

Merged

Conversation

petertdavies
Copy link
Collaborator

What was wrong?

Having to duplicate the code in ethereum_optimized on a per-fork basis was unwieldy and annoying.

How was it fixed?

The optimized implementations now consist of a function that takes a fork and returns a dictionary of optimized patches.

Cute Animal Picture

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (2da1ad3) 74.06% compared to head (da285e2) 74.06%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #811   +/-   ##
=======================================
  Coverage   74.06%   74.06%           
=======================================
  Files         571      571           
  Lines       31726    31726           
=======================================
  Hits        23498    23498           
  Misses       8228     8228           
Flag Coverage Δ
unittests 74.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines 67 to 81
for fork_name in (
"frontier",
"homestead",
"dao_fork",
"tangerine_whistle",
"spurious_dragon",
"byzantium",
"constantinople",
"istanbul",
"muir_glacier",
"berlin",
"london",
"arrow_glacier",
"gray_glacier",
):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there some way we can put this information into the fork itself and retrieve it using functions from ethereum_spec_tools/forks.py?

I really want to keep metadata about forks (like these pre-/post-merge lists) in as few places as possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Having thought about this it is possible and I have done it.

src/ethereum_optimized/fork.py Outdated Show resolved Hide resolved
src/ethereum_optimized/state_db.py Outdated Show resolved Hide resolved
@SamWilsn SamWilsn force-pushed the unify-optimized-implementations branch from f07d022 to 5360030 Compare July 12, 2023 17:01
@petertdavies petertdavies force-pushed the unify-optimized-implementations branch from 0911467 to 6d02d09 Compare July 27, 2023 20:55
if (
isinstance(fork.criteria, ByBlockNumber)
and fork.short_name != "paris"
):
monkey_patch_optimized_spec(fork.short_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would the Hardfork.consensus property from #820 work here?

@petertdavies petertdavies force-pushed the unify-optimized-implementations branch from 6d02d09 to 21427b9 Compare August 3, 2023 11:01
@SamWilsn SamWilsn merged commit decd4e5 into ethereum:master Aug 5, 2023
4 checks passed
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