From b89434f8311e139a7c52f3422d2e38998f2316da Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 00:30:14 +0800 Subject: [PATCH 01/10] Extract the hardcoded spec names into a `specs.py` file --- tests/core/pyspec/eth2spec/test/context.py | 79 +++---------------- .../pyspec/eth2spec/test/helpers/specs.py | 26 ++++++ .../pyspec/eth2spec/test/helpers/typing.py | 16 +++- 3 files changed, 50 insertions(+), 71 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/helpers/specs.py diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 84c5c636b..0adc24b5c 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -3,14 +3,6 @@ from copy import deepcopy 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 @@ -18,22 +10,28 @@ from .helpers.constants import ( PHASE0, ALTAIR, BELLATRIX, CAPELLA, DENEB, EIP6110, EIP7002, WHISK, - MINIMAL, MAINNET, + MINIMAL, ALL_PHASES, ALL_FORK_UPGRADES, 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,34 +42,6 @@ DEFAULT_TEST_PRESET = MINIMAL 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 @@ -79,37 +49,6 @@ class ForkMeta: 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) diff --git a/tests/core/pyspec/eth2spec/test/helpers/specs.py b/tests/core/pyspec/eth2spec/test/helpers/specs.py new file mode 100644 index 000000000..501d8b922 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/helpers/specs.py @@ -0,0 +1,26 @@ +from typing import ( + Dict, +) +from .constants import ( + MINIMAL, MAINNET, + ALL_PHASES, WHISK, +) +from .typing import ( + PresetBaseName, + SpecForkName, + Spec, +) + + +# NOTE: special case like `ALLOWED_TEST_RUNNER_FORKS` +ALL_EXECUTABLE_SPEC_NAMES = ALL_PHASES + (WHISK,) + +# import the spec for each fork and preset +for fork in ALL_EXECUTABLE_SPEC_NAMES: + exec(f"from eth2spec.{fork} import mainnet as spec_{fork}_mainnet, minimal as spec_{fork}_minimal") + +# this is the only output of this file +spec_targets: Dict[PresetBaseName, Dict[SpecForkName, Spec]] = { + MINIMAL: {fork: eval(f"spec_{fork}_minimal") for fork in ALL_EXECUTABLE_SPEC_NAMES}, + MAINNET: {fork: eval(f"spec_{fork}_mainnet") for fork in ALL_EXECUTABLE_SPEC_NAMES}, +} diff --git a/tests/core/pyspec/eth2spec/test/helpers/typing.py b/tests/core/pyspec/eth2spec/test/helpers/typing.py index 19657a8f7..df1da7a08 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/typing.py +++ b/tests/core/pyspec/eth2spec/test/helpers/typing.py @@ -1,4 +1,18 @@ -from typing import NewType +from typing import ( + NewType, + Protocol, + Sequence, +) SpecForkName = NewType("SpecForkName", str) PresetBaseName = NewType("PresetBaseName", str) +SpecForks = Sequence[SpecForkName] + + +class Configuration(Protocol): + PRESET_BASE: str + + +class Spec(Protocol): + fork: str + config: Configuration From d399cdedfab5b104e0fc3899276a64d3bcc57d2a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 00:56:41 +0800 Subject: [PATCH 02/10] Fix linter config --- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index cd0967e7c..fc3ae085b 100644 --- a/Makefile +++ b/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 From 67279448be9bc506263cafa145f178bfa96cc339 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 01:03:35 +0800 Subject: [PATCH 03/10] Fix import --- .../pyspec/eth2spec/test/phase0/fork_choice/test_ex_ante.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_ex_ante.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_ex_ante.py index 15feffa83..9978f7d46 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_ex_ante.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_ex_ante.py @@ -1,5 +1,4 @@ from eth2spec.test.context import ( - MAINNET, spec_state_test, with_altair_and_later, with_presets, @@ -11,6 +10,7 @@ from eth2spec.test.helpers.attestations import ( from eth2spec.test.helpers.block import ( build_empty_block, ) +from eth2spec.test.helpers.constants import MAINNET from eth2spec.test.helpers.fork_choice import ( get_genesis_forkchoice_store_and_block, on_tick_and_append_step, From f0c900282e2cd747a2ee636763b22f06e10b0d36 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 01:04:01 +0800 Subject: [PATCH 04/10] update new-feature doc --- docs/docs/new-feature.md | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/docs/docs/new-feature.md b/docs/docs/new-feature.md index b987e2e97..50c737460 100644 --- a/docs/docs/new-feature.md +++ b/docs/docs/new-feature.md @@ -70,24 +70,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 `` - -```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__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 `` to `ALL_PHASES` and `TESTGEN_FORKS` From 1d7c3d4164e555d762b085381756c13f28412ed0 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 03:00:06 +0800 Subject: [PATCH 05/10] Use md_doc_paths.py `is_post_fork` in pyspec itself; clean up `create_genesis_state` helper --- docs/docs/new-feature.md | 14 -------- tests/core/pyspec/eth2spec/test/context.py | 4 +-- .../pyspec/eth2spec/test/helpers/constants.py | 31 ++++++++++------ .../pyspec/eth2spec/test/helpers/forks.py | 35 +++++++++---------- .../pyspec/eth2spec/test/helpers/genesis.py | 30 +++++----------- 5 files changed, 48 insertions(+), 66 deletions(-) diff --git a/docs/docs/new-feature.md b/docs/docs/new-feature.md index 50c737460..0b00ebb84 100644 --- a/docs/docs/new-feature.md +++ b/docs/docs/new-feature.md @@ -79,20 +79,6 @@ You can refer to the previous fork's `fork.md` file. 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. - current_version = spec.config.EIP9999_FORK_VERSION -``` - - If the given feature changes `BeaconState` fields, you have to set the initial values by adding: ```python diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 0adc24b5c..d75a05447 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -12,7 +12,7 @@ from .helpers.constants import ( WHISK, MINIMAL, ALL_PHASES, - ALL_FORK_UPGRADES, + POST_FORK_OF, ALLOWED_TEST_RUNNER_FORKS, LIGHT_CLIENT_TESTING_FORKS, ) @@ -469,7 +469,7 @@ def with_phases(phases, other_phases=None): # 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` diff --git a/tests/core/pyspec/eth2spec/test/helpers/constants.py b/tests/core/pyspec/eth2spec/test/helpers/constants.py index 9dcaa6548..2d1b4a821 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/constants.py +++ b/tests/core/pyspec/eth2spec/test/helpers/constants.py @@ -45,7 +45,22 @@ TESTGEN_FORKS = (*MAINNET_FORKS, DENEB, EIP6110, WHISK) # 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, @@ -53,15 +68,11 @@ ALL_FORK_UPGRADES = { 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 diff --git a/tests/core/pyspec/eth2spec/test/helpers/forks.py b/tests/core/pyspec/eth2spec/test/helpers/forks.py index 9640f9585..99cab9a84 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/forks.py +++ b/tests/core/pyspec/eth2spec/test/helpers/forks.py @@ -1,27 +1,24 @@ from .constants import ( - PHASE0, ALTAIR, BELLATRIX, CAPELLA, DENEB, + ALTAIR, BELLATRIX, CAPELLA, DENEB, EIP6110, EIP7002, WHISK, + PREVIOUS_FORK_OF, ) -def is_post_fork(a, b): - if a == WHISK: - return b in [PHASE0, ALTAIR, BELLATRIX, CAPELLA, WHISK] - if a == EIP7002: - return b in [PHASE0, ALTAIR, BELLATRIX, CAPELLA, EIP7002] - if a == EIP6110: - return b in [PHASE0, ALTAIR, BELLATRIX, CAPELLA, DENEB, EIP6110] - if a == DENEB: - return b in [PHASE0, ALTAIR, BELLATRIX, CAPELLA, DENEB] - if a == CAPELLA: - return b in [PHASE0, ALTAIR, BELLATRIX, CAPELLA] - if a == BELLATRIX: - return b in [PHASE0, ALTAIR, BELLATRIX] - if a == ALTAIR: - return b in [PHASE0, ALTAIR] - if a == PHASE0: - return b in [PHASE0] - raise ValueError("Unknown fork name %s" % a) +def is_post_fork(a, b) -> bool: + """ + Returns true if fork a is after b, or if a == b + """ + if a == b: + return True + + prev_fork = PREVIOUS_FORK_OF[a] + if prev_fork == b: + return True + elif prev_fork is None: + return False + else: + return is_post_fork(prev_fork, b) def is_post_altair(spec): diff --git a/tests/core/pyspec/eth2spec/test/helpers/genesis.py b/tests/core/pyspec/eth2spec/test/helpers/genesis.py index 597e7e2ac..ab8273f66 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/genesis.py +++ b/tests/core/pyspec/eth2spec/test/helpers/genesis.py @@ -1,5 +1,6 @@ from eth2spec.test.helpers.constants import ( - ALTAIR, BELLATRIX, CAPELLA, DENEB, EIP6110, EIP7002, WHISK, + PHASE0, + PREVIOUS_FORK_OF, ) from eth2spec.test.helpers.execution_payload import ( compute_el_header_block_hash, @@ -77,26 +78,13 @@ def create_genesis_state(spec, validator_balances, activation_threshold): previous_version = spec.config.GENESIS_FORK_VERSION current_version = spec.config.GENESIS_FORK_VERSION - if spec.fork == ALTAIR: - current_version = spec.config.ALTAIR_FORK_VERSION - elif spec.fork == BELLATRIX: - previous_version = spec.config.ALTAIR_FORK_VERSION - current_version = spec.config.BELLATRIX_FORK_VERSION - elif spec.fork == CAPELLA: - previous_version = spec.config.BELLATRIX_FORK_VERSION - current_version = spec.config.CAPELLA_FORK_VERSION - elif spec.fork == DENEB: - previous_version = spec.config.CAPELLA_FORK_VERSION - current_version = spec.config.DENEB_FORK_VERSION - elif spec.fork == EIP6110: - previous_version = spec.config.DENEB_FORK_VERSION - current_version = spec.config.EIP6110_FORK_VERSION - elif spec.fork == EIP7002: - previous_version = spec.config.CAPELLA_FORK_VERSION - current_version = spec.config.EIP7002_FORK_VERSION - elif spec.fork == WHISK: - previous_version = spec.config.CAPELLA_FORK_VERSION - current_version = spec.config.WHISK_FORK_VERSION + if spec.fork != PHASE0: + previous_fork = PREVIOUS_FORK_OF[spec.fork] + if previous_fork == PHASE0: + previous_version = spec.config.GENESIS_FORK_VERSION + else: + previous_version = getattr(spec.config, f"{previous_fork.upper()}_FORK_VERSION") + current_version = getattr(spec.config, f"{spec.fork.upper()}_FORK_VERSION") state = spec.BeaconState( genesis_time=0, From eb16a77702ff9653ace969294214de00f140a151 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 03:24:42 +0800 Subject: [PATCH 06/10] Refactor `do_fork` with terrifying eval() and `PREVIOUS_FORK_OF` --- .../eth2spec/test/helpers/fork_transition.py | 57 +++++++------------ 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py index 80c54d9c1..93879512d 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py @@ -11,12 +11,8 @@ from eth2spec.test.helpers.block import ( ) 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, + PREVIOUS_FORK_OF, ) from eth2spec.test.helpers.deposits import ( prepare_state_and_deposit, @@ -146,45 +142,34 @@ def state_transition_across_slots_with_ignoring_proposers(spec, next_slot(spec, state) +def get_upgrade_fn(spec, fork): + 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( From c1945926b6b2b68cc6a06b5a948c23d272cc0620 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 03:27:13 +0800 Subject: [PATCH 07/10] update doc --- docs/docs/new-feature.md | 32 +++----------------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/docs/docs/new-feature.md b/docs/docs/new-feature.md index 0b00ebb84..c570c1580 100644 --- a/docs/docs/new-feature.md +++ b/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) @@ -92,32 +91,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. - 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) From b123f9faee809cf92f219dc53b0c63ec8aa6ecff Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 15 Dec 2023 19:12:57 +0800 Subject: [PATCH 08/10] update doc --- docs/docs/new-feature.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/docs/new-feature.md b/docs/docs/new-feature.md index c570c1580..b36129bd4 100644 --- a/docs/docs/new-feature.md +++ b/docs/docs/new-feature.md @@ -57,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 `SpecBuilder` in `pysetup/spec_builders/.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 From 9fbf75139dee9df9f4142db87a94fce6b4c3a17a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 21 Dec 2023 22:55:46 +0800 Subject: [PATCH 09/10] Add assertion to ensure eval usage is fine --- tests/core/pyspec/eth2spec/test/helpers/fork_transition.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py index 93879512d..329fbdbda 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py @@ -12,6 +12,7 @@ from eth2spec.test.helpers.block import ( from eth2spec.test.helpers.bls_to_execution_changes import get_signed_address_change from eth2spec.test.helpers.constants import ( PHASE0, + POST_FORK_OF, PREVIOUS_FORK_OF, ) from eth2spec.test.helpers.deposits import ( @@ -143,6 +144,9 @@ def state_transition_across_slots_with_ignoring_proposers(spec, 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}") From e52594634c89950359e971ca6db201c398e2f85f Mon Sep 17 00:00:00 2001 From: Parithosh Jayanthi Date: Thu, 4 Jan 2024 17:09:06 +0100 Subject: [PATCH 10/10] WIP: Update dockerisation (#3477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐳 add Dockerfile and dockerfile based script for consensus-spec tests --- .dockerignore | 2 + .github/workflows/run-tests.yml | 31 +++++++++ .gitignore | 3 + Makefile | 5 +- docker/Dockerfile | 22 +++++++ requirements_preinstallation.txt | 1 + scripts/build_run_docker_tests.sh | 103 ++++++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 .dockerignore create mode 100644 docker/Dockerfile create mode 100755 scripts/build_run_docker_tests.sh diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 000000000..792d2420d --- /dev/null +++ b/.dockerignore @@ -0,0 +1,2 @@ +**/venv +**/.venv \ No newline at end of file diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index f32311fa5..c7c43eef5 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -66,6 +66,37 @@ jobs: - name: Run linter for test generators run: make lint_generators + dockerfile-test: + runs-on: self-hosted + needs: preclear + services: + registry: + image: registry:2 + ports: + - 5000:5000 + steps: + - name: Checkout this repo + uses: actions/checkout@v3.2.0 + - name: get git commit hash + id: git_commit_hash + shell: bash + run: | + echo "git_commit_hash=$(echo $(git log --pretty=format:'%h' -n 1))" >> $GITHUB_OUTPUT + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v2 + with: + driver-opts: network=host + - name: Build and push to local registry + uses: docker/build-push-action@v4 + with: + context: . + file: ./docker/Dockerfile + push: true + tags: localhost:5000/consensus-specs-dockerfile-test:${{ steps.git_commit_hash.outputs.git_commit_hash }} + - name: Test the image by running the linter + run: | + docker run localhost:5000/consensus-specs-dockerfile-test:${{ steps.git_commit_hash.outputs.git_commit_hash }} make lint + pyspec-tests: runs-on: self-hosted needs: [preclear,lint,codespell,table_of_contents] diff --git a/.gitignore b/.gitignore index cdfddfb0c..e88207dd7 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,6 @@ docs/ssz docs/fork_choice docs/README.md site + +# docker test results +testResults diff --git a/Makefile b/Makefile index fc3ae085b..d0d750e89 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,7 @@ SOLIDITY_FILE_NAME = deposit_contract.json DEPOSIT_CONTRACT_TESTER_DIR = ${SOLIDITY_DEPOSIT_CONTRACT_DIR}/web3_tester CONFIGS_DIR = ./configs TEST_PRESET_TYPE ?= minimal +NUMBER_OF_CORES=16 # Collect a list of generator names GENERATORS = $(sort $(dir $(wildcard $(GENERATOR_DIR)/*/.))) # Map this list of generator paths to "gen_{generator name}" entries @@ -128,10 +129,10 @@ citest: pyspec mkdir -p $(TEST_REPORT_DIR); ifdef fork . venv/bin/activate; cd $(PY_SPEC_DIR); \ - python3 -m pytest -n 16 --bls-type=fastest --preset=$(TEST_PRESET_TYPE) --fork=$(fork) --junitxml=test-reports/test_results.xml eth2spec + python3 -m pytest -n $(NUMBER_OF_CORES) --bls-type=fastest --preset=$(TEST_PRESET_TYPE) --fork=$(fork) --junitxml=test-reports/test_results.xml eth2spec else . venv/bin/activate; cd $(PY_SPEC_DIR); \ - python3 -m pytest -n 16 --bls-type=fastest --preset=$(TEST_PRESET_TYPE) --junitxml=test-reports/test_results.xml eth2spec + python3 -m pytest -n $(NUMBER_OF_CORES) --bls-type=fastest --preset=$(TEST_PRESET_TYPE) --junitxml=test-reports/test_results.xml eth2spec endif diff --git a/docker/Dockerfile b/docker/Dockerfile new file mode 100644 index 000000000..771309931 --- /dev/null +++ b/docker/Dockerfile @@ -0,0 +1,22 @@ +# Rename the build stage from 'base' to 'builder' for clarification and code readability +FROM python:3.11.0-slim-bullseye as builder + +ENV DEBIAN_FRONTEND=noninteractive \ + WORKDIR=/consensus-specs \ + PIP_UPGRADE_CMD="python -m pip install --upgrade pip" \ + INSTALL_CMD="apt install -y git build-essential" + +RUN mkdir ${WORKDIR} +WORKDIR ${WORKDIR} + +# Chain the commands together +RUN apt update && ${INSTALL_CMD} && ${PIP_UPGRADE_CMD} && rm -rf /var/lib/apt/lists/* + +# Copy the current directory contents into the builder +COPY . . + +# Inline installation commands +RUN make install_test && \ + make preinstallation && \ + make pyspec + diff --git a/requirements_preinstallation.txt b/requirements_preinstallation.txt index 69d9a6660..02f6d2f0b 100644 --- a/requirements_preinstallation.txt +++ b/requirements_preinstallation.txt @@ -1,3 +1,4 @@ pip>=23.1.2 wheel>=0.40.0 setuptools>=68.0.0 +pylint>=3.0.0 \ No newline at end of file diff --git a/scripts/build_run_docker_tests.sh b/scripts/build_run_docker_tests.sh new file mode 100755 index 000000000..368fab94c --- /dev/null +++ b/scripts/build_run_docker_tests.sh @@ -0,0 +1,103 @@ +#! /bin/sh + +# Run 'consensus-specs' tests from a docker container instance. +# *Be sure to launch Docker before running this script.* +# +# It does the below: +# 1. Run pytest for consensus-specs in a container. +# 2. Copy and paste the coverage report. +# 3. Remove all exited containers that use the consensus-specs: images. + + +# Set variables +ALL_EXECUTABLE_SPECS=("phase0" "altair" "bellatrix" "capella" "deneb" "eip6110" "whisk") +TEST_PRESET_TYPE=minimal +FORK_TO_TEST=phase0 +NUMBER_OF_CORES=4 +WORKDIR="//consensus-specs//tests//core//pyspec" +ETH2SPEC_FOLDER_NAME="eth2spec" +CONTAINER_NAME="consensus-specs-tests" +DATE=$(date +"%Y%m%d-%H-%M") +# Default flag values +version=$(git log --pretty=format:'%h' -n 1) +IMAGE_NAME="consensus-specs:$version" +number_of_core=4 + +# displays the available options +display_help() { + echo "Run 'consensus-specs' tests from a container instance." + echo "Be sure to launch Docker before running this script." + echo + echo "Syntax: build_run_test.sh [--v TAG | --n NUMBER_OF_CORE | --f FORK_TO_TEST | --p PRESET_TYPE | --a | --h HELP]" + echo " --f Specify the fork to test" + echo " --i Specify the docker image to use" + echo " --n Specify the number of cores" + echo " --p Specify the test preset type" + echo " --a Test all forks" + echo " --h Display this help and exit" +} + +# Stop and remove the 'consensus-specs-dockerfile-test' container. +# If this container doesn't exist, then a error message is printed +# (but the process is not stopped). +cleanup() { + echo "Stop and remove the 'consensus-specs-tests' container." + docker stop $CONTAINER_NAME || true && docker rm $CONTAINER_NAME || true + +} + +# Copy the results from the container to a local folder +copy_test_results() { + local fork_name="$1" # Storing the first argument in a variable + + docker cp $CONTAINER_NAME:$WORKDIR/test-reports/test_results.xml ./testResults/test-results-$fork_name-$DATE.xml +} + +# Function to check if the Docker image already exists +image_exists() { + docker images --format '{{.Repository}}:{{.Tag}}' | grep -q "$1" +} + +# Parse command line arguments +while [[ "$#" -gt 0 ]]; do + case $1 in + --f) FORK_TO_TEST="$2"; shift ;; + --v) IMAGE_NAME="$2"; shift ;; + --n) NUMBER_OF_CORES="$2"; shift ;; + --p) TEST_PRESET_TYPE="$2"; shift ;; + --a) FORK_TO_TEST="all" ;; + --h) display_help; exit 0 ;; + *) echo "Unknown parameter: $1"; display_help; exit 1 ;; + esac + shift +done + +# initialize a test result directory +mkdir -p ./testResults + +# Only clean container after user exit console +trap cleanup SIGINT + +# Build Docker container if it doesn't exist +if ! image_exists "$IMAGE_NAME"; then + echo "Image $IMAGE_NAME does not exist. Building Docker image..." + docker build ../ -t $IMAGE_NAME -f ../docker/Dockerfile +else + echo "Image $IMAGE_NAME already exists. Skipping build..." +fi + +# Equivalent to `make citest with the subsequent flags` +if [ "$FORK_TO_TEST" == "all" ]; then + for fork in "${ALL_EXECUTABLE_SPECS[@]}"; do + docker run --name $CONTAINER_NAME $IMAGE_NAME \ + make citest fork=$fork TEST_PRESET_TYPE=$TEST_PRESET_TYPE NUMBER_OF_CORES=$NUMBER_OF_CORES + copy_test_results $fork + done +else + docker run --name $CONTAINER_NAME $IMAGE_NAME \ + make citest fork=$FORK_TO_TEST TEST_PRESET_TYPE=$TEST_PRESET_TYPE NUMBER_OF_CORES=$NUMBER_OF_CORES + copy_test_results $FORK_TO_TEST +fi + +# Stop and remove the container +cleanup