Merge pull request #3562 from ethereum/refactor-context

Refactor pyspec to reduce some hardcoded spec fork names

Author: hwwhww

Makefile

@@ -34,11 +34,11 @@ MARKDOWN_FILES = $(wildcard $(SPEC_DIR)/*/*.md) \
                  $(wildcard $(SPEC_DIR)/_features/*/*/*.md) \
                  $(wildcard $(SSZ_DIR)/*.md)
 
-ALL_EXECUTABLE_SPECS = phase0 altair bellatrix capella deneb eip6110 whisk
+ALL_EXECUTABLE_SPEC_NAMES = phase0 altair bellatrix capella deneb eip6110 eip7002 whisk
 # The parameters for commands. Use `foreach` to avoid listing specs again.
-COVERAGE_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPECS), --cov=eth2spec.$S.$(TEST_PRESET_TYPE))
-PYLINT_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPECS), ./eth2spec/$S)
-MYPY_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPECS), -p eth2spec.$S)
+COVERAGE_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPEC_NAMES), --cov=eth2spec.$S.$(TEST_PRESET_TYPE))
+PYLINT_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPEC_NAMES), ./eth2spec/$S)
+MYPY_SCOPE := $(foreach S,$(ALL_EXECUTABLE_SPEC_NAMES), -p eth2spec.$S)
 
 COV_HTML_OUT=.htmlcov
 COV_HTML_OUT_DIR=$(PY_SPEC_DIR)/$(COV_HTML_OUT)
@@ -74,7 +74,7 @@ partial_clean:
 	rm -rf $(TEST_REPORT_DIR)
 	rm -rf eth2spec.egg-info dist build
 	rm -rf build;
-	@for spec_name in $(ALL_EXECUTABLE_SPECS) ; do \
+	@for spec_name in $(ALL_EXECUTABLE_SPEC_NAMES) ; do \
 		echo $$spec_name; \
 		rm -rf $(ETH2SPEC_MODULE_DIR)/$$spec_name; \
 	done

docs/docs/new-feature.md

@@ -11,13 +11,12 @@
   - [4. Add `fork.md`](#4-add-forkmd)
   - [5. Make it executable](#5-make-it-executable)
 - [B: Make it executable for pytest and test generator](#b-make-it-executable-for-pytest-and-test-generator)
-  - [1. Add `light-client/*` docs if you updated the content of `BeaconBlock`](#1-add-light-client-docs-if-you-updated-the-content-of-beaconblock)
+  - [1. [Optional] Add `light-client/*` docs if you updated the content of `BeaconBlock`](#1-optional-add-light-client-docs-if-you-updated-the-content-of-beaconblock)
   - [2. Add the mainnet and minimal presets and update the configs](#2-add-the-mainnet-and-minimal-presets-and-update-the-configs)
   - [3. Update `context.py`](#3-update-contextpy)
   - [4. Update `constants.py`](#4-update-constantspy)
   - [5. Update `genesis.py`:](#5-update-genesispy)
-  - [6. To add fork transition tests, update fork_transition.py](#6-to-add-fork-transition-tests-update-fork_transitionpy)
-  - [7. Update CI configurations](#7-update-ci-configurations)
+  - [6. Update CI configurations](#6-update-ci-configurations)
 - [Others](#others)
   - [Bonus](#bonus)
   - [Need help?](#need-help)
@@ -58,6 +57,8 @@ You can refer to the previous fork's `fork.md` file.
     - Update [`pysetup/constants.py`](https://github.com/ethereum/consensus-specs/blob/dev/constants.py) with the new feature name as Pyspec `constants.py` defined.
     - Update [`pysetup/spec_builders/__init__.py`](https://github.com/ethereum/consensus-specs/blob/dev/pysetup/spec_builders/__init__.py). Implement a new `<FEATURE_NAME>SpecBuilder` in `pysetup/spec_builders/<FEATURE_NAME>.py` with the new feature name. e.g., `EIP9999SpecBuilder`. Append it to the `spec_builders` list.
     - Update [`pysetup/md_doc_paths.py`](https://github.com/ethereum/consensus-specs/blob/dev/pysetup/md_doc_paths.py): add the path of the new markdown files in `get_md_doc_paths` function if needed.
+- Update `PREVIOUS_FORK_OF` setting in both [`test/helpers/constants.py`](https://github.com/ethereum/consensus-specs/blob/dev/constants.py) and [`pysetup/md_doc_paths.py`](https://github.com/ethereum/consensus-specs/blob/dev/pysetup/md_doc_paths.py).
+    - NOTE: since these two modules (the pyspec itself and the spec builder tool) must be separate, the fork sequence setting has to be defined again.
 
 ## B: Make it executable for pytest and test generator
 
@@ -70,24 +71,7 @@ You can refer to the previous fork's `fork.md` file.
 - Update configs: `configs/mainnet.yaml` and `configs/minimal.yaml`
 
 ### 3. Update [`context.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/context.py)
-- Update `spec_targets` by adding `<NEW_FEATURE>`
-
-```python
-from eth2spec.eip9999 import mainnet as spec_eip9999_mainnet, minimal as spec_eip9999_minimal
-
-...
-
-spec_targets: Dict[PresetBaseName, Dict[SpecForkName, Spec]] = {
-    MINIMAL: {
-        ...
-        EIP9999: spec_eip9999_minimal,
-    },
-    MAINNET: {
-        ...
-        EIP9999: spec_eip9999_mainnet
-    },
-}
-```
+- [Optional] Add `with_<new-feature-name>_and_later` decorator for writing pytest cases. e.g., `with_capella_and_later`.
 
 ### 4. Update [`constants.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/helpers/constants.py)
 - Add `<NEW_FEATURE>` to `ALL_PHASES` and `TESTGEN_FORKS`
@@ -96,20 +80,6 @@ spec_targets: Dict[PresetBaseName, Dict[SpecForkName, Spec]] = {
 
 We use `create_genesis_state` to create the default `state` in tests.
 
-- Update `create_genesis_state` by adding `fork_version` setting:
-
-```python
-def create_genesis_state(spec, validator_balances, activation_threshold):
-    ...
-    if spec.fork == ALTAIR:
-        current_version = spec.config.ALTAIR_FORK_VERSION
-    ...
-    elif spec.fork == EIP9999:
-        # Add the previous fork version of given fork
-        previous_version = spec.config.<PREVIOUS_FORK_VERSION>
-        current_version = spec.config.EIP9999_FORK_VERSION
-```
-
 - If the given feature changes `BeaconState` fields, you have to set the initial values by adding:
 
 ```python
@@ -123,32 +93,7 @@ def create_genesis_state(spec, validator_balances, activation_threshold):
 
 - If the given feature changes `ExecutionPayload` fields, you have to set the initial values by updating `get_sample_genesis_execution_payload_header` helper.
 
-### 6. To add fork transition tests, update [fork_transition.py](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py)
-
-```python
-def do_fork(state, spec, post_spec, fork_epoch, with_block=True, sync_aggregate=None, operation_dict=None):
-    ...
-
-    if post_spec.fork == ALTAIR:
-        state = post_spec.upgrade_to_altair(state)
-    ...
-    elif post_spec.fork == EIP9999:
-        state = post_spec.upgrade_to_eip9999(state)
-
-    ...
-
-    if post_spec.fork == ALTAIR:
-        assert state.fork.previous_version == post_spec.config.GENESIS_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.ALTAIR_FORK_VERSION
-    ...
-    elif post_spec.fork == EIP9999:
-        assert state.fork.previous_version == post_spec.config.<PREVIOUS_FORK_VERSION>
-        assert state.fork.current_version == post_spec.config.EIP9999_FORK_VERSION
-
-    ...
-```
-
-### 7. Update CI configurations
+### 6. Update CI configurations
 - Update [GitHub Actions config](https://github.com/ethereum/consensus-specs/blob/dev/.github/workflows/run-tests.yml)
     - Update `pyspec-tests.strategy.matrix.version` list by adding new feature to it
 - Update [CircleCI config](https://github.com/ethereum/consensus-specs/blob/dev/.circleci/config.yml)

tests/core/pyspec/eth2spec/test/context.py

@@ -3,37 +3,35 @@
 from dataclasses import dataclass
 import importlib
 
-from eth2spec.phase0 import mainnet as spec_phase0_mainnet, minimal as spec_phase0_minimal
-from eth2spec.altair import mainnet as spec_altair_mainnet, minimal as spec_altair_minimal
-from eth2spec.bellatrix import mainnet as spec_bellatrix_mainnet, minimal as spec_bellatrix_minimal
-from eth2spec.capella import mainnet as spec_capella_mainnet, minimal as spec_capella_minimal
-from eth2spec.deneb import mainnet as spec_deneb_mainnet, minimal as spec_deneb_minimal
-from eth2spec.eip6110 import mainnet as spec_eip6110_mainnet, minimal as spec_eip6110_minimal
-from eth2spec.whisk import mainnet as spec_whisk_mainnet, minimal as spec_whisk_minimal
-from eth2spec.eip7002 import mainnet as spec_eip7002_mainnet, minimal as spec_eip7002_minimal
 from eth2spec.utils import bls
 
 from .exceptions import SkippedTest
 from .helpers.constants import (
     PHASE0, ALTAIR, BELLATRIX, CAPELLA, DENEB,
     EIP6110, EIP7002,
     WHISK,
-    MINIMAL, MAINNET,
+    MINIMAL,
     ALL_PHASES,
-    ALL_FORK_UPGRADES,
+    POST_FORK_OF,
     ALLOWED_TEST_RUNNER_FORKS,
     LIGHT_CLIENT_TESTING_FORKS,
 )
 from .helpers.forks import is_post_fork
-from .helpers.typing import SpecForkName, PresetBaseName
 from .helpers.genesis import create_genesis_state
+from .helpers.typing import (
+    Spec,
+    SpecForks,
+)
+from .helpers.specs import (
+    spec_targets,
+)
 from .utils import (
     vector_test,
     with_meta_tags,
 )
 
 from random import Random
-from typing import Any, Callable, Sequence, TypedDict, Protocol, Dict
+from typing import Any, Callable, Sequence, Dict
 
 from lru import LRU
 
@@ -44,72 +42,13 @@
 DEFAULT_PYTEST_FORKS = ALL_PHASES
 
 
-# TODO: currently phases are defined as python modules.
-# It would be better if they would be more well-defined interfaces for stronger typing.
-
-class Configuration(Protocol):
-    PRESET_BASE: str
-
-
-class Spec(Protocol):
-    fork: str
-    config: Configuration
-
-
-class SpecPhase0(Spec):
-    ...
-
-
-class SpecAltair(Spec):
-    ...
-
-
-class SpecBellatrix(Spec):
-    ...
-
-
-class SpecCapella(Spec):
-    ...
-
-
 @dataclass(frozen=True)
 class ForkMeta:
     pre_fork_name: str
     post_fork_name: str
     fork_epoch: int
 
 
-spec_targets: Dict[PresetBaseName, Dict[SpecForkName, Spec]] = {
-    MINIMAL: {
-        PHASE0: spec_phase0_minimal,
-        ALTAIR: spec_altair_minimal,
-        BELLATRIX: spec_bellatrix_minimal,
-        CAPELLA: spec_capella_minimal,
-        DENEB: spec_deneb_minimal,
-        EIP6110: spec_eip6110_minimal,
-        EIP7002: spec_eip7002_minimal,
-        WHISK: spec_whisk_minimal,
-    },
-    MAINNET: {
-        PHASE0: spec_phase0_mainnet,
-        ALTAIR: spec_altair_mainnet,
-        BELLATRIX: spec_bellatrix_mainnet,
-        CAPELLA: spec_capella_mainnet,
-        DENEB: spec_deneb_mainnet,
-        EIP6110: spec_eip6110_mainnet,
-        EIP7002: spec_eip7002_mainnet,
-        WHISK: spec_whisk_mainnet,
-    },
-}
-
-
-class SpecForks(TypedDict, total=False):
-    PHASE0: SpecPhase0
-    ALTAIR: SpecAltair
-    BELLATRIX: SpecBellatrix
-    CAPELLA: SpecCapella
-
-
 def _prepare_state(balances_fn: Callable[[Any], Sequence[int]], threshold_fn: Callable[[Any], int],
                    spec: Spec, phases: SpecForks):
     balances = balances_fn(spec)
@@ -530,7 +469,7 @@ def wrapper(*args, **kw):
                     # When running test generator, it sets specific `phase`
                     phase = kw['phase']
                     _phases = [phase]
-                    _other_phases = [ALL_FORK_UPGRADES[phase]]
+                    _other_phases = [POST_FORK_OF[phase]]
                     ret = _run_test_case_with_phases(fn, _phases, _other_phases, kw, args, is_fork_transition=True)
                 else:
                     # When running pytest, go through `fork_metas` instead of using `phases`

tests/core/pyspec/eth2spec/test/helpers/constants.py

@@ -45,23 +45,34 @@
 # Forks allowed in the test runner `--fork` flag, to fail fast in case of typos
 ALLOWED_TEST_RUNNER_FORKS = (*ALL_PHASES, WHISK)
 
-ALL_FORK_UPGRADES = {
+# NOTE: the same definition as in `pysetup/md_doc_paths.py`
+PREVIOUS_FORK_OF = {
+    # post_fork_name: pre_fork_name
+    PHASE0: None,
+    ALTAIR: PHASE0,
+    BELLATRIX: ALTAIR,
+    CAPELLA: BELLATRIX,
+    DENEB: CAPELLA,
+    # Experimental patches
+    EIP6110: DENEB,
+    WHISK: CAPELLA,
+    EIP7002: CAPELLA,
+}
+
+# For fork transition tests
+POST_FORK_OF = {
     # pre_fork_name: post_fork_name
     PHASE0: ALTAIR,
     ALTAIR: BELLATRIX,
     BELLATRIX: CAPELLA,
     CAPELLA: DENEB,
     DENEB: EIP6110,
 }
-ALL_PRE_POST_FORKS = ALL_FORK_UPGRADES.items()
-AFTER_BELLATRIX_UPGRADES = {key: value for key, value in ALL_FORK_UPGRADES.items() if key != PHASE0}
-AFTER_BELLATRIX_PRE_POST_FORKS = AFTER_BELLATRIX_UPGRADES.items()
-AFTER_CAPELLA_UPGRADES = {key: value for key, value in ALL_FORK_UPGRADES.items()
-                          if key not in [PHASE0, ALTAIR]}
-AFTER_CAPELLA_PRE_POST_FORKS = AFTER_CAPELLA_UPGRADES.items()
-AFTER_DENEB_UPGRADES = {key: value for key, value in ALL_FORK_UPGRADES.items()
-                        if key not in [PHASE0, ALTAIR, BELLATRIX]}
-AFTER_DENEB_PRE_POST_FORKS = AFTER_DENEB_UPGRADES.items()
+
+ALL_PRE_POST_FORKS = POST_FORK_OF.items()
+DENEB_TRANSITION_UPGRADES_AND_AFTER = {key: value for key, value in POST_FORK_OF.items()
+                                       if key not in [PHASE0, ALTAIR, BELLATRIX]}
+AFTER_DENEB_PRE_POST_FORKS = DENEB_TRANSITION_UPGRADES_AND_AFTER.items()
 
 #
 # Config and Preset

tests/core/pyspec/eth2spec/test/helpers/fork_transition.py

@@ -11,12 +11,9 @@
 )
 from eth2spec.test.helpers.bls_to_execution_changes import get_signed_address_change
 from eth2spec.test.helpers.constants import (
-    ALTAIR,
-    BELLATRIX,
-    CAPELLA,
-    DENEB,
-    EIP6110,
-    EIP7002,
+    PHASE0,
+    POST_FORK_OF,
+    PREVIOUS_FORK_OF,
 )
 from eth2spec.test.helpers.deposits import (
     prepare_state_and_deposit,
@@ -146,45 +143,37 @@ def state_transition_across_slots_with_ignoring_proposers(spec,
             next_slot(spec, state)
 
 
+def get_upgrade_fn(spec, fork):
+    # pylint: disable=unused-argument
+    # NOTE: `spec` is used for the `eval` call
+    assert fork in POST_FORK_OF.values()
+    try:
+        # TODO: make all upgrade_to_* function names consistent?
+        fn = eval(f"spec.upgrade_to_{fork}")
+        return fn
+    except Exception:
+        raise ValueError(f"Unknown fork: {fork}")
+
+
 def do_fork(state, spec, post_spec, fork_epoch, with_block=True, sync_aggregate=None, operation_dict=None):
     spec.process_slots(state, state.slot + 1)
 
     assert state.slot % spec.SLOTS_PER_EPOCH == 0
     assert spec.get_current_epoch(state) == fork_epoch
 
-    if post_spec.fork == ALTAIR:
-        state = post_spec.upgrade_to_altair(state)
-    elif post_spec.fork == BELLATRIX:
-        state = post_spec.upgrade_to_bellatrix(state)
-    elif post_spec.fork == CAPELLA:
-        state = post_spec.upgrade_to_capella(state)
-    elif post_spec.fork == DENEB:
-        state = post_spec.upgrade_to_deneb(state)
-    elif post_spec.fork == EIP6110:
-        state = post_spec.upgrade_to_eip6110(state)
-    elif post_spec.fork == EIP7002:
-        state = post_spec.upgrade_to_eip7002(state)
+    state = get_upgrade_fn(post_spec, post_spec.fork)(state)
 
     assert state.fork.epoch == fork_epoch
 
-    if post_spec.fork == ALTAIR:
-        assert state.fork.previous_version == post_spec.config.GENESIS_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.ALTAIR_FORK_VERSION
-    elif post_spec.fork == BELLATRIX:
-        assert state.fork.previous_version == post_spec.config.ALTAIR_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.BELLATRIX_FORK_VERSION
-    elif post_spec.fork == CAPELLA:
-        assert state.fork.previous_version == post_spec.config.BELLATRIX_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.CAPELLA_FORK_VERSION
-    elif post_spec.fork == DENEB:
-        assert state.fork.previous_version == post_spec.config.CAPELLA_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.DENEB_FORK_VERSION
-    elif post_spec.fork == EIP6110:
-        assert state.fork.previous_version == post_spec.config.DENEB_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.EIP6110_FORK_VERSION
-    elif post_spec.fork == EIP7002:
-        assert state.fork.previous_version == post_spec.config.CAPELLA_FORK_VERSION
-        assert state.fork.current_version == post_spec.config.EIP7002_FORK_VERSION
+    previous_fork = PREVIOUS_FORK_OF[post_spec.fork]
+    if previous_fork == PHASE0:
+        previous_version = spec.config.GENESIS_FORK_VERSION
+    else:
+        previous_version = getattr(post_spec.config, f"{previous_fork.upper()}_FORK_VERSION")
+    current_version = getattr(post_spec.config, f"{post_spec.fork.upper()}_FORK_VERSION")
+
+    assert state.fork.previous_version == previous_version
+    assert state.fork.current_version == current_version
 
     if with_block:
         return state, _state_transition_and_sign_block_at_slot(