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: Pass Correct Fork Name to T8N on Transition Forks #222

Merged
merged 7 commits into from
Jul 25, 2023

Conversation

marioevz
Copy link
Member

Changes Included

  • Correct fork is now passed to transition tools on transition forks:
    • E.g. on BerlinToLondonAt5, only Berlin and London forks are now passed to the transition on the appropriate block number
  • Transition fork classes now automatically derive information from their base classes

Fixes #208

Copy link
Collaborator

@spencer-tb spencer-tb left a comment

Choose a reason for hiding this comment

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

LGTM!

@danceratopz danceratopz marked this pull request as draft July 25, 2023 09:00
Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Rebased to get besu support that was added in #206.

Looks good! I did wonder about two changes to keep rather framework-specific code out of the transition tool interface implementation:

  1. Change base fork's fork() signature to take keyword arguments with default values in order to get "reasonable" default behaviour that return the merged fork.
  2. Avoid adding additional function arguments to transition tools' evaulate() methods.

More clarification below.

src/ethereum_test_forks/forks/forks.py Outdated Show resolved Hide resolved
src/ethereum_test_tools/spec/blockchain_test.py Outdated Show resolved Hide resolved
src/ethereum_test_tools/spec/state_test.py Outdated Show resolved Hide resolved
src/evm_transition_tool/besu.py Outdated Show resolved Hide resolved
src/evm_transition_tool/besu.py Outdated Show resolved Hide resolved
@danceratopz danceratopz marked this pull request as ready for review July 25, 2023 11:58
Co-authored-by: danceratopz <danceratopz@gmail.com>
@marioevz marioevz merged commit 99d46d9 into ethereum:main Jul 25, 2023
3 checks passed
@marioevz marioevz deleted the auto-transition-forks branch July 25, 2023 19:54
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.

pyspec pass transition fork to t8n
3 participants