From 0b0fe15c75408ff6419848132c798ba0d4e0d4d3 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 29 Oct 2021 23:17:57 +0800 Subject: [PATCH 01/48] Make altair transition tests support merge forks --- .../transition/test_activations_and_exits.py | 31 ++- .../test/altair/transition/test_leaking.py | 19 +- .../test/altair/transition/test_operations.py | 31 ++- .../test/altair/transition/test_slashing.py | 15 +- .../test/altair/transition/test_transition.py | 39 +-- tests/core/pyspec/eth2spec/test/context.py | 249 +++++++++++++----- .../pyspec/eth2spec/test/helpers/constants.py | 5 + .../eth2spec/test/helpers/fork_transition.py | 22 +- .../pyspec/eth2spec/test/utils/__init__.py | 2 - .../core/pyspec/eth2spec/test/utils/utils.py | 48 ---- tests/generators/transition/main.py | 40 +-- 11 files changed, 300 insertions(+), 201 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py index 12aa815ad8..0f5b28db67 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py @@ -1,12 +1,15 @@ import random from eth2spec.test.context import ( - MINIMAL, - fork_transition_test, + ForkMeta, with_presets, + with_fork_metas, +) +from eth2spec.test.helpers.constants import ( + ALL_FORKS, + MINIMAL, ) -from eth2spec.test.helpers.constants import PHASE0, ALTAIR from eth2spec.test.helpers.fork_transition import ( - do_altair_fork, + do_fork, transition_until_fork, transition_to_next_epoch_and_append_blocks, ) @@ -21,7 +24,7 @@ # Exit # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) @with_presets([MINIMAL], reason="only test with enough validators such that at least one exited index is not in sync committee") def test_transition_with_one_fourth_exiting_validators_exit_post_fork(state, @@ -59,7 +62,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_post_fork(state, # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # ensure that some of the current sync committee members are exiting @@ -81,7 +84,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_post_fork(state, yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, fork_epoch, spec, @@ -117,7 +120,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # check post transition state @@ -127,10 +130,6 @@ def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, assert not post_spec.is_active_validator(validator, post_spec.get_current_epoch(state)) assert not post_spec.is_in_inactivity_leak(state) - # ensure that none of the current sync committee members are exited validators - exited_pubkeys = [state.validators[index].pubkey for index in exited_indices] - assert not any(set(exited_pubkeys).intersection(list(state.current_sync_committee.pubkeys))) - # continue regular state transition with new spec into next epoch transition_to_next_epoch_and_append_blocks(post_spec, state, post_tag, blocks, only_last_block=True) @@ -143,7 +142,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_with_non_empty_activation_queue(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create some deposits before the transition @@ -161,7 +160,7 @@ def test_transition_with_non_empty_activation_queue(state, fork_epoch, spec, pos # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition with new spec into next epoch @@ -171,7 +170,7 @@ def test_transition_with_non_empty_activation_queue(state, fork_epoch, spec, pos yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_with_activation_at_fork_epoch(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create some deposits before the transition @@ -191,7 +190,7 @@ def test_transition_with_activation_at_fork_epoch(state, fork_epoch, spec, post_ # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition with new spec into next epoch diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py index 6cdac16610..3ed2e37ee2 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py @@ -1,13 +1,18 @@ -from eth2spec.test.context import fork_transition_test -from eth2spec.test.helpers.constants import PHASE0, ALTAIR +from eth2spec.test.context import ( + ForkMeta, + with_fork_metas, +) +from eth2spec.test.helpers.constants import ( + ALL_FORKS, +) from eth2spec.test.helpers.fork_transition import ( - do_altair_fork, + do_fork, transition_until_fork, transition_to_next_epoch_and_append_blocks, ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=7) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=7) for pre, post in ALL_FORKS.items()]) def test_transition_with_leaking_pre_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Leaking starts at epoch 6 (MIN_EPOCHS_TO_INACTIVITY_PENALTY + 2). @@ -22,7 +27,7 @@ def test_transition_with_leaking_pre_fork(state, fork_epoch, spec, post_spec, pr # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # check post transition state @@ -35,7 +40,7 @@ def test_transition_with_leaking_pre_fork(state, fork_epoch, spec, post_spec, pr yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=6) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=6) for pre, post in ALL_FORKS.items()]) def test_transition_with_leaking_at_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Leaking starts at epoch 6 (MIN_EPOCHS_TO_INACTIVITY_PENALTY + 2). @@ -50,7 +55,7 @@ def test_transition_with_leaking_at_fork(state, fork_epoch, spec, post_spec, pre # irregular state transition to handle fork: blocks = [] - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # check post transition state diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py index e19c57fb1c..54f37a2350 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py @@ -1,8 +1,13 @@ from eth2spec.test.context import ( + ForkMeta, always_bls, - fork_transition_test, + with_fork_metas, + with_presets, +) +from eth2spec.test.helpers.constants import ( + ALL_FORKS, + MINIMAL, ) -from eth2spec.test.helpers.constants import PHASE0, ALTAIR from eth2spec.test.helpers.fork_transition import ( OperationType, run_transition_with_operation, @@ -13,7 +18,7 @@ # PROPOSER_SLASHING # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) @always_bls def test_transition_with_proposer_slashing_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -31,7 +36,7 @@ def test_transition_with_proposer_slashing_right_after_fork(state, fork_epoch, s ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) @always_bls def test_transition_with_proposer_slashing_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -54,7 +59,7 @@ def test_transition_with_proposer_slashing_right_before_fork(state, fork_epoch, # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) @always_bls def test_transition_with_attester_slashing_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -72,7 +77,7 @@ def test_transition_with_attester_slashing_right_after_fork(state, fork_epoch, s ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) @always_bls def test_transition_with_attester_slashing_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -95,7 +100,7 @@ def test_transition_with_attester_slashing_right_before_fork(state, fork_epoch, # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_with_deposit_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a deposit right *after* the transition @@ -112,7 +117,7 @@ def test_transition_with_deposit_right_after_fork(state, fork_epoch, spec, post_ ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_with_deposit_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a deposit right *before* the transition @@ -134,11 +139,12 @@ def test_transition_with_deposit_right_before_fork(state, fork_epoch, spec, post # -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=260) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_FORKS.items()]) +@with_presets([MINIMAL], reason="too slow") def test_transition_with_voluntary_exit_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a voluntary exit right *after* the transition. - fork_epoch=260 because mainnet `SHARD_COMMITTEE_PERIOD` is 256 epochs. + fork_epoch=66 because minimal preset `SHARD_COMMITTEE_PERIOD` is 64 epochs. """ # Fast forward to the future epoch so that validator can do voluntary exit state.slot = spec.config.SHARD_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH @@ -155,11 +161,12 @@ def test_transition_with_voluntary_exit_right_after_fork(state, fork_epoch, spec ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=260) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_FORKS.items()]) +@with_presets([MINIMAL], reason="too slow") def test_transition_with_voluntary_exit_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a voluntary exit right *before* the transition. - fork_epoch=260 because mainnet `SHARD_COMMITTEE_PERIOD` is 256 epochs. + fork_epoch=66 because minimal preset `SHARD_COMMITTEE_PERIOD` is 64 epochs. """ # Fast forward to the future epoch so that validator can do voluntary exit state.slot = spec.config.SHARD_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py index 211a4fbfee..6d8f23b7dd 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py @@ -1,12 +1,15 @@ import random from eth2spec.test.context import ( - MINIMAL, - fork_transition_test, + ForkMeta, + with_fork_metas, with_presets, ) -from eth2spec.test.helpers.constants import PHASE0, ALTAIR +from eth2spec.test.helpers.constants import ( + ALL_FORKS, + MINIMAL, +) from eth2spec.test.helpers.fork_transition import ( - do_altair_fork, + do_fork, transition_to_next_epoch_and_append_blocks, transition_until_fork, ) @@ -15,7 +18,7 @@ ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=1) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=1) for pre, post in ALL_FORKS.items()]) @with_presets([MINIMAL], reason="only test with enough validators such that at least one exited index is not in sync committee") def test_transition_with_one_fourth_slashed_active_validators_pre_fork(state, @@ -45,7 +48,7 @@ def test_transition_with_one_fourth_slashed_active_validators_pre_fork(state, yield "pre", state # irregular state transition to handle fork: - state, _ = do_altair_fork(state, spec, post_spec, fork_epoch, with_block=False) + state, _ = do_fork(state, spec, post_spec, fork_epoch, with_block=False) # ensure that some of the current sync committee members are slashed slashed_pubkeys = [state.validators[index].pubkey for index in slashed_indices] diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py index fe6248c5f3..2439b79fcd 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py @@ -1,12 +1,17 @@ import random -from eth2spec.test.context import fork_transition_test -from eth2spec.test.helpers.constants import PHASE0, ALTAIR +from eth2spec.test.context import ( + ForkMeta, + with_fork_metas, +) +from eth2spec.test.helpers.constants import ( + ALL_FORKS, +) from eth2spec.test.helpers.state import ( next_epoch_via_signed_block, ) from eth2spec.test.helpers.attestations import next_slots_with_attestations from eth2spec.test.helpers.fork_transition import ( - do_altair_fork, + do_fork, no_blocks, only_at, skip_slots, @@ -15,7 +20,7 @@ ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_normal_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -34,7 +39,7 @@ def test_normal_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag ]) # irregular state transition to handle fork: - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition with new spec into next epoch @@ -51,7 +56,7 @@ def test_normal_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_missing_first_post_block(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -71,7 +76,7 @@ def test_transition_missing_first_post_block(state, fork_epoch, spec, post_spec, ]) # irregular state transition to handle fork: - state, _ = do_altair_fork(state, spec, post_spec, fork_epoch, with_block=False) + state, _ = do_fork(state, spec, post_spec, fork_epoch, with_block=False) # continue regular state transition with new spec into next epoch transition_to_next_epoch_and_append_blocks(post_spec, state, post_tag, blocks) @@ -88,7 +93,7 @@ def test_transition_missing_first_post_block(state, fork_epoch, spec, post_spec, yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_missing_last_pre_fork_block(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -109,7 +114,7 @@ def test_transition_missing_last_pre_fork_block(state, fork_epoch, spec, post_sp ]) # irregular state transition to handle fork: - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition with new spec into next epoch @@ -127,7 +132,7 @@ def test_transition_missing_last_pre_fork_block(state, fork_epoch, spec, post_sp yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) def test_transition_only_blocks_post_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -148,7 +153,7 @@ def test_transition_only_blocks_post_fork(state, fork_epoch, spec, post_spec, pr ]) # irregular state transition to handle fork: - state, _ = do_altair_fork(state, spec, post_spec, fork_epoch, with_block=False) + state, _ = do_fork(state, spec, post_spec, fork_epoch, with_block=False) # continue regular state transition with new spec into next epoch to_slot = post_spec.SLOTS_PER_EPOCH + state.slot @@ -215,7 +220,7 @@ def _run_transition_test_with_attestations(state, assert (state.slot + 1) % spec.SLOTS_PER_EPOCH == 0 # irregular state transition to handle fork: - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition with new spec into next epoch @@ -253,7 +258,7 @@ def _run_transition_test_with_attestations(state, yield "post", state -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=3) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) def test_transition_with_finality(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -262,7 +267,7 @@ def test_transition_with_finality(state, fork_epoch, spec, post_spec, pre_tag, p yield from _run_transition_test_with_attestations(state, fork_epoch, spec, post_spec, pre_tag, post_tag) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=3) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) def test_transition_with_random_three_quarters_participation(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -289,7 +294,7 @@ def _drop_random_quarter(_slot, _index, indices): ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=3) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) def test_transition_with_random_half_participation(state, fork_epoch, spec, post_spec, pre_tag, post_tag): rng = random.Random(2020) @@ -313,7 +318,7 @@ def _drop_random_half(_slot, _index, indices): ) -@fork_transition_test(PHASE0, ALTAIR, fork_epoch=2) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) def test_transition_with_no_attestations_until_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the ``fork_epoch`` with no attestations, @@ -332,7 +337,7 @@ def test_transition_with_no_attestations_until_after_fork(state, fork_epoch, spe ]) # irregular state transition to handle fork: - state, block = do_altair_fork(state, spec, post_spec, fork_epoch) + state, block = do_fork(state, spec, post_spec, fork_epoch) blocks.append(post_tag(block)) # continue regular state transition but add attestations diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 944351bfde..8dd7c88d5f 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -1,5 +1,7 @@ import pytest from copy import deepcopy +from dataclasses import dataclass + 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.merge import mainnet as spec_merge_mainnet, minimal as spec_merge_minimal @@ -7,12 +9,16 @@ from .exceptions import SkippedTest from .helpers.constants import ( - SpecForkName, PresetBaseName, PHASE0, ALTAIR, MERGE, MINIMAL, MAINNET, ALL_PHASES, FORKS_BEFORE_ALTAIR, FORKS_BEFORE_MERGE, + ALL_FORKS, ) +from .helpers.typing import SpecForkName, PresetBaseName from .helpers.genesis import create_genesis_state -from .utils import vector_test, with_meta_tags, build_transition_test +from .utils import ( + vector_test, + with_meta_tags, +) from random import Random from typing import Any, Callable, Sequence, TypedDict, Protocol, Dict @@ -50,6 +56,13 @@ class SpecMerge(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, @@ -86,7 +99,6 @@ def _prepare_state(balances_fn: Callable[[Any], Sequence[int]], threshold_fn: Ca def with_custom_state(balances_fn: Callable[[Any], Sequence[int]], threshold_fn: Callable[[Any], int]): def deco(fn): - def entry(*args, spec: Spec, phases: SpecForks, **kw): # make a key for the state, unique to the fork + config (incl preset choice) and balances/activations key = (spec.fork, spec.config.__hash__(), spec.__file__, balances_fn, threshold_fn) @@ -104,7 +116,7 @@ def entry(*args, spec: Spec, phases: SpecForks, **kw): return deco -def default_activation_threshold(spec): +def default_activation_threshold(spec: Spec): """ Helper method to use the default balance activation threshold for state creation for tests. Usage: `@with_custom_state(threshold_fn=default_activation_threshold, ...)` @@ -112,7 +124,7 @@ def default_activation_threshold(spec): return spec.MAX_EFFECTIVE_BALANCE -def zero_activation_threshold(spec): +def zero_activation_threshold(spec: Spec): """ Helper method to use 0 gwei as the activation threshold for state creation for tests. Usage: `@with_custom_state(threshold_fn=zero_activation_threshold, ...)` @@ -120,7 +132,7 @@ def zero_activation_threshold(spec): return 0 -def default_balances(spec): +def default_balances(spec: Spec): """ Helper method to create a series of default balances. Usage: `@with_custom_state(balances_fn=default_balances, ...)` @@ -129,7 +141,7 @@ def default_balances(spec): return [spec.MAX_EFFECTIVE_BALANCE] * num_validators -def scaled_churn_balances(spec): +def scaled_churn_balances(spec: Spec): """ Helper method to create enough validators to scale the churn limit. (This is *firmly* over the churn limit -- thus the +2 instead of just +1) @@ -143,7 +155,7 @@ def scaled_churn_balances(spec): with_state = with_custom_state(default_balances, default_activation_threshold) -def low_balances(spec): +def low_balances(spec: Spec): """ Helper method to create a series of low balances. Usage: `@with_custom_state(balances_fn=low_balances, ...)` @@ -154,7 +166,7 @@ def low_balances(spec): return [low_balance] * num_validators -def misc_balances(spec): +def misc_balances(spec: Spec): """ Helper method to create a series of balances that includes some misc. balances. Usage: `@with_custom_state(balances_fn=misc_balances, ...)` @@ -166,7 +178,7 @@ def misc_balances(spec): return balances -def misc_balances_in_default_range_with_many_validators(spec): +def misc_balances_in_default_range_with_many_validators(spec: Spec): """ Helper method to create a series of balances that includes some misc. balances but none that are below the ``EJECTION_BALANCE``. @@ -182,7 +194,7 @@ def misc_balances_in_default_range_with_many_validators(spec): return balances -def low_single_balance(spec): +def low_single_balance(spec: Spec): """ Helper method to create a single of balance of 1 Gwei. Usage: `@with_custom_state(balances_fn=low_single_balance, ...)` @@ -190,7 +202,7 @@ def low_single_balance(spec): return [1] -def large_validator_set(spec): +def large_validator_set(spec: Spec): """ Helper method to create a large series of default balances. Usage: `@with_custom_state(balances_fn=default_balances, ...)` @@ -347,6 +359,66 @@ def decorator(fn): return decorator +def _get_preset_targets(kw): + preset_name = DEFAULT_TEST_PRESET + if 'preset' in kw: + preset_name = kw.pop('preset') + return spec_targets[preset_name] + + +def _get_run_phases(phases, kw): + """ + Return the fork names for the base `spec` in test cases + """ + if 'phase' in kw: + # Limit phases if one explicitly specified + phase = kw.pop('phase') + if phase not in phases: + dump_skipping_message(f"doesn't support this fork: {phase}") + return None + run_phases = [phase] + else: + # If pytest `--fork` flag is set, filter out the rest forks + run_phases = set(phases).intersection(DEFAULT_PYTEST_FORKS) + + return run_phases + + +def _get_available_phases(run_phases, other_phases): + """ + The return the available fork names for multi-phase tests + """ + available_phases = set(run_phases) + if other_phases is not None: + available_phases |= set(other_phases) + return available_phases + + +def _run_test_case_with_phases(fn, phases, other_phases, kw, args, is_fork_transition=False): + run_phases = _get_run_phases(phases, kw) + + if len(run_phases) == 0: + if not is_fork_transition: + dump_skipping_message("none of the recognized phases are executable, skipping test.") + return None + + available_phases = _get_available_phases(run_phases, other_phases) + + targets = _get_preset_targets(kw) + + # Populate all phases for multi-phase tests + phase_dir = {} + for phase in available_phases: + phase_dir[phase] = targets[phase] + + # Return is ignored whenever multiple phases are ran. + # This return is for test generators to emit python generators (yielding test vector outputs) + for phase in run_phases: + ret = fn(spec=targets[phase], phases=phase_dir, *args, **kw) + + return ret + + def with_phases(phases, other_phases=None): """ Decorator factory that returns a decorator that runs a test for the appropriate phases. @@ -354,49 +426,24 @@ def with_phases(phases, other_phases=None): """ def decorator(fn): def wrapper(*args, **kw): - run_phases = set(phases).intersection(DEFAULT_PYTEST_FORKS) - - # limit phases if one explicitly specified - if 'phase' in kw: - phase = kw.pop('phase') - if phase not in phases: - dump_skipping_message(f"doesn't support this fork: {phase}") - return None - run_phases = [phase] - - if PHASE0 not in run_phases and ALTAIR not in run_phases and MERGE not in run_phases: - dump_skipping_message("none of the recognized phases are executable, skipping test.") - return None - - available_phases = set(run_phases) - if other_phases is not None: - available_phases |= set(other_phases) - - preset_name = DEFAULT_TEST_PRESET - if 'preset' in kw: - preset_name = kw.pop('preset') - targets = spec_targets[preset_name] - - # Populate all phases for multi-phase tests - phase_dir = {} - if PHASE0 in available_phases: - phase_dir[PHASE0] = targets[PHASE0] - if ALTAIR in available_phases: - phase_dir[ALTAIR] = targets[ALTAIR] - if MERGE in available_phases: - phase_dir[MERGE] = targets[MERGE] - - # return is ignored whenever multiple phases are ran. - # This return is for test generators to emit python generators (yielding test vector outputs) - if PHASE0 in run_phases: - ret = fn(spec=targets[PHASE0], phases=phase_dir, *args, **kw) - if ALTAIR in run_phases: - ret = fn(spec=targets[ALTAIR], phases=phase_dir, *args, **kw) - if MERGE in run_phases: - ret = fn(spec=targets[MERGE], phases=phase_dir, *args, **kw) - - # TODO: merge, sharding, custody_game and das are not executable yet. - # Tests that specify these features will not run, and get ignored for these specific phases. + if 'fork_metas' in kw: + fork_metas = kw.pop('fork_metas') + if 'phase' in kw: + # When running test generator, it sets specific `phase` + phase = kw['phase'] + _phases = [phase] + _other_phases = [ALL_FORKS[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` + for fork_meta in fork_metas: + pre_fork_name = fork_meta.pre_fork_name + post_fork_name = fork_meta.post_fork_name + _phases = [pre_fork_name] + _other_phases = [post_fork_name] + ret = _run_test_case_with_phases(fn, _phases, _other_phases, kw, args, is_fork_transition=True) + else: + ret = _run_test_case_with_phases(fn, phases, other_phases, kw, args) return ret return wrapper return decorator @@ -481,10 +528,25 @@ def _wrapper(*args, **kwargs): return _decorator -def fork_transition_test(pre_fork_name, post_fork_name, fork_epoch=None): +# +# Fork transition state tests +# + + +def set_fork_metas(fork_metas: Sequence[ForkMeta]): + def decorator(fn): + def wrapper(*args, **kwargs): + return fn(*args, fork_metas=fork_metas, **kwargs) + return wrapper + return decorator + + +def with_fork_metas(fork_metas: Sequence[ForkMeta]): """ - A decorator to construct a "transition" test from one fork of the eth2 spec - to another. + A decorator to construct a "transition" test from one fork to another. + + Decorator takes a list of `ForkMeta` and each item defines `pre_fork_name`, + `post_fork_name`, and `fork_epoch`. Decorator assumes a transition from the `pre_fork_name` fork to the `post_fork_name` fork. The user can supply a `fork_epoch` at which the @@ -502,15 +564,62 @@ def fork_transition_test(pre_fork_name, post_fork_name, fork_epoch=None): `post_tag`: a function to tag data as belonging to `post_fork_name` fork. Used to discriminate data during consumption of the generated spec tests. """ - def _wrapper(fn): - @with_phases([pre_fork_name], other_phases=[post_fork_name]) - @spec_test - @with_state - def _adapter(*args, **kwargs): - wrapped = build_transition_test(fn, - pre_fork_name, - post_fork_name, - fork_epoch=fork_epoch) - return wrapped(*args, **kwargs) - return _adapter - return _wrapper + run_yield_fork_meta = yield_fork_meta(fork_metas) + run_with_phases = with_phases(ALL_PHASES) + run_set_fork_metas = set_fork_metas(fork_metas) + + def decorator(fn): + return run_set_fork_metas(run_with_phases(spec_test(with_state(run_yield_fork_meta(fn))))) + return decorator + + +def yield_fork_meta(fork_metas: Sequence[ForkMeta]): + """ + Yield meta fields to `meta.yaml` and pass post spec and meta fields to `fn`. + """ + def decorator(fn): + def wrapper(*args, **kw): + phases = kw.pop('phases') + spec = kw["spec"] + fork_meta = [m for m in fork_metas if m.pre_fork_name == spec.fork][0] + fork_meta = next(filter(lambda m: m.pre_fork_name == spec.fork, fork_metas)) + post_spec = phases[fork_meta.post_fork_name] + + # Reset counter + pre_fork_counter = 0 + + def pre_tag(obj): + nonlocal pre_fork_counter + pre_fork_counter += 1 + return obj + + def post_tag(obj): + return obj + + yield "post_fork", "meta", fork_meta.post_fork_name + + has_fork_epoch = False + if fork_meta.fork_epoch: + kw["fork_epoch"] = fork_meta.fork_epoch + has_fork_epoch = True + yield "fork_epoch", "meta", fork_meta.fork_epoch + + result = fn( + *args, + post_spec=post_spec, + pre_tag=pre_tag, + post_tag=post_tag, + **kw, + ) + if result is not None: + for part in result: + if part[0] == "fork_epoch": + has_fork_epoch = True + yield part + assert has_fork_epoch + + if pre_fork_counter > 0: + yield "fork_block", "meta", pre_fork_counter - 1 + + return wrapper + return decorator diff --git a/tests/core/pyspec/eth2spec/test/helpers/constants.py b/tests/core/pyspec/eth2spec/test/helpers/constants.py index 5ab8473279..4b2f37b66f 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/constants.py +++ b/tests/core/pyspec/eth2spec/test/helpers/constants.py @@ -21,6 +21,11 @@ FORKS_BEFORE_ALTAIR = (PHASE0,) FORKS_BEFORE_MERGE = (PHASE0, ALTAIR) +ALL_FORKS = { + # pre_fork_name: post_fork_name + PHASE0: ALTAIR, + ALTAIR: MERGE, +} # # Config diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py index 310c4a8d2a..853863e511 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_transition.py @@ -9,6 +9,10 @@ build_empty_block, sign_block, ) +from eth2spec.test.helpers.constants import ( + ALTAIR, + MERGE, +) from eth2spec.test.helpers.deposits import ( prepare_state_and_deposit, ) @@ -133,17 +137,25 @@ def state_transition_across_slots_with_ignoring_proposers(spec, next_slot(spec, state) -def do_altair_fork(state, spec, post_spec, fork_epoch, with_block=True, operation_dict=None): +def do_fork(state, spec, post_spec, fork_epoch, with_block=True, 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 - state = post_spec.upgrade_to_altair(state) + if post_spec.fork == ALTAIR: + state = post_spec.upgrade_to_altair(state) + elif post_spec.fork == MERGE: + state = post_spec.upgrade_to_merge(state) assert state.fork.epoch == fork_epoch - assert state.fork.previous_version == post_spec.config.GENESIS_FORK_VERSION - assert state.fork.current_version == post_spec.config.ALTAIR_FORK_VERSION + + 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 == MERGE: + assert state.fork.previous_version == post_spec.config.ALTAIR_FORK_VERSION + assert state.fork.current_version == post_spec.config.MERGE_FORK_VERSION if with_block: return state, _state_transition_and_sign_block_at_slot(post_spec, state, operation_dict=operation_dict) @@ -280,7 +292,7 @@ def _check_state(): # irregular state transition to handle fork: _operation_at_slot = operation_dict if is_at_fork else None - state, block = do_altair_fork(state, spec, post_spec, fork_epoch, operation_dict=_operation_at_slot) + state, block = do_fork(state, spec, post_spec, fork_epoch, operation_dict=_operation_at_slot) blocks.append(post_tag(block)) if is_at_fork: diff --git a/tests/core/pyspec/eth2spec/test/utils/__init__.py b/tests/core/pyspec/eth2spec/test/utils/__init__.py index f6b2a8a44b..abd79f9ede 100644 --- a/tests/core/pyspec/eth2spec/test/utils/__init__.py +++ b/tests/core/pyspec/eth2spec/test/utils/__init__.py @@ -1,12 +1,10 @@ from .utils import ( vector_test, with_meta_tags, - build_transition_test, ) __all__ = [ # avoid "unused import" lint error "vector_test", "with_meta_tags", - "build_transition_test", ] diff --git a/tests/core/pyspec/eth2spec/test/utils/utils.py b/tests/core/pyspec/eth2spec/test/utils/utils.py index 61fc75040e..bad6c867bd 100644 --- a/tests/core/pyspec/eth2spec/test/utils/utils.py +++ b/tests/core/pyspec/eth2spec/test/utils/utils.py @@ -1,4 +1,3 @@ -import inspect from typing import Dict, Any from eth2spec.utils.ssz.ssz_typing import View from eth2spec.utils.ssz.ssz_impl import serialize @@ -94,50 +93,3 @@ def entry(*args, **kw): yield k, 'meta', v return entry return runner - - -def build_transition_test(fn, pre_fork_name, post_fork_name, fork_epoch=None): - """ - Handles the inner plumbing to generate `transition_test`s. - See that decorator in `context.py` for more information. - """ - def _adapter(*args, **kwargs): - post_spec = kwargs["phases"][post_fork_name] - - pre_fork_counter = 0 - - def pre_tag(obj): - nonlocal pre_fork_counter - pre_fork_counter += 1 - return obj - - def post_tag(obj): - return obj - - yield "post_fork", "meta", post_fork_name - - has_fork_epoch = False - if fork_epoch: - kwargs["fork_epoch"] = fork_epoch - has_fork_epoch = True - yield "fork_epoch", "meta", fork_epoch - - # massage args to handle an optional custom state using - # `with_custom_state` decorator - expected_args = inspect.getfullargspec(fn) - if "phases" not in expected_args.kwonlyargs: - kwargs.pop("phases", None) - - for part in fn(*args, - post_spec=post_spec, - pre_tag=pre_tag, - post_tag=post_tag, - **kwargs): - if part[0] == "fork_epoch": - has_fork_epoch = True - yield part - assert has_fork_epoch - - if pre_fork_counter > 0: - yield "fork_block", "meta", pre_fork_counter - 1 - return _adapter diff --git a/tests/generators/transition/main.py b/tests/generators/transition/main.py index a850a7f450..80aef8fecf 100644 --- a/tests/generators/transition/main.py +++ b/tests/generators/transition/main.py @@ -1,6 +1,14 @@ from typing import Iterable -from eth2spec.test.helpers.constants import ALTAIR, MINIMAL, MAINNET, PHASE0 +from eth2spec.test.helpers.constants import ( + MINIMAL, + MAINNET, + ALL_FORKS, +) +from eth2spec.gen_helpers.gen_base import gen_runner, gen_typing +from eth2spec.gen_helpers.gen_from_tests.gen import ( + generate_from_tests, +) from eth2spec.test.altair.transition import ( test_transition as test_altair_transition, test_activations_and_exits as test_altair_activations_and_exits, @@ -9,9 +17,6 @@ test_operations as test_altair_operations, ) -from eth2spec.gen_helpers.gen_base import gen_runner, gen_typing -from eth2spec.gen_helpers.gen_from_tests.gen import generate_from_tests - def create_provider(tests_src, preset_name: str, pre_fork_name: str, post_fork_name: str) -> gen_typing.TestProvider: @@ -31,18 +36,17 @@ def cases_fn() -> Iterable[gen_typing.TestCase]: return gen_typing.TestProvider(prepare=prepare_fn, make_cases=cases_fn) -TRANSITION_TESTS = ( - (PHASE0, ALTAIR, test_altair_transition), - (PHASE0, ALTAIR, test_altair_activations_and_exits), - (PHASE0, ALTAIR, test_altair_leaking), - (PHASE0, ALTAIR, test_altair_slashing), - (PHASE0, ALTAIR, test_altair_operations), -) - - if __name__ == "__main__": - for pre_fork, post_fork, transition_test_module in TRANSITION_TESTS: - gen_runner.run_generator("transition", [ - create_provider(transition_test_module, MINIMAL, pre_fork, post_fork), - create_provider(transition_test_module, MAINNET, pre_fork, post_fork), - ]) + altair_tests = ( + test_altair_transition, + test_altair_activations_and_exits, + test_altair_leaking, + test_altair_slashing, + test_altair_operations, + ) + for transition_test_module in altair_tests: + for pre_fork, post_fork in ALL_FORKS.items(): + gen_runner.run_generator("transition", [ + create_provider(transition_test_module, MINIMAL, pre_fork, post_fork), + create_provider(transition_test_module, MAINNET, pre_fork, post_fork), + ]) From fa4dc0c1681b4068f6b180374ffff4abbe8ad277 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 2 Nov 2021 18:38:24 +0800 Subject: [PATCH 02/48] Apply suggestions from code review Co-authored-by: Alex Stokes --- tests/core/pyspec/eth2spec/test/context.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 8dd7c88d5f..3001946aa9 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -378,7 +378,7 @@ def _get_run_phases(phases, kw): return None run_phases = [phase] else: - # If pytest `--fork` flag is set, filter out the rest forks + # If pytest `--fork` flag is set, filter out the rest of the forks run_phases = set(phases).intersection(DEFAULT_PYTEST_FORKS) return run_phases @@ -386,7 +386,7 @@ def _get_run_phases(phases, kw): def _get_available_phases(run_phases, other_phases): """ - The return the available fork names for multi-phase tests + Return the available fork names for multi-phase tests """ available_phases = set(run_phases) if other_phases is not None: @@ -581,7 +581,6 @@ def decorator(fn): def wrapper(*args, **kw): phases = kw.pop('phases') spec = kw["spec"] - fork_meta = [m for m in fork_metas if m.pre_fork_name == spec.fork][0] fork_meta = next(filter(lambda m: m.pre_fork_name == spec.fork, fork_metas)) post_spec = phases[fork_meta.post_fork_name] From 0641d1c1840f1b374ae4d1383ba720fb1879d99c Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 2 Nov 2021 19:01:32 +0800 Subject: [PATCH 03/48] `ALL_FORKS` sounds like a list of fork names. Rename it to `ALL_FORK_UPGRADES` --- .../transition/test_activations_and_exits.py | 10 +++++----- .../test/altair/transition/test_leaking.py | 6 +++--- .../test/altair/transition/test_operations.py | 18 +++++++++--------- .../test/altair/transition/test_slashing.py | 4 ++-- .../test/altair/transition/test_transition.py | 18 +++++++++--------- tests/core/pyspec/eth2spec/test/context.py | 4 ++-- .../pyspec/eth2spec/test/helpers/constants.py | 4 +++- tests/generators/transition/main.py | 4 ++-- 8 files changed, 35 insertions(+), 33 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py index 0f5b28db67..e698c56bab 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py @@ -5,7 +5,7 @@ with_fork_metas, ) from eth2spec.test.helpers.constants import ( - ALL_FORKS, + ALL_PRE_POST_FORKS, MINIMAL, ) from eth2spec.test.helpers.fork_transition import ( @@ -24,7 +24,7 @@ # Exit # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) @with_presets([MINIMAL], reason="only test with enough validators such that at least one exited index is not in sync committee") def test_transition_with_one_fourth_exiting_validators_exit_post_fork(state, @@ -84,7 +84,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_post_fork(state, yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, fork_epoch, spec, @@ -142,7 +142,7 @@ def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_non_empty_activation_queue(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create some deposits before the transition @@ -170,7 +170,7 @@ def test_transition_with_non_empty_activation_queue(state, fork_epoch, spec, pos yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_activation_at_fork_epoch(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create some deposits before the transition diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py index 3ed2e37ee2..338289597b 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_leaking.py @@ -3,7 +3,7 @@ with_fork_metas, ) from eth2spec.test.helpers.constants import ( - ALL_FORKS, + ALL_PRE_POST_FORKS, ) from eth2spec.test.helpers.fork_transition import ( do_fork, @@ -12,7 +12,7 @@ ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=7) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=7) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_leaking_pre_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Leaking starts at epoch 6 (MIN_EPOCHS_TO_INACTIVITY_PENALTY + 2). @@ -40,7 +40,7 @@ def test_transition_with_leaking_pre_fork(state, fork_epoch, spec, post_spec, pr yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=6) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=6) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_leaking_at_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Leaking starts at epoch 6 (MIN_EPOCHS_TO_INACTIVITY_PENALTY + 2). diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py index 54f37a2350..1210531860 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_operations.py @@ -5,7 +5,7 @@ with_presets, ) from eth2spec.test.helpers.constants import ( - ALL_FORKS, + ALL_PRE_POST_FORKS, MINIMAL, ) from eth2spec.test.helpers.fork_transition import ( @@ -18,7 +18,7 @@ # PROPOSER_SLASHING # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) @always_bls def test_transition_with_proposer_slashing_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -36,7 +36,7 @@ def test_transition_with_proposer_slashing_right_after_fork(state, fork_epoch, s ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) @always_bls def test_transition_with_proposer_slashing_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -59,7 +59,7 @@ def test_transition_with_proposer_slashing_right_before_fork(state, fork_epoch, # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) @always_bls def test_transition_with_attester_slashing_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -77,7 +77,7 @@ def test_transition_with_attester_slashing_right_after_fork(state, fork_epoch, s ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) @always_bls def test_transition_with_attester_slashing_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -100,7 +100,7 @@ def test_transition_with_attester_slashing_right_before_fork(state, fork_epoch, # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_deposit_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a deposit right *after* the transition @@ -117,7 +117,7 @@ def test_transition_with_deposit_right_after_fork(state, fork_epoch, spec, post_ ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_deposit_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Create a deposit right *before* the transition @@ -139,7 +139,7 @@ def test_transition_with_deposit_right_before_fork(state, fork_epoch, spec, post # -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_PRE_POST_FORKS]) @with_presets([MINIMAL], reason="too slow") def test_transition_with_voluntary_exit_right_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ @@ -161,7 +161,7 @@ def test_transition_with_voluntary_exit_right_after_fork(state, fork_epoch, spec ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=66) for pre, post in ALL_PRE_POST_FORKS]) @with_presets([MINIMAL], reason="too slow") def test_transition_with_voluntary_exit_right_before_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py index 6d8f23b7dd..b3ba923215 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_slashing.py @@ -5,7 +5,7 @@ with_presets, ) from eth2spec.test.helpers.constants import ( - ALL_FORKS, + ALL_PRE_POST_FORKS, MINIMAL, ) from eth2spec.test.helpers.fork_transition import ( @@ -18,7 +18,7 @@ ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=1) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=1) for pre, post in ALL_PRE_POST_FORKS]) @with_presets([MINIMAL], reason="only test with enough validators such that at least one exited index is not in sync committee") def test_transition_with_one_fourth_slashed_active_validators_pre_fork(state, diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py index 2439b79fcd..2ac8bfb485 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_transition.py @@ -4,7 +4,7 @@ with_fork_metas, ) from eth2spec.test.helpers.constants import ( - ALL_FORKS, + ALL_PRE_POST_FORKS, ) from eth2spec.test.helpers.state import ( next_epoch_via_signed_block, @@ -20,7 +20,7 @@ ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_normal_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -56,7 +56,7 @@ def test_normal_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_missing_first_post_block(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -93,7 +93,7 @@ def test_transition_missing_first_post_block(state, fork_epoch, spec, post_spec, yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_missing_last_pre_fork_block(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -132,7 +132,7 @@ def test_transition_missing_last_pre_fork_block(state, fork_epoch, spec, post_sp yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_only_blocks_post_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -258,7 +258,7 @@ def _run_transition_test_with_attestations(state, yield "post", state -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_finality(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -267,7 +267,7 @@ def test_transition_with_finality(state, fork_epoch, spec, post_spec, pre_tag, p yield from _run_transition_test_with_attestations(state, fork_epoch, spec, post_spec, pre_tag, post_tag) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_random_three_quarters_participation(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the epoch after the ``fork_epoch``, @@ -294,7 +294,7 @@ def _drop_random_quarter(_slot, _index, indices): ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_random_half_participation(state, fork_epoch, spec, post_spec, pre_tag, post_tag): rng = random.Random(2020) @@ -318,7 +318,7 @@ def _drop_random_half(_slot, _index, indices): ) -@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_FORKS.items()]) +@with_fork_metas([ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=3) for pre, post in ALL_PRE_POST_FORKS]) def test_transition_with_no_attestations_until_after_fork(state, fork_epoch, spec, post_spec, pre_tag, post_tag): """ Transition from the initial ``state`` to the ``fork_epoch`` with no attestations, diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 3001946aa9..04da8e6531 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -11,7 +11,7 @@ from .helpers.constants import ( PHASE0, ALTAIR, MERGE, MINIMAL, MAINNET, ALL_PHASES, FORKS_BEFORE_ALTAIR, FORKS_BEFORE_MERGE, - ALL_FORKS, + ALL_FORK_UPGRADES, ) from .helpers.typing import SpecForkName, PresetBaseName from .helpers.genesis import create_genesis_state @@ -432,7 +432,7 @@ def wrapper(*args, **kw): # When running test generator, it sets specific `phase` phase = kw['phase'] _phases = [phase] - _other_phases = [ALL_FORKS[phase]] + _other_phases = [ALL_FORK_UPGRADES[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` diff --git a/tests/core/pyspec/eth2spec/test/helpers/constants.py b/tests/core/pyspec/eth2spec/test/helpers/constants.py index 4b2f37b66f..4f0f8bd6d8 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/constants.py +++ b/tests/core/pyspec/eth2spec/test/helpers/constants.py @@ -21,11 +21,13 @@ FORKS_BEFORE_ALTAIR = (PHASE0,) FORKS_BEFORE_MERGE = (PHASE0, ALTAIR) -ALL_FORKS = { +ALL_FORK_UPGRADES = { # pre_fork_name: post_fork_name PHASE0: ALTAIR, ALTAIR: MERGE, } +ALL_PRE_POST_FORKS = ALL_FORK_UPGRADES.items() + # # Config diff --git a/tests/generators/transition/main.py b/tests/generators/transition/main.py index 80aef8fecf..de00e774a0 100644 --- a/tests/generators/transition/main.py +++ b/tests/generators/transition/main.py @@ -3,7 +3,7 @@ from eth2spec.test.helpers.constants import ( MINIMAL, MAINNET, - ALL_FORKS, + ALL_FORK_UPGRADES, ) from eth2spec.gen_helpers.gen_base import gen_runner, gen_typing from eth2spec.gen_helpers.gen_from_tests.gen import ( @@ -45,7 +45,7 @@ def cases_fn() -> Iterable[gen_typing.TestCase]: test_altair_operations, ) for transition_test_module in altair_tests: - for pre_fork, post_fork in ALL_FORKS.items(): + for pre_fork, post_fork in ALL_PRE_POST_FORKS: gen_runner.run_generator("transition", [ create_provider(transition_test_module, MINIMAL, pre_fork, post_fork), create_provider(transition_test_module, MAINNET, pre_fork, post_fork), From 6cfd38e8007e7dadb68c6c2ebcd8b2630964bc26 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 11 Nov 2021 11:47:01 +1100 Subject: [PATCH 04/48] Patch for genesis blocks --- specs/merge/validator.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index d36397353e..6ca7c9c12a 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -44,8 +44,10 @@ Please see related Beacon Chain doc before continuing and use them as a referenc def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock]) -> Optional[PowBlock]: # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain: - parent = pow_chain[block.parent_hash] block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + if block_reached_ttd and block.parent_hash == Hash32(): + return block + parent = pow_chain[block.parent_hash] parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY if block_reached_ttd and not parent_reached_ttd: return block From 61207c5b6155243c3dec6529ecc214d5e23cfb58 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 11 Nov 2021 07:29:38 -0700 Subject: [PATCH 05/48] Apply suggestions from code review --- specs/merge/validator.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 6ca7c9c12a..22fc3eb137 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -45,6 +45,7 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock] # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain: block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + # If genesis block, no parent exists so reaching TTD alone qualifies as valid terminal block if block_reached_ttd and block.parent_hash == Hash32(): return block parent = pow_chain[block.parent_hash] From 49d96f92ef6f6750e55b0cf089d173de5a643852 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Fri, 12 Nov 2021 17:06:10 +0800 Subject: [PATCH 06/48] Add a sample altair-to-merge-only transition --- tests/core/pyspec/eth2spec/test/context.py | 6 +++- .../pyspec/eth2spec/test/helpers/constants.py | 3 +- .../test/merge/transition/__init__.py | 0 .../test/merge/transition/test_transition.py | 35 +++++++++++++++++++ tests/generators/transition/main.py | 11 ++++-- 5 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/merge/transition/__init__.py create mode 100644 tests/core/pyspec/eth2spec/test/merge/transition/test_transition.py diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 04da8e6531..7f7c6bf28d 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -581,7 +581,11 @@ def decorator(fn): def wrapper(*args, **kw): phases = kw.pop('phases') spec = kw["spec"] - fork_meta = next(filter(lambda m: m.pre_fork_name == spec.fork, fork_metas)) + try: + fork_meta = next(filter(lambda m: m.pre_fork_name == spec.fork, fork_metas)) + except StopIteration: + dump_skipping_message(f"doesn't support this fork: {spec.fork}") + post_spec = phases[fork_meta.post_fork_name] # Reset counter diff --git a/tests/core/pyspec/eth2spec/test/helpers/constants.py b/tests/core/pyspec/eth2spec/test/helpers/constants.py index 4f0f8bd6d8..bb8f49cbc9 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/constants.py +++ b/tests/core/pyspec/eth2spec/test/helpers/constants.py @@ -27,7 +27,8 @@ ALTAIR: MERGE, } ALL_PRE_POST_FORKS = ALL_FORK_UPGRADES.items() - +AFTER_MERGE_UPGRADES = {key: value for key, value in ALL_FORK_UPGRADES.items() if key not in FORKS_BEFORE_ALTAIR} +AFTER_MERGE_PRE_POST_FORKS = AFTER_MERGE_UPGRADES.items() # # Config diff --git a/tests/core/pyspec/eth2spec/test/merge/transition/__init__.py b/tests/core/pyspec/eth2spec/test/merge/transition/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/core/pyspec/eth2spec/test/merge/transition/test_transition.py b/tests/core/pyspec/eth2spec/test/merge/transition/test_transition.py new file mode 100644 index 0000000000..d488d81dc2 --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/merge/transition/test_transition.py @@ -0,0 +1,35 @@ +from eth2spec.test.context import ( + ForkMeta, + with_fork_metas, +) +from eth2spec.test.helpers.constants import ( + AFTER_MERGE_PRE_POST_FORKS, +) +from eth2spec.test.helpers.fork_transition import ( + do_fork, + transition_to_next_epoch_and_append_blocks, + transition_until_fork, +) + + +@with_fork_metas([ + ForkMeta(pre_fork_name=pre, post_fork_name=post, fork_epoch=2) for pre, post in AFTER_MERGE_PRE_POST_FORKS +]) +def test_sample_transition(state, fork_epoch, spec, post_spec, pre_tag, post_tag): + transition_until_fork(spec, state, fork_epoch) + + # check pre state + assert spec.get_current_epoch(state) < fork_epoch + + yield "pre", state + + # irregular state transition to handle fork: + blocks = [] + state, block = do_fork(state, spec, post_spec, fork_epoch) + blocks.append(post_tag(block)) + + # continue regular state transition with new spec into next epoch + transition_to_next_epoch_and_append_blocks(post_spec, state, post_tag, blocks, only_last_block=True) + + yield "blocks", blocks + yield "post", state diff --git a/tests/generators/transition/main.py b/tests/generators/transition/main.py index de00e774a0..d6195de689 100644 --- a/tests/generators/transition/main.py +++ b/tests/generators/transition/main.py @@ -3,7 +3,7 @@ from eth2spec.test.helpers.constants import ( MINIMAL, MAINNET, - ALL_FORK_UPGRADES, + ALL_PRE_POST_FORKS, ) from eth2spec.gen_helpers.gen_base import gen_runner, gen_typing from eth2spec.gen_helpers.gen_from_tests.gen import ( @@ -16,6 +16,9 @@ test_slashing as test_altair_slashing, test_operations as test_altair_operations, ) +from eth2spec.test.merge.transition import ( + test_transition as test_merge_transition, +) def create_provider(tests_src, preset_name: str, pre_fork_name: str, post_fork_name: str) -> gen_typing.TestProvider: @@ -44,7 +47,11 @@ def cases_fn() -> Iterable[gen_typing.TestCase]: test_altair_slashing, test_altair_operations, ) - for transition_test_module in altair_tests: + merge_tests = ( + test_merge_transition, + ) + all_tests = altair_tests + merge_tests + for transition_test_module in all_tests: for pre_fork, post_fork in ALL_PRE_POST_FORKS: gen_runner.run_generator("transition", [ create_provider(transition_test_module, MINIMAL, pre_fork, post_fork), From cd3d2ce69219b80d92eec0dde8d3dec7b1be6a3c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 12 Nov 2021 12:36:02 -0700 Subject: [PATCH 07/48] working through test issues --- specs/phase0/fork-choice.md | 14 +------------- .../pyspec/eth2spec/test/helpers/fork_choice.py | 2 ++ .../test/phase0/fork_choice/test_on_block.py | 7 +++++++ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 276aa8029a..e8f6eacf1f 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -372,19 +372,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Update finalized checkpoint if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: store.finalized_checkpoint = state.finalized_checkpoint - - # Potentially update justified if different from store - if store.justified_checkpoint != state.current_justified_checkpoint: - # Update justified if new justified is later than store justified - if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: - store.justified_checkpoint = state.current_justified_checkpoint - return - - # Update justified if store justified is not in chain with finalized checkpoint - finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) - ancestor_at_finalized_slot = get_ancestor(store, store.justified_checkpoint.root, finalized_slot) - if ancestor_at_finalized_slot != store.finalized_checkpoint.root: - store.justified_checkpoint = state.current_justified_checkpoint + store.justified_checkpoint = state.current_justified_checkpoint ``` #### `on_attestation` diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 7c6ac89a54..ae088fccda 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -166,6 +166,7 @@ def add_block(spec, raise block_root = signed_block.message.hash_tree_root() + print(encode_hex(block_root)) assert store.blocks[block_root] == signed_block.message assert store.block_states[block_root].hash_tree_root() == signed_block.message.state_root test_steps.append({ @@ -186,6 +187,7 @@ def add_block(spec, }, } }) + print(test_steps[-1]) return store.block_states[signed_block.message.hash_tree_root()] diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 79bf353b17..f90d1acbc9 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -658,6 +658,7 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): """ test_steps = [] # Initialization + print("INIT") store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) yield 'anchor_state', state yield 'anchor_block', anchor_block @@ -679,12 +680,14 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, False, True, test_steps=test_steps) + print("INIT FIN") assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 4 assert store.justified_checkpoint == state.current_justified_checkpoint # Create another chain # Forking from epoch 3 + print("FORK") all_blocks = [] slot = spec.compute_start_slot_at_epoch(3) block_root = spec.get_block_root_at_slot(state, slot) @@ -696,6 +699,7 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert another_state.finalized_checkpoint.epoch == 3 assert another_state.current_justified_checkpoint.epoch == 4 + print("APPLY FORK TO FORK CHOICE") pre_store_justified_checkpoint_root = store.justified_checkpoint.root for block in all_blocks: # FIXME: Once `on_block` and `on_attestation` logic is fixed, @@ -707,6 +711,9 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert ancestor_at_finalized_slot == store.finalized_checkpoint.root assert store.finalized_checkpoint == another_state.finalized_checkpoint + # Thus should fail with the fix. Once show fail, swap to == assert store.justified_checkpoint != another_state.current_justified_checkpoint + print(store.finalized_checkpoint) + print(store.justified_checkpoint) yield 'steps', test_steps From f643554aa519ce6efe015ddc97d564e90c0fe248 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sat, 13 Nov 2021 18:23:21 +0800 Subject: [PATCH 08/48] Fix issue around on_attestation validation by skipping epoch scope validation if attestation is from a block message --- specs/phase0/fork-choice.md | 21 +++++++-- .../eth2spec/test/helpers/fork_choice.py | 43 ++++--------------- .../test/phase0/fork_choice/test_on_block.py | 35 ++++++++------- 3 files changed, 43 insertions(+), 56 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index e8f6eacf1f..ed572a96e7 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -22,6 +22,7 @@ - [`get_head`](#get_head) - [`should_update_justified_checkpoint`](#should_update_justified_checkpoint) - [`on_attestation` helpers](#on_attestation-helpers) + - [`validate_target_epoch_scope`](#validate_target_epoch_scope) - [`validate_on_attestation`](#validate_on_attestation) - [`store_target_checkpoint_state`](#store_target_checkpoint_state) - [`update_latest_messages`](#update_latest_messages) @@ -257,10 +258,11 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C #### `on_attestation` helpers -##### `validate_on_attestation` + +##### `validate_target_epoch_scope` ```python -def validate_on_attestation(store: Store, attestation: Attestation) -> None: +def validate_target_epoch_scope(store: Store, attestation: Attestation) -> None: target = attestation.data.target # Attestations must be from the current or previous epoch @@ -269,6 +271,15 @@ def validate_on_attestation(store: Store, attestation: Attestation) -> None: previous_epoch = current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH # If attestation target is from a future epoch, delay consideration until the epoch arrives assert target.epoch in [current_epoch, previous_epoch] +``` + +##### `validate_on_attestation` + +```python +def validate_on_attestation(store: Store, attestation: Attestation) -> None: + target = attestation.data.target + + # Check that the epoch number and slot number are matching assert target.epoch == compute_epoch_at_slot(attestation.data.slot) # Attestations target be for a known block. If target block is unknown, delay consideration until the block is found @@ -378,7 +389,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: #### `on_attestation` ```python -def on_attestation(store: Store, attestation: Attestation) -> None: +def on_attestation(store: Store, attestation: Attestation, is_from_block: bool=False) -> None: """ Run ``on_attestation`` upon receiving a new ``attestation`` from either within a block or directly on the wire. @@ -386,6 +397,10 @@ def on_attestation(store: Store, attestation: Attestation) -> None: consider scheduling it for later processing in such case. """ validate_on_attestation(store, attestation) + if not is_from_block: + # If the given attestation is not from a beacon block message, we have to check the target epoch scope. + validate_target_epoch_scope(store, attestation) + store_target_checkpoint_state(store, attestation.data.target) # Get state at the `target` to fully validate attestation diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index ae088fccda..1c49c102ce 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -23,7 +23,7 @@ def add_block_to_store(spec, store, signed_block): spec.on_block(store, signed_block) -def tick_and_add_block(spec, store, signed_block, test_steps, valid=True, allow_invalid_attestations=False, +def tick_and_add_block(spec, store, signed_block, test_steps, valid=True, merge_block=False, block_not_found=False): pre_state = store.block_states[signed_block.message.parent_root] block_time = pre_state.genesis_time + signed_block.message.slot * spec.config.SECONDS_PER_SLOT @@ -36,14 +36,13 @@ def tick_and_add_block(spec, store, signed_block, test_steps, valid=True, allow_ post_state = yield from add_block( spec, store, signed_block, test_steps, valid=valid, - allow_invalid_attestations=allow_invalid_attestations, block_not_found=block_not_found, ) return post_state -def tick_and_run_on_attestation(spec, store, attestation, test_steps): +def tick_and_run_on_attestation(spec, store, attestation, test_steps, is_from_block=False): parent_block = store.blocks[attestation.data.beacon_block_root] pre_state = store.block_states[spec.hash_tree_root(parent_block)] block_time = pre_state.genesis_time + parent_block.slot * spec.config.SECONDS_PER_SLOT @@ -53,40 +52,21 @@ def tick_and_run_on_attestation(spec, store, attestation, test_steps): spec.on_tick(store, next_epoch_time) test_steps.append({'tick': int(next_epoch_time)}) - spec.on_attestation(store, attestation) + spec.on_attestation(store, attestation, is_from_block=is_from_block) yield get_attestation_file_name(attestation), attestation test_steps.append({'attestation': get_attestation_file_name(attestation)}) -def add_attestation(spec, store, attestation, test_steps, valid=True): - yield get_attestation_file_name(attestation), attestation - - if not valid: - try: - run_on_attestation(spec, store, attestation, valid=True) - except AssertionError: - test_steps.append({ - 'attestation': get_attestation_file_name(attestation), - 'valid': False, - }) - return - else: - assert False - - run_on_attestation(spec, store, attestation, valid=True) - test_steps.append({'attestation': get_attestation_file_name(attestation)}) - - -def run_on_attestation(spec, store, attestation, valid=True): +def run_on_attestation(spec, store, attestation, is_from_block=False, valid=True): if not valid: try: - spec.on_attestation(store, attestation) + spec.on_attestation(store, attestation, is_from_block) except AssertionError: return else: assert False - spec.on_attestation(store, attestation) + spec.on_attestation(store, attestation, is_from_block) def get_genesis_forkchoice_store(spec, genesis_state): @@ -131,7 +111,6 @@ def add_block(spec, signed_block, test_steps, valid=True, - allow_invalid_attestations=False, block_not_found=False): """ Run on_block and on_attestation @@ -156,14 +135,8 @@ def add_block(spec, test_steps.append({'block': get_block_file_name(signed_block)}) # An on_block step implies receiving block's attestations - try: - for attestation in signed_block.message.body.attestations: - run_on_attestation(spec, store, attestation, valid=True) - except AssertionError: - if allow_invalid_attestations: - pass - else: - raise + for attestation in signed_block.message.body.attestations: + run_on_attestation(spec, store, attestation, is_from_block=True, valid=True) block_root = signed_block.message.hash_tree_root() print(encode_hex(block_root)) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index f90d1acbc9..1f2f453522 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -543,21 +543,19 @@ def test_new_justified_is_later_than_store_justified(spec, state): assert fork_3_state.finalized_checkpoint.epoch == 3 assert fork_3_state.current_justified_checkpoint.epoch == 4 - # FIXME: pending on the `on_block`, `on_attestation` fix - # # Apply blocks of `fork_3_state` to `store` - # for block in all_blocks: - # if store.time < spec.compute_time_at_slot(fork_2_state, block.message.slot): - # time = store.genesis_time + block.message.slot * spec.config.SECONDS_PER_SLOT - # on_tick_and_append_step(spec, store, time, test_steps) - # # valid_attestations=False because the attestations are outdated (older than previous epoch) - # yield from add_block(spec, store, block, test_steps, allow_invalid_attestations=False) - - # assert store.finalized_checkpoint == fork_3_state.finalized_checkpoint - # assert (store.justified_checkpoint - # == fork_3_state.current_justified_checkpoint - # != store.best_justified_checkpoint) - # assert (store.best_justified_checkpoint - # == fork_2_state.current_justified_checkpoint) + # Apply blocks of `fork_3_state` to `store` + for block in all_blocks: + if store.time < spec.compute_time_at_slot(fork_2_state, block.message.slot): + time = store.genesis_time + block.message.slot * spec.config.SECONDS_PER_SLOT + on_tick_and_append_step(spec, store, time, test_steps) + yield from add_block(spec, store, block, test_steps) + + assert store.finalized_checkpoint == fork_3_state.finalized_checkpoint + assert (store.justified_checkpoint + == fork_3_state.current_justified_checkpoint + != store.best_justified_checkpoint) + assert (store.best_justified_checkpoint + == fork_2_state.current_justified_checkpoint) yield 'steps', test_steps @@ -628,7 +626,7 @@ def test_new_finalized_slot_is_not_justified_checkpoint_ancestor(spec, state): # # Apply blocks of `another_state` to `store` # for block in all_blocks: # # NOTE: Do not call `on_tick` here - # yield from add_block(spec, store, block, test_steps, allow_invalid_attestations=True) + # yield from add_block(spec, store, block, test_steps) # finalized_slot = spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) # ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) @@ -704,7 +702,7 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): for block in all_blocks: # FIXME: Once `on_block` and `on_attestation` logic is fixed, # fix test case and remove allow_invalid_attestations flag - yield from tick_and_add_block(spec, store, block, test_steps, allow_invalid_attestations=True) + yield from tick_and_add_block(spec, store, block, test_steps) finalized_slot = spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) @@ -712,8 +710,9 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert store.finalized_checkpoint == another_state.finalized_checkpoint # Thus should fail with the fix. Once show fail, swap to == - assert store.justified_checkpoint != another_state.current_justified_checkpoint + # assert store.justified_checkpoint != another_state.current_justified_checkpoint print(store.finalized_checkpoint) print(store.justified_checkpoint) + print('spec.get_head(store)', spec.get_head(store)) yield 'steps', test_steps From d9e8306c5a587a40f8285184428023f845048f95 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sat, 13 Nov 2021 18:38:53 +0800 Subject: [PATCH 09/48] Refactoring --- specs/phase0/fork-choice.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index ed572a96e7..6331f496f3 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -276,9 +276,13 @@ def validate_target_epoch_scope(store: Store, attestation: Attestation) -> None: ##### `validate_on_attestation` ```python -def validate_on_attestation(store: Store, attestation: Attestation) -> None: +def validate_on_attestation(store: Store, attestation: Attestation, is_from_block: bool) -> None: target = attestation.data.target + # If the given attestation is not from a beacon block message, we have to check the target epoch scope. + if not is_from_block: + validate_target_epoch_scope(store, attestation) + # Check that the epoch number and slot number are matching assert target.epoch == compute_epoch_at_slot(attestation.data.slot) @@ -396,10 +400,7 @@ def on_attestation(store: Store, attestation: Attestation, is_from_block: bool=F An ``attestation`` that is asserted as invalid may be valid at a later time, consider scheduling it for later processing in such case. """ - validate_on_attestation(store, attestation) - if not is_from_block: - # If the given attestation is not from a beacon block message, we have to check the target epoch scope. - validate_target_epoch_scope(store, attestation) + validate_on_attestation(store, attestation, is_from_block) store_target_checkpoint_state(store, attestation.data.target) From f0bb032eb17ae441429405b89b7c158623f865c0 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Mon, 15 Nov 2021 23:46:40 +0800 Subject: [PATCH 10/48] Fix `test_transition_with_one_fourth_exiting_validators_exit_at_fork` assertions --- .../test/altair/transition/test_activations_and_exits.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py index e698c56bab..438e2c20c4 100644 --- a/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py +++ b/tests/core/pyspec/eth2spec/test/altair/transition/test_activations_and_exits.py @@ -1,6 +1,7 @@ import random from eth2spec.test.context import ( ForkMeta, + ALTAIR, with_presets, with_fork_metas, ) @@ -130,6 +131,14 @@ def test_transition_with_one_fourth_exiting_validators_exit_at_fork(state, assert not post_spec.is_active_validator(validator, post_spec.get_current_epoch(state)) assert not post_spec.is_in_inactivity_leak(state) + exited_pubkeys = [state.validators[index].pubkey for index in exited_indices] + some_sync_committee_exited = any(set(exited_pubkeys).intersection(list(state.current_sync_committee.pubkeys))) + if post_spec.fork == ALTAIR: + # in Altair fork, the sync committee members would be set with only active validators + assert not some_sync_committee_exited + else: + assert some_sync_committee_exited + # continue regular state transition with new spec into next epoch transition_to_next_epoch_and_append_blocks(post_spec, state, post_tag, blocks, only_last_block=True) From 63c9e5ea56b1daa725d2d9ee9a1e368adc9d64d1 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 Nov 2021 00:15:39 +0800 Subject: [PATCH 11/48] Minor refactoring and add comments --- tests/core/pyspec/eth2spec/test/context.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 7f7c6bf28d..184c0d6098 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -435,12 +435,10 @@ def wrapper(*args, **kw): _other_phases = [ALL_FORK_UPGRADES[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` + # When running pytest, go through `fork_metas` instead of using `phases` for fork_meta in fork_metas: - pre_fork_name = fork_meta.pre_fork_name - post_fork_name = fork_meta.post_fork_name - _phases = [pre_fork_name] - _other_phases = [post_fork_name] + _phases = [fork_meta.pre_fork_name] + _other_phases = [fork_meta.post_fork_name] ret = _run_test_case_with_phases(fn, _phases, _other_phases, kw, args, is_fork_transition=True) else: ret = _run_test_case_with_phases(fn, phases, other_phases, kw, args) From 16aaf74cfc368bc4a29d6b6e7a2b34f5dbba1da0 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 16 Nov 2021 10:28:04 +1100 Subject: [PATCH 12/48] Remove `difficulty` from `PowBlock` --- specs/merge/fork-choice.md | 1 - 1 file changed, 1 deletion(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 3f0c672001..dec2738539 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -87,7 +87,6 @@ class PowBlock(Container): block_hash: Hash32 parent_hash: Hash32 total_difficulty: uint256 - difficulty: uint256 ``` ### `get_pow_block` From 257173fc9ed4c672d8ec6a23885c304eb55e63ad Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 Nov 2021 12:35:57 +0800 Subject: [PATCH 13/48] Add The Merge spec to coverage report targets --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 547ed215f3..7810ff19e2 100644 --- a/Makefile +++ b/Makefile @@ -97,12 +97,12 @@ install_test: # Testing against `minimal` config by default test: pyspec . venv/bin/activate; cd $(PY_SPEC_DIR); \ - python3 -m pytest -n 4 --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec + python3 -m pytest -n 4 --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.merge.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec # Testing against `minimal` config by default find_test: pyspec . venv/bin/activate; cd $(PY_SPEC_DIR); \ - python3 -m pytest -k=$(K) --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec + python3 -m pytest -k=$(K) --disable-bls --cov=eth2spec.phase0.minimal --cov=eth2spec.altair.minimal --cov=eth2spec.merge.minimal --cov-report="html:$(COV_HTML_OUT)" --cov-branch eth2spec citest: pyspec mkdir -p tests/core/pyspec/test-reports/eth2spec; From 2b9692a22cecc0151d92c1385bf1929bfa9a6f5a Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 Nov 2021 21:56:55 +0800 Subject: [PATCH 14/48] Add a test to cover the case of `is_execution_enabled` result is false --- .../eth2spec/test/merge/sanity/test_blocks.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/merge/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/merge/sanity/test_blocks.py index 09cb547c25..1e8bb39a63 100644 --- a/tests/core/pyspec/eth2spec/test/merge/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/merge/sanity/test_blocks.py @@ -23,3 +23,22 @@ def test_empty_block_transition_no_tx(spec, state): yield 'post', state # TODO: tests with EVM, mock or replacement? + + +@with_merge_and_later +@spec_state_test +def test_is_execution_enabled_false(spec, state): + # Set `latest_execution_payload_header` to empty + state.latest_execution_payload_header = spec.ExecutionPayloadHeader() + yield 'pre', state + + block = build_empty_block_for_next_slot(spec, state) + + # Set `execution_payload` to empty + block.body.execution_payload = spec.ExecutionPayload() + assert len(block.body.execution_payload.transactions) == 0 + + signed_block = state_transition_and_sign_block(spec, state, block) + + yield 'blocks', [signed_block] + yield 'post', state From 79f1f16adf4fc7f1c621a8bbe1471dd06430ccb3 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 16 Nov 2021 22:08:00 +0800 Subject: [PATCH 15/48] Clean `PowBlock.difficulty` leftover --- setup.py | 2 +- tests/core/pyspec/eth2spec/test/helpers/pow_block.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 355f6e2c19..0ced87be2e 100644 --- a/setup.py +++ b/setup.py @@ -511,7 +511,7 @@ def sundry_functions(cls) -> str: def get_pow_block(hash: Bytes32) -> Optional[PowBlock]: - return PowBlock(block_hash=hash, parent_hash=Bytes32(), total_difficulty=uint256(0), difficulty=uint256(0)) + return PowBlock(block_hash=hash, parent_hash=Bytes32(), total_difficulty=uint256(0)) def get_execution_state(execution_state_root: Bytes32) -> ExecutionState: diff --git a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py index 58989a420b..939360e938 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py +++ b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py @@ -21,7 +21,6 @@ def prepare_random_pow_block(spec, rng=Random(3131)): block_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), parent_hash=spec.Hash32(spec.hash(bytearray(rng.getrandbits(8) for _ in range(32)))), total_difficulty=uint256(0), - difficulty=uint256(0) ) From e56bbb8c44a4b4bbe4bfdecde368f3fb9e2e28a1 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 17 Nov 2021 17:48:10 +0800 Subject: [PATCH 16/48] Fix `get_pow_block_at_terminal_total_difficulty` and add unit tests --- specs/merge/validator.md | 14 ++-- .../pyspec/eth2spec/test/helpers/pow_block.py | 6 ++ .../unittests/validator/test_validator.py | 64 +++++++++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 22fc3eb137..6b8fa2a01b 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -43,11 +43,17 @@ Please see related Beacon Chain doc before continuing and use them as a referenc ```python def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock]) -> Optional[PowBlock]: # `pow_chain` abstractly represents all blocks in the PoW chain - for block in pow_chain: + for block in pow_chain.values(): block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - # If genesis block, no parent exists so reaching TTD alone qualifies as valid terminal block - if block_reached_ttd and block.parent_hash == Hash32(): - return block + # Genesis block + if block.parent_hash == Hash32(): + # No parent exists, so reaching TTD alone qualifies as valid terminal block + if block_reached_ttd: + return block + # Continue to iterate the next block + if block.parent_hash not in pow_chain: + continue + parent = pow_chain[block.parent_hash] parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY if block_reached_ttd and not parent_reached_ttd: diff --git a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py index 58989a420b..aed138edfd 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/pow_block.py +++ b/tests/core/pyspec/eth2spec/test/helpers/pow_block.py @@ -15,6 +15,12 @@ def head(self, offset=0): assert offset <= 0 return self.blocks[offset - 1] + def to_dict(self): + return { + block.block_hash: block + for block in self.blocks + } + def prepare_random_pow_block(spec, rng=Random(3131)): return spec.PowBlock( diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py new file mode 100644 index 0000000000..9b710d2ead --- /dev/null +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py @@ -0,0 +1,64 @@ +from eth2spec.test.helpers.pow_block import ( + prepare_random_pow_chain, +) +from eth2spec.test.context import ( + spec_state_test, + with_merge_and_later, +) + + +# For test_get_pow_block_at_terminal_total_difficulty +IS_HEAD_BLOCK = 'is_head_block' +IS_HEAD_PARENT_BLOCK = 'is_head_parent_block' + +# NOTE: The following parameter names are in the view of the head block (the second block) +# 'block_reached_ttd', 'block_parent_hash_is_empty', 'parent_reached_ttd', 'return_block' +expected_results = [ + (False, False, False, None), + (False, False, True, IS_HEAD_PARENT_BLOCK), + (False, True, False, None), + (False, True, True, IS_HEAD_PARENT_BLOCK), + (True, False, False, IS_HEAD_BLOCK), + (True, False, True, IS_HEAD_PARENT_BLOCK), + (True, True, False, IS_HEAD_BLOCK), + (True, True, True, IS_HEAD_PARENT_BLOCK), +] +# NOTE: since the first block's `parent_hash` is set to `Hash32()` in test, if `parent_reached_ttd is True`, +# it would return the first block (IS_HEAD_PARENT_BLOCK). + + +@with_merge_and_later +@spec_state_test +def test_get_pow_block_at_terminal_total_difficulty(spec, state): + for result in expected_results: + ( + block_reached_ttd, + block_parent_hash_is_empty, + parent_reached_ttd, + return_block + ) = result + pow_chain = prepare_random_pow_chain(spec, 2) + pow_chain.head(-1).parent_hash = spec.Hash32() + + if block_reached_ttd: + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + else: + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - 1 + + if parent_reached_ttd: + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + else: + pow_chain.head(-1).total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - 1 + + if block_parent_hash_is_empty: + pow_chain.head().parent_hash = spec.Hash32() + + pow_block = spec.get_pow_block_at_terminal_total_difficulty(pow_chain.to_dict()) + if return_block == IS_HEAD_BLOCK: + assert pow_block == pow_chain.head() + elif return_block == IS_HEAD_PARENT_BLOCK: + assert pow_block == pow_chain.head(-1) + elif return_block is None: + assert pow_block is None + else: + raise Exception('Something is wrong') From 28762096d3b7dc6f2656d9e36d1d9ffa88aaf449 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 17 Nov 2021 23:45:41 +0800 Subject: [PATCH 17/48] PR feedback from @mkalinin --- specs/merge/validator.md | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 6b8fa2a01b..0d1344bfca 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -45,19 +45,15 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock] # `pow_chain` abstractly represents all blocks in the PoW chain for block in pow_chain.values(): block_reached_ttd = block.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - # Genesis block - if block.parent_hash == Hash32(): - # No parent exists, so reaching TTD alone qualifies as valid terminal block - if block_reached_ttd: + if block_reached_ttd: + # If genesis block, no parent exists so reaching TTD alone qualifies as valid terminal block + if block.parent_hash == Hash32(): return block - # Continue to iterate the next block - if block.parent_hash not in pow_chain: - continue - - parent = pow_chain[block.parent_hash] - parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - if block_reached_ttd and not parent_reached_ttd: - return block + else: + parent = pow_chain[block.parent_hash] + parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + if not parent_reached_ttd: + return block return None ``` From eb00f8f735a1d6e5d3fe614119d0cbcbceeeef77 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 17 Nov 2021 18:15:12 -0700 Subject: [PATCH 18/48] cleanup forkchoice tests --- specs/phase0/fork-choice.md | 11 ++++++---- .../eth2spec/test/helpers/fork_choice.py | 6 ++---- .../test/phase0/fork_choice/test_on_block.py | 20 ++++++++----------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 6331f496f3..723102e4df 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -22,7 +22,7 @@ - [`get_head`](#get_head) - [`should_update_justified_checkpoint`](#should_update_justified_checkpoint) - [`on_attestation` helpers](#on_attestation-helpers) - - [`validate_target_epoch_scope`](#validate_target_epoch_scope) + - [`validate_target_epoch_against_current_time`](#validate_target_epoch_against_current_time) - [`validate_on_attestation`](#validate_on_attestation) - [`store_target_checkpoint_state`](#store_target_checkpoint_state) - [`update_latest_messages`](#update_latest_messages) @@ -259,10 +259,10 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C #### `on_attestation` helpers -##### `validate_target_epoch_scope` +##### `validate_target_epoch_against_current_time` ```python -def validate_target_epoch_scope(store: Store, attestation: Attestation) -> None: +def validate_target_epoch_against_current_time(store: Store, attestation: Attestation) -> None: target = attestation.data.target # Attestations must be from the current or previous epoch @@ -281,7 +281,7 @@ def validate_on_attestation(store: Store, attestation: Attestation, is_from_bloc # If the given attestation is not from a beacon block message, we have to check the target epoch scope. if not is_from_block: - validate_target_epoch_scope(store, attestation) + validate_target_epoch_against_current_time(store, attestation) # Check that the epoch number and slot number are matching assert target.epoch == compute_epoch_at_slot(attestation.data.slot) @@ -386,6 +386,9 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Update finalized checkpoint if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: + print("made it") + print(state.finalized_checkpoint) + print(state.current_justified_checkpoint) store.finalized_checkpoint = state.finalized_checkpoint store.justified_checkpoint = state.current_justified_checkpoint ``` diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 1c49c102ce..098c05bf9d 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -60,13 +60,13 @@ def tick_and_run_on_attestation(spec, store, attestation, test_steps, is_from_bl def run_on_attestation(spec, store, attestation, is_from_block=False, valid=True): if not valid: try: - spec.on_attestation(store, attestation, is_from_block) + spec.on_attestation(store, attestation, is_from_block=is_from_block) except AssertionError: return else: assert False - spec.on_attestation(store, attestation, is_from_block) + spec.on_attestation(store, attestation, is_from_block=is_from_block) def get_genesis_forkchoice_store(spec, genesis_state): @@ -139,7 +139,6 @@ def add_block(spec, run_on_attestation(spec, store, attestation, is_from_block=True, valid=True) block_root = signed_block.message.hash_tree_root() - print(encode_hex(block_root)) assert store.blocks[block_root] == signed_block.message assert store.block_states[block_root].hash_tree_root() == signed_block.message.state_root test_steps.append({ @@ -160,7 +159,6 @@ def add_block(spec, }, } }) - print(test_steps[-1]) return store.block_states[signed_block.message.hash_tree_root()] diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 1f2f453522..3670a883af 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -551,11 +551,9 @@ def test_new_justified_is_later_than_store_justified(spec, state): yield from add_block(spec, store, block, test_steps) assert store.finalized_checkpoint == fork_3_state.finalized_checkpoint - assert (store.justified_checkpoint - == fork_3_state.current_justified_checkpoint - != store.best_justified_checkpoint) - assert (store.best_justified_checkpoint - == fork_2_state.current_justified_checkpoint) + assert store.justified_checkpoint == fork_3_state.current_justified_checkpoint + assert store.justified_checkpoint != store.best_justified_checkpoint + assert store.best_justified_checkpoint == fork_2_state.current_justified_checkpoint yield 'steps', test_steps @@ -656,7 +654,6 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): """ test_steps = [] # Initialization - print("INIT") store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) yield 'anchor_state', state yield 'anchor_block', anchor_block @@ -678,14 +675,12 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): state, store, _ = yield from apply_next_epoch_with_attestations( spec, state, store, False, True, test_steps=test_steps) - print("INIT FIN") assert state.finalized_checkpoint.epoch == store.finalized_checkpoint.epoch == 2 assert state.current_justified_checkpoint.epoch == store.justified_checkpoint.epoch == 4 assert store.justified_checkpoint == state.current_justified_checkpoint # Create another chain # Forking from epoch 3 - print("FORK") all_blocks = [] slot = spec.compute_start_slot_at_epoch(3) block_root = spec.get_block_root_at_slot(state, slot) @@ -697,7 +692,6 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert another_state.finalized_checkpoint.epoch == 3 assert another_state.current_justified_checkpoint.epoch == 4 - print("APPLY FORK TO FORK CHOICE") pre_store_justified_checkpoint_root = store.justified_checkpoint.root for block in all_blocks: # FIXME: Once `on_block` and `on_attestation` logic is fixed, @@ -708,11 +702,13 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) assert ancestor_at_finalized_slot == store.finalized_checkpoint.root - assert store.finalized_checkpoint == another_state.finalized_checkpoint - # Thus should fail with the fix. Once show fail, swap to == - # assert store.justified_checkpoint != another_state.current_justified_checkpoint print(store.finalized_checkpoint) print(store.justified_checkpoint) + print(another_state.current_justified_checkpoint) print('spec.get_head(store)', spec.get_head(store)) + assert store.finalized_checkpoint == another_state.finalized_checkpoint + # Thus should fail with the fix. Once show fail, swap to == + assert store.justified_checkpoint == another_state.current_justified_checkpoint + yield 'steps', test_steps From 3d4ece44df9b0e32bf07820f9ea820358e9cf626 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 17 Nov 2021 18:37:45 -0700 Subject: [PATCH 19/48] port phase0 forkchocie changes to merge --- specs/merge/fork-choice.md | 14 +------------- specs/phase0/fork-choice.md | 3 --- .../test/phase0/fork_choice/test_on_block.py | 8 -------- 3 files changed, 1 insertion(+), 24 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 3f0c672001..d60a07d1c8 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -186,17 +186,5 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Update finalized checkpoint if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: store.finalized_checkpoint = state.finalized_checkpoint - - # Potentially update justified if different from store - if store.justified_checkpoint != state.current_justified_checkpoint: - # Update justified if new justified is later than store justified - if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: - store.justified_checkpoint = state.current_justified_checkpoint - return - - # Update justified if store justified is not in chain with finalized checkpoint - finalized_slot = compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) - ancestor_at_finalized_slot = get_ancestor(store, store.justified_checkpoint.root, finalized_slot) - if ancestor_at_finalized_slot != store.finalized_checkpoint.root: - store.justified_checkpoint = state.current_justified_checkpoint + store.justified_checkpoint = state.current_justified_checkpoint ``` diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 723102e4df..6854a8487c 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -386,9 +386,6 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Update finalized checkpoint if state.finalized_checkpoint.epoch > store.finalized_checkpoint.epoch: - print("made it") - print(state.finalized_checkpoint) - print(state.current_justified_checkpoint) store.finalized_checkpoint = state.finalized_checkpoint store.justified_checkpoint = state.current_justified_checkpoint ``` diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 3670a883af..ff2d77a8fb 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -694,21 +694,13 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): pre_store_justified_checkpoint_root = store.justified_checkpoint.root for block in all_blocks: - # FIXME: Once `on_block` and `on_attestation` logic is fixed, - # fix test case and remove allow_invalid_attestations flag yield from tick_and_add_block(spec, store, block, test_steps) finalized_slot = spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) assert ancestor_at_finalized_slot == store.finalized_checkpoint.root - print(store.finalized_checkpoint) - print(store.justified_checkpoint) - print(another_state.current_justified_checkpoint) - print('spec.get_head(store)', spec.get_head(store)) - assert store.finalized_checkpoint == another_state.finalized_checkpoint - # Thus should fail with the fix. Once show fail, swap to == assert store.justified_checkpoint == another_state.current_justified_checkpoint yield 'steps', test_steps From 2c865e362799470356b703afdef83c49bb55bd38 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Thu, 18 Nov 2021 11:02:12 +0800 Subject: [PATCH 20/48] Resolve the commented out code in `test_new_finalized_slot_is_not_justified_checkpoint_ancestor` --- .../test/phase0/fork_choice/test_on_block.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index ff2d77a8fb..7194c6a204 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -618,20 +618,19 @@ def test_new_finalized_slot_is_not_justified_checkpoint_ancestor(spec, state): assert state.finalized_checkpoint != another_state.finalized_checkpoint assert state.current_justified_checkpoint != another_state.current_justified_checkpoint - # pre_store_justified_checkpoint_root = store.justified_checkpoint.root + pre_store_justified_checkpoint_root = store.justified_checkpoint.root - # FIXME: pending on the `on_block`, `on_attestation` fix - # # Apply blocks of `another_state` to `store` - # for block in all_blocks: - # # NOTE: Do not call `on_tick` here - # yield from add_block(spec, store, block, test_steps) + # Apply blocks of `another_state` to `store` + for block in all_blocks: + # NOTE: Do not call `on_tick` here + yield from add_block(spec, store, block, test_steps) - # finalized_slot = spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) - # ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) - # assert ancestor_at_finalized_slot != store.finalized_checkpoint.root + finalized_slot = spec.compute_start_slot_at_epoch(store.finalized_checkpoint.epoch) + ancestor_at_finalized_slot = spec.get_ancestor(store, pre_store_justified_checkpoint_root, finalized_slot) + assert ancestor_at_finalized_slot != store.finalized_checkpoint.root - # assert store.finalized_checkpoint == another_state.finalized_checkpoint - # assert store.justified_checkpoint == another_state.current_justified_checkpoint + assert store.finalized_checkpoint == another_state.finalized_checkpoint + assert store.justified_checkpoint == another_state.current_justified_checkpoint yield 'steps', test_steps From 58968f92865d3d17a06f456675d1f5fa4b72a568 Mon Sep 17 00:00:00 2001 From: Mikhail Kalinin Date: Thu, 18 Nov 2021 19:23:52 +0600 Subject: [PATCH 21/48] Rename coinbase to fee_recipient --- specs/merge/beacon-chain.md | 6 +++--- specs/merge/fork-choice.md | 2 +- specs/merge/validator.md | 8 ++++---- .../pyspec/eth2spec/test/helpers/execution_payload.py | 4 ++-- tests/core/pyspec/eth2spec/test/helpers/genesis.py | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index a4b4a23c75..ac5b6d0ecf 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -167,7 +167,7 @@ class BeaconState(Container): class ExecutionPayload(Container): # Execution block header fields parent_hash: Hash32 - coinbase: ExecutionAddress # 'beneficiary' in the yellow paper + fee_recipient: ExecutionAddress # 'beneficiary' in the yellow paper state_root: Bytes32 receipt_root: Bytes32 # 'receipts root' in the yellow paper logs_bloom: ByteVector[BYTES_PER_LOGS_BLOOM] @@ -189,7 +189,7 @@ class ExecutionPayload(Container): class ExecutionPayloadHeader(Container): # Execution block header fields parent_hash: Hash32 - coinbase: ExecutionAddress + fee_recipient: ExecutionAddress state_root: Bytes32 receipt_root: Bytes32 logs_bloom: ByteVector[BYTES_PER_LOGS_BLOOM] @@ -357,7 +357,7 @@ def process_execution_payload(state: BeaconState, payload: ExecutionPayload, exe # Cache execution payload header state.latest_execution_payload_header = ExecutionPayloadHeader( parent_hash=payload.parent_hash, - coinbase=payload.coinbase, + fee_recipient=payload.fee_recipient, state_root=payload.state_root, receipt_root=payload.receipt_root, logs_bloom=payload.logs_bloom, diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 3f0c672001..4a42a260e7 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -77,7 +77,7 @@ Used to signal to initiate the payload build process via `notify_forkchoice_upda class PayloadAttributes(object): timestamp: uint64 random: Bytes32 - fee_recipient: ExecutionAddress + suggested_fee_recipient: ExecutionAddress ``` ### `PowBlock` diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 22fc3eb137..ff400fce34 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -109,18 +109,18 @@ All validator responsibilities remain unchanged other than those noted below. Na To obtain an execution payload, a block proposer building a block on top of a `state` must take the following actions: -1. Set `payload_id = prepare_execution_payload(state, pow_chain, finalized_block_hash, fee_recipient, execution_engine)`, where: +1. Set `payload_id = prepare_execution_payload(state, pow_chain, finalized_block_hash, suggested_fee_recipient, execution_engine)`, where: * `state` is the state object after applying `process_slots(state, slot)` transition to the resulting state of the parent block processing * `pow_chain` is a `Dict[Hash32, PowBlock]` dictionary that abstractly represents all blocks in the PoW chain with block hash as the dictionary key * `finalized_block_hash` is the hash of the latest finalized execution payload (`Hash32()` if none yet finalized) - * `fee_recipient` is the value suggested to be used for the `coinbase` field of the execution payload + * `suggested_fee_recipient` is the value suggested to be used for the `fee_recipient` field of the execution payload ```python def prepare_execution_payload(state: BeaconState, pow_chain: Dict[Hash32, PowBlock], finalized_block_hash: Hash32, - fee_recipient: ExecutionAddress, + suggested_fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() @@ -143,7 +143,7 @@ def prepare_execution_payload(state: BeaconState, payload_attributes = PayloadAttributes( timestamp=compute_timestamp_at_slot(state, state.slot), random=get_randao_mix(state, get_current_epoch(state)), - fee_recipient=fee_recipient, + suggested_fee_recipient=suggested_fee_recipient, ) return execution_engine.notify_forkchoice_updated(parent_hash, finalized_block_hash, payload_attributes) ``` diff --git a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py index 59308e102a..9c9663584b 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py +++ b/tests/core/pyspec/eth2spec/test/helpers/execution_payload.py @@ -11,7 +11,7 @@ def build_empty_execution_payload(spec, state, randao_mix=None): payload = spec.ExecutionPayload( parent_hash=latest.block_hash, - coinbase=spec.ExecutionAddress(), + fee_recipient=spec.ExecutionAddress(), state_root=latest.state_root, # no changes to the state receipt_root=b"no receipts here" + b"\x00" * 16, # TODO: root of empty MPT may be better. logs_bloom=spec.ByteVector[spec.BYTES_PER_LOGS_BLOOM](), # TODO: zeroed logs bloom for empty logs ok? @@ -34,7 +34,7 @@ def build_empty_execution_payload(spec, state, randao_mix=None): def get_execution_payload_header(spec, execution_payload): return spec.ExecutionPayloadHeader( parent_hash=execution_payload.parent_hash, - coinbase=execution_payload.coinbase, + fee_recipient=execution_payload.fee_recipient, state_root=execution_payload.state_root, receipt_root=execution_payload.receipt_root, logs_bloom=execution_payload.logs_bloom, diff --git a/tests/core/pyspec/eth2spec/test/helpers/genesis.py b/tests/core/pyspec/eth2spec/test/helpers/genesis.py index 4082950728..ed90a7d4e8 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/genesis.py +++ b/tests/core/pyspec/eth2spec/test/helpers/genesis.py @@ -26,7 +26,7 @@ def get_sample_genesis_execution_payload_header(spec, eth1_block_hash = b'\x55' * 32 return spec.ExecutionPayloadHeader( parent_hash=b'\x30' * 32, - coinbase=b'\x42' * 20, + fee_recipient=b'\x42' * 20, state_root=b'\x20' * 32, receipt_root=b'\x20' * 32, logs_bloom=b'\x35' * spec.BYTES_PER_LOGS_BLOOM, From 83335144223dfd0bbe2faef2ea5899a1bbbe4bea Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 19 Nov 2021 09:26:43 -0700 Subject: [PATCH 22/48] Update specs/merge/validator.md --- specs/merge/validator.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 0d1344bfca..6bd9217ca2 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -49,11 +49,10 @@ def get_pow_block_at_terminal_total_difficulty(pow_chain: Dict[Hash32, PowBlock] # If genesis block, no parent exists so reaching TTD alone qualifies as valid terminal block if block.parent_hash == Hash32(): return block - else: - parent = pow_chain[block.parent_hash] - parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY - if not parent_reached_ttd: - return block + parent = pow_chain[block.parent_hash] + parent_reached_ttd = parent.total_difficulty >= TERMINAL_TOTAL_DIFFICULTY + if not parent_reached_ttd: + return block return None ``` From 2d161b4244a1318546130ce57f5b87b613951908 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Fri, 19 Nov 2021 12:35:59 -0800 Subject: [PATCH 23/48] Add proposer score boosting & related tests --- specs/merge/fork-choice.md | 8 ++ specs/phase0/fork-choice.md | 30 +++++++- .../test/phase0/fork_choice/test_get_head.py | 76 ++++++++++++++++++- .../test/phase0/fork_choice/test_on_block.py | 26 +++++++ 4 files changed, 136 insertions(+), 4 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 8c5bc13128..f62c8e014a 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -175,6 +175,14 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[hash_tree_root(block)] = state + # Add proposer score boost if the block is timely + if (get_current_slot(store) == block.slot and + store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): + store.proposer_score_boost = LatestMessage( + root=hash_tree_root(block), + epoch=compute_epoch_at_slot(block.slot) + ) + # Update justified checkpoint if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: if state.current_justified_checkpoint.epoch > store.best_justified_checkpoint.epoch: diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 6854a8487c..7310d68512 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -61,6 +61,7 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | Name | Value | Unit | Duration | | - | - | :-: | :-: | | `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` | `2**3` (= 8) | slots | 96 seconds | +| `ATTESTATION_OFFSET_QUOTIENT` | `3` | - | - | ### Helpers @@ -83,6 +84,7 @@ class Store(object): justified_checkpoint: Checkpoint finalized_checkpoint: Checkpoint best_justified_checkpoint: Checkpoint + proposer_score_boost: LatestMessage blocks: Dict[Root, BeaconBlock] = field(default_factory=dict) block_states: Dict[Root, BeaconState] = field(default_factory=dict) checkpoint_states: Dict[Checkpoint, BeaconState] = field(default_factory=dict) @@ -103,12 +105,14 @@ def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) - anchor_epoch = get_current_epoch(anchor_state) justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) + proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) return Store( time=uint64(anchor_state.genesis_time + SECONDS_PER_SLOT * anchor_state.slot), genesis_time=anchor_state.genesis_time, justified_checkpoint=justified_checkpoint, finalized_checkpoint=finalized_checkpoint, best_justified_checkpoint=justified_checkpoint, + proposer_score_boost=proposer_score_boost, blocks={anchor_root: copy(anchor_block)}, block_states={anchor_root: copy(anchor_state)}, checkpoint_states={justified_checkpoint: copy(anchor_state)}, @@ -156,11 +160,23 @@ def get_ancestor(store: Store, root: Root, slot: Slot) -> Root: def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: state = store.checkpoint_states[store.justified_checkpoint] active_indices = get_active_validator_indices(state, get_current_epoch(state)) - return Gwei(sum( + attestation_score = Gwei(sum( state.validators[i].effective_balance for i in active_indices if (i in store.latest_messages and get_ancestor(store, store.latest_messages[i].root, store.blocks[root].slot) == root) )) + proposer_score = Gwei(0) + if store.proposer_score_boost.root != Root(): + block_slot = store.blocks[root].slot + if get_ancestor(store, root, block_slot) == store.proposer_score_boost.root: + num_validators = len(get_active_validator_indices(state, get_current_epoch(state))) + avg_balance = get_total_active_balance(state) // num_validators + block_epoch = compute_epoch_at_slot(block_slot) + committee_size = get_committee_count_per_slot(state, block_epoch) * TARGET_COMMITTEE_SIZE + committee_weight = committee_size * avg_balance + proposer_score = committee_weight // 4 + return attestation_score + proposer_score + ``` #### `filter_block_tree` @@ -339,6 +355,10 @@ def on_tick(store: Store, time: uint64) -> None: store.time = time current_slot = get_current_slot(store) + # Reset store.proposer_score_boost if this is a new slot + if store.proposer_score_boost.root != Root(): + if current_slot != store.blocks[store.proposer_score_boost.root].slot: + store.proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) # Not a new epoch, return if not (current_slot > previous_slot and compute_slots_since_epoch_start(current_slot) == 0): return @@ -377,6 +397,14 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add new state for this block to the store store.block_states[hash_tree_root(block)] = state + # Add proposer score boost if the block is timely + if (get_current_slot(store) == block.slot and + store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): + store.proposer_score_boost = LatestMessage( + root=hash_tree_root(block), + epoch=compute_epoch_at_slot(block.slot) + ) + # Update justified checkpoint if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: if state.current_justified_checkpoint.epoch > store.best_justified_checkpoint.epoch: diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py index 12b261e4e5..bb9cbee2e2 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py @@ -1,3 +1,4 @@ +import random from eth_utils import encode_hex from eth2spec.test.context import ( @@ -19,6 +20,7 @@ add_block, ) from eth2spec.test.helpers.state import ( + next_slots, next_epoch, state_transition_and_sign_block, ) @@ -103,17 +105,21 @@ def test_split_tie_breaker_no_attestations(spec, state): } }) - # block at slot 1 + # Create block at slot 1 block_1_state = genesis_state.copy() block_1 = build_empty_block_for_next_slot(spec, block_1_state) signed_block_1 = state_transition_and_sign_block(spec, block_1_state, block_1) - yield from tick_and_add_block(spec, store, signed_block_1, test_steps) - # additional block at slot 1 + # Create additional block at slot 1 block_2_state = genesis_state.copy() block_2 = build_empty_block_for_next_slot(spec, block_2_state) block_2.body.graffiti = b'\x42' * 32 signed_block_2 = state_transition_and_sign_block(spec, block_2_state, block_2) + + # Tick time past slot 1 so proposer score boost does not apply + spec.on_tick(store, store.genesis_time + (block_2.slot + 1) * spec.config.SECONDS_PER_SLOT) + + yield from tick_and_add_block(spec, store, signed_block_1, test_steps) yield from tick_and_add_block(spec, store, signed_block_2, test_steps) highest_root = max(spec.hash_tree_root(block_1), spec.hash_tree_root(block_2)) @@ -261,3 +267,67 @@ def test_filtered_block_tree(spec, state): }) yield 'steps', test_steps + + +@with_all_phases +@spec_state_test +def test_proposer_score_boost_basic(spec, state): + test_steps = [] + genesis_state = state.copy() + + # Initialization + store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) + yield 'anchor_state', state + yield 'anchor_block', anchor_block + anchor_root = get_anchor_root(spec, state) + assert spec.get_head(store) == anchor_root + test_steps.append({ + 'checks': { + 'head': get_formatted_head_output(spec, store), + } + }) + + # Build block that serves as head ONLY on timely arrival, and ONLY in that slot + state_1 = genesis_state.copy() + next_slots(spec, state_1, 3) + block_1 = build_empty_block_for_next_slot(spec, state_1) + signed_block_1 = state_transition_and_sign_block(spec, state_1, block_1) + + # Build block that serves as current head, and remains the head after block_1.slot + state_2 = genesis_state.copy() + next_slots(spec, state_2, 2) + block_2 = build_empty_block_for_next_slot(spec, state_2) + block_2.body.graffiti = spec.Bytes32(hex(random.getrandbits(8 * 32))[2:].zfill(64)) + signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2) + while spec.hash_tree_root(block_1) > spec.hash_tree_root(block_2): + block_2.body.graffiti = spec.Bytes32(hex(random.getrandbits(8 * 32))[2:].zfill(64)) + signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2) + assert spec.hash_tree_root(block_1) < spec.hash_tree_root(block_2) + + # Tick to block_1 slot time + spec.on_tick(store, store.genesis_time + block_1.slot * spec.config.SECONDS_PER_SLOT) + + # Process block_2 + yield from tick_and_add_block(spec, store, signed_block_2, test_steps) + assert store.proposer_score_boost.root == spec.Root() + assert spec.get_head(store) == spec.hash_tree_root(block_2) + + # Process block_1 on timely arrival + # The head should temporarily change to block_1 + yield from tick_and_add_block(spec, store, signed_block_1, test_steps) + assert store.proposer_score_boost == spec.LatestMessage(root=spec.hash_tree_root(block_1), + epoch=spec.compute_epoch_at_slot(block_1.slot)) + assert spec.get_head(store) == spec.hash_tree_root(block_1) + + # After block_1.slot, the head should revert to block_2 + spec.on_tick(store, store.genesis_time + (block_1.slot + 1) * spec.config.SECONDS_PER_SLOT) + assert store.proposer_score_boost.root == spec.Root() + assert spec.get_head(store) == spec.hash_tree_root(block_2) + + test_steps.append({ + 'checks': { + 'head': get_formatted_head_output(spec, store), + } + }) + + yield 'steps', test_steps diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 7194c6a204..6cf04559b5 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -703,3 +703,29 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): assert store.justified_checkpoint == another_state.current_justified_checkpoint yield 'steps', test_steps + + +@with_all_phases +@spec_state_test +def test_proposer_score_boost_same_slot_untimely_block(spec, state): + test_steps = [] + genesis_state = state.copy() + + # Initialization + store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) + yield 'anchor_state', state + yield 'anchor_block', anchor_block + + # Build block that serves as head ONLY on timely arrival, and ONLY in that slot + state = genesis_state.copy() + next_slots(spec, state, 3) + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + + # Process block on untimely arrival in the same slot + spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT // spec.ATTESTATION_OFFSET_QUOTIENT) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert store.proposer_score_boost.root == spec.Root() + + yield 'steps', test_steps From 281c1b2d1ab333710f07e80bcd07a172db97d07c Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Fri, 19 Nov 2021 12:47:33 -0800 Subject: [PATCH 24/48] Update validator guide with ATTESTATION_OFFSET_QUOTIENT --- specs/phase0/validator.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index cc5b7aeb59..b25a9a8b3f 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -446,7 +446,7 @@ def get_block_signature(state: BeaconState, block: BeaconBlock, privkey: int) -> A validator is expected to create, sign, and broadcast an attestation during each epoch. The `committee`, assigned `index`, and assigned `slot` for which the validator performs this role during an epoch are defined by `get_committee_assignment(state, epoch, validator_index)`. -A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) one-third of the `slot` has transpired (`SECONDS_PER_SLOT / 3` seconds after the start of `slot`) -- whichever comes _first_. +A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) one-third of the `slot` has transpired (`SECONDS_PER_SLOT / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`) -- whichever comes _first_. *Note*: Although attestations during `GENESIS_EPOCH` do not count toward FFG finality, these initial attestations do give weight to the fork choice, are rewarded, and should be made. From 47fa6d108a9d7d0a5fbfc127219a34f1cfd86b39 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Fri, 19 Nov 2021 18:27:47 -0800 Subject: [PATCH 25/48] Add parameter for score boost value --- specs/phase0/fork-choice.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 7310d68512..65783d34da 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -8,6 +8,7 @@ - [Introduction](#introduction) - [Fork choice](#fork-choice) - [Preset](#preset) + - [Configuration](#configuration) - [Helpers](#helpers) - [`LatestMessage`](#latestmessage) - [`Store`](#store) @@ -63,6 +64,12 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` | `2**3` (= 8) | slots | 96 seconds | | `ATTESTATION_OFFSET_QUOTIENT` | `3` | - | - | +### Configuration + +| Name | Value | +| - | - | +| `PROPOSER_SCORE_BOOST_QUOTIENT` | `4` | + ### Helpers #### `LatestMessage` @@ -174,7 +181,7 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: block_epoch = compute_epoch_at_slot(block_slot) committee_size = get_committee_count_per_slot(state, block_epoch) * TARGET_COMMITTEE_SIZE committee_weight = committee_size * avg_balance - proposer_score = committee_weight // 4 + proposer_score = committee_weight // PROPOSER_SCORE_BOOST_QUOTIENT return attestation_score + proposer_score ``` From 504d82cc1ac751bb39c14323a73bd9ebea92ca5e Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Sat, 20 Nov 2021 09:16:43 -0800 Subject: [PATCH 26/48] Add datatype to new parameters --- specs/phase0/fork-choice.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 65783d34da..e43fca3a56 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -62,13 +62,13 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | Name | Value | Unit | Duration | | - | - | :-: | :-: | | `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` | `2**3` (= 8) | slots | 96 seconds | -| `ATTESTATION_OFFSET_QUOTIENT` | `3` | - | - | +| `ATTESTATION_OFFSET_QUOTIENT` | `uint64(3)` | - | - | ### Configuration | Name | Value | | - | - | -| `PROPOSER_SCORE_BOOST_QUOTIENT` | `4` | +| `PROPOSER_SCORE_BOOST_QUOTIENT` | `uint64(4)` | ### Helpers From b0fb861cf51197df71736db06ebcb3363950791d Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Sat, 20 Nov 2021 09:35:45 -0800 Subject: [PATCH 27/48] Make PROPOSER_SCORE_BOOST a percentage value --- specs/phase0/fork-choice.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index e43fca3a56..5cb87a0615 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -68,7 +68,9 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | Name | Value | | - | - | -| `PROPOSER_SCORE_BOOST_QUOTIENT` | `uint64(4)` | +| `PROPOSER_SCORE_BOOST` | `uint64(25)` | + +- The proposer score boost is worth `PROPOSER_SCORE_BOOST` percentage of the committee's weight, i.e., for slot with committee weight `committee_weight` the boost weight is equal to `(committee_weight * PROPOSER_SCORE_BOOST) // 100`. ### Helpers @@ -181,7 +183,7 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: block_epoch = compute_epoch_at_slot(block_slot) committee_size = get_committee_count_per_slot(state, block_epoch) * TARGET_COMMITTEE_SIZE committee_weight = committee_size * avg_balance - proposer_score = committee_weight // PROPOSER_SCORE_BOOST_QUOTIENT + proposer_score = (committee_weight * PROPOSER_SCORE_BOOST) // 100 return attestation_score + proposer_score ``` From 3b20e3ea02828c367be7841020e4be4eafb21263 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Mon, 22 Nov 2021 08:38:08 -0800 Subject: [PATCH 28/48] Apply suggestions from code review Co-authored-by: Danny Ryan --- specs/merge/fork-choice.md | 4 ++-- specs/phase0/fork-choice.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index f62c8e014a..342ef0f69d 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -176,8 +176,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - if (get_current_slot(store) == block.slot and - store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): + is_before_attestation_broadcast = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT + if get_current_slot(store) == block.slot and is_before_attestation_broadcast: store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 5cb87a0615..e3e18f9b16 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -114,7 +114,7 @@ def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) - anchor_epoch = get_current_epoch(anchor_state) justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) - proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) + proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch()) return Store( time=uint64(anchor_state.genesis_time + SECONDS_PER_SLOT * anchor_state.slot), genesis_time=anchor_state.genesis_time, From 859bbf435854d14bc397817d25cbb613fc465afe Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Mon, 22 Nov 2021 08:48:40 -0800 Subject: [PATCH 29/48] This reverts commit 4c726cdff39a10c5d096b294fb562cfc99c1f068. --- specs/merge/fork-choice.md | 4 ++-- specs/phase0/fork-choice.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 342ef0f69d..f62c8e014a 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -176,8 +176,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - is_before_attestation_broadcast = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT - if get_current_slot(store) == block.slot and is_before_attestation_broadcast: + if (get_current_slot(store) == block.slot and + store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index e3e18f9b16..5cb87a0615 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -114,7 +114,7 @@ def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) - anchor_epoch = get_current_epoch(anchor_state) justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) - proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch()) + proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) return Store( time=uint64(anchor_state.genesis_time + SECONDS_PER_SLOT * anchor_state.slot), genesis_time=anchor_state.genesis_time, From 88c76abd7fba95ef8352dbf2b256e7eb3687a65b Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Mon, 22 Nov 2021 09:34:32 -0800 Subject: [PATCH 30/48] Apply Danny's code review --- specs/merge/fork-choice.md | 6 ++++-- specs/phase0/fork-choice.md | 13 +++++++------ specs/phase0/validator.md | 4 ++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index f62c8e014a..bf67d2f4f7 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -176,8 +176,10 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - if (get_current_slot(store) == block.slot and - store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): + is_before_attestation_broadcast = ( + store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT + ) + if get_current_slot(store) == block.slot and is_before_attestation_broadcast: store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 5cb87a0615..f302887ecf 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -176,12 +176,11 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: )) proposer_score = Gwei(0) if store.proposer_score_boost.root != Root(): - block_slot = store.blocks[root].slot - if get_ancestor(store, root, block_slot) == store.proposer_score_boost.root: + block = store.blocks[root] + if get_ancestor(store, root, block.slot) == store.proposer_score_boost.root: num_validators = len(get_active_validator_indices(state, get_current_epoch(state))) avg_balance = get_total_active_balance(state) // num_validators - block_epoch = compute_epoch_at_slot(block_slot) - committee_size = get_committee_count_per_slot(state, block_epoch) * TARGET_COMMITTEE_SIZE + committee_size = num_validators // SLOTS_PER_EPOCH committee_weight = committee_size * avg_balance proposer_score = (committee_weight * PROPOSER_SCORE_BOOST) // 100 return attestation_score + proposer_score @@ -407,8 +406,10 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - if (get_current_slot(store) == block.slot and - store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT): + is_before_attestation_broadcast = ( + store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT + ) + if get_current_slot(store) == block.slot and is_before_attestation_broadcast: store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index b25a9a8b3f..209cc7e5d2 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -446,7 +446,7 @@ def get_block_signature(state: BeaconState, block: BeaconBlock, privkey: int) -> A validator is expected to create, sign, and broadcast an attestation during each epoch. The `committee`, assigned `index`, and assigned `slot` for which the validator performs this role during an epoch are defined by `get_committee_assignment(state, epoch, validator_index)`. -A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) one-third of the `slot` has transpired (`SECONDS_PER_SLOT / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`) -- whichever comes _first_. +A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) `1 / ATTESTATION_OFFSET_QUOTIENT` of the `slot` has transpired (`SECONDS_PER_SLOT / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`) -- whichever comes _first_. *Note*: Although attestations during `GENESIS_EPOCH` do not count toward FFG finality, these initial attestations do give weight to the fork choice, are rewarded, and should be made. @@ -569,7 +569,7 @@ def get_aggregate_signature(attestations: Sequence[Attestation]) -> BLSSignature #### Broadcast aggregate -If the validator is selected to aggregate (`is_aggregator`), then they broadcast their best aggregate as a `SignedAggregateAndProof` to the global aggregate channel (`beacon_aggregate_and_proof`) two-thirds of the way through the `slot`-that is, `SECONDS_PER_SLOT * 2 / 3` seconds after the start of `slot`. +If the validator is selected to aggregate (`is_aggregator`), then they broadcast their best aggregate as a `SignedAggregateAndProof` to the global aggregate channel (`beacon_aggregate_and_proof`) `2 / ATTESTATION_OFFSET_QUOTIENT` of the way through the `slot`-that is, `SECONDS_PER_SLOT * 2 / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`. Selection proofs are provided in `AggregateAndProof` to prove to the gossip channel that the validator has been selected as an aggregator. From cebe6ba7e75d0917bc4f6891d49ebda1200b347c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 22 Nov 2021 11:29:12 -0700 Subject: [PATCH 31/48] minor formatting cleanups --- specs/merge/fork-choice.md | 6 ++---- specs/phase0/beacon-chain.md | 1 - specs/phase0/fork-choice.md | 14 +++++++++----- specs/phase0/validator.md | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index bf67d2f4f7..7a8db7dd9c 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -176,10 +176,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - is_before_attestation_broadcast = ( - store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT - ) - if get_current_slot(store) == block.slot and is_before_attestation_broadcast: + is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + if get_current_slot(store) == block.slot and is_before_attesting_interval: store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index e2e235acf1..5216027cdc 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -169,7 +169,6 @@ We define the following Python custom types for type hinting and readability: | `BLSPubkey` | `Bytes48` | a BLS12-381 public key | | `BLSSignature` | `Bytes96` | a BLS12-381 signature | - ## Constants The following values are (non-configurable) constants used throughout the specification. diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index f302887ecf..7a87d00703 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -57,12 +57,18 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass 4) **Manual forks**: Manual forks may arbitrarily change the fork choice rule but are expected to be enacted at epoch transitions, with the fork details reflected in `state.fork`. 5) **Implementation**: The implementation found in this specification is constructed for ease of understanding rather than for optimization in computation, space, or any other resource. A number of optimized alternatives can be found [here](https://github.com/protolambda/lmd-ghost). + +### Constant + +| Name | Value | +| - | - | +| `INTERVALS_PER_SLOT` | `uint64(3)` | + ### Preset | Name | Value | Unit | Duration | | - | - | :-: | :-: | | `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` | `2**3` (= 8) | slots | 96 seconds | -| `ATTESTATION_OFFSET_QUOTIENT` | `uint64(3)` | - | - | ### Configuration @@ -406,10 +412,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - is_before_attestation_broadcast = ( - store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // ATTESTATION_OFFSET_QUOTIENT - ) - if get_current_slot(store) == block.slot and is_before_attestation_broadcast: + is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + if get_current_slot(store) == block.slot and is_before_attesting_interval: store.proposer_score_boost = LatestMessage( root=hash_tree_root(block), epoch=compute_epoch_at_slot(block.slot) diff --git a/specs/phase0/validator.md b/specs/phase0/validator.md index 209cc7e5d2..e21ff980ab 100644 --- a/specs/phase0/validator.md +++ b/specs/phase0/validator.md @@ -446,7 +446,7 @@ def get_block_signature(state: BeaconState, block: BeaconBlock, privkey: int) -> A validator is expected to create, sign, and broadcast an attestation during each epoch. The `committee`, assigned `index`, and assigned `slot` for which the validator performs this role during an epoch are defined by `get_committee_assignment(state, epoch, validator_index)`. -A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) `1 / ATTESTATION_OFFSET_QUOTIENT` of the `slot` has transpired (`SECONDS_PER_SLOT / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`) -- whichever comes _first_. +A validator should create and broadcast the `attestation` to the associated attestation subnet when either (a) the validator has received a valid block from the expected block proposer for the assigned `slot` or (b) `1 / INTERVALS_PER_SLOT` of the `slot` has transpired (`SECONDS_PER_SLOT / INTERVALS_PER_SLOT` seconds after the start of `slot`) -- whichever comes _first_. *Note*: Although attestations during `GENESIS_EPOCH` do not count toward FFG finality, these initial attestations do give weight to the fork choice, are rewarded, and should be made. @@ -569,7 +569,7 @@ def get_aggregate_signature(attestations: Sequence[Attestation]) -> BLSSignature #### Broadcast aggregate -If the validator is selected to aggregate (`is_aggregator`), then they broadcast their best aggregate as a `SignedAggregateAndProof` to the global aggregate channel (`beacon_aggregate_and_proof`) `2 / ATTESTATION_OFFSET_QUOTIENT` of the way through the `slot`-that is, `SECONDS_PER_SLOT * 2 / ATTESTATION_OFFSET_QUOTIENT` seconds after the start of `slot`. +If the validator is selected to aggregate (`is_aggregator`), then they broadcast their best aggregate as a `SignedAggregateAndProof` to the global aggregate channel (`beacon_aggregate_and_proof`) `2 / INTERVALS_PER_SLOT` of the way through the `slot`-that is, `SECONDS_PER_SLOT * 2 / INTERVALS_PER_SLOT` seconds after the start of `slot`. Selection proofs are provided in `AggregateAndProof` to prove to the gossip channel that the validator has been selected as an aggregator. From 282d85b9e7ea3148c7bae80a11c375d3e7497ecd Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 22 Nov 2021 11:31:44 -0700 Subject: [PATCH 32/48] simplify on_tick proposer boost update --- specs/phase0/fork-choice.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 7a87d00703..420ae57222 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -369,10 +369,11 @@ def on_tick(store: Store, time: uint64) -> None: store.time = time current_slot = get_current_slot(store) + # Reset store.proposer_score_boost if this is a new slot - if store.proposer_score_boost.root != Root(): - if current_slot != store.blocks[store.proposer_score_boost.root].slot: - store.proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) + if current_slot > previous_slot: + store.proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) + # Not a new epoch, return if not (current_slot > previous_slot and compute_slots_since_epoch_start(current_slot) == 0): return From ea09df50db2d764825ebcc9eddc468b48440c1a2 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 22 Nov 2021 11:33:03 -0700 Subject: [PATCH 33/48] toc --- specs/phase0/fork-choice.md | 1 + .../pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 420ae57222..9c27a423f9 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -7,6 +7,7 @@ - [Introduction](#introduction) - [Fork choice](#fork-choice) + - [Constant](#constant) - [Preset](#preset) - [Configuration](#configuration) - [Helpers](#helpers) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 6cf04559b5..c4c9fffc11 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -724,7 +724,7 @@ def test_proposer_score_boost_same_slot_untimely_block(spec, state): # Process block on untimely arrival in the same slot spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + - spec.config.SECONDS_PER_SLOT // spec.ATTESTATION_OFFSET_QUOTIENT) + spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT) yield from tick_and_add_block(spec, store, signed_block, test_steps) assert store.proposer_score_boost.root == spec.Root() From 1d835c5198aa744a1ce64e7c61c0f61da421cda3 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Mon, 22 Nov 2021 14:44:52 -0800 Subject: [PATCH 34/48] Apply Danny's code review & suggestions --- specs/merge/fork-choice.md | 5 +- specs/phase0/fork-choice.md | 19 +++---- .../test/phase0/fork_choice/test_on_block.py | 53 ++++++++++++++++++- 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 7a8db7dd9c..cbfe33f4a6 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -178,10 +178,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add proposer score boost if the block is timely is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT if get_current_slot(store) == block.slot and is_before_attesting_interval: - store.proposer_score_boost = LatestMessage( - root=hash_tree_root(block), - epoch=compute_epoch_at_slot(block.slot) - ) + store.proposer_boost_root = hash_tree_root(block) # Update justified checkpoint if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 9c27a423f9..6e50b99d4b 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -100,7 +100,7 @@ class Store(object): justified_checkpoint: Checkpoint finalized_checkpoint: Checkpoint best_justified_checkpoint: Checkpoint - proposer_score_boost: LatestMessage + proposer_boost_root: Root blocks: Dict[Root, BeaconBlock] = field(default_factory=dict) block_states: Dict[Root, BeaconState] = field(default_factory=dict) checkpoint_states: Dict[Checkpoint, BeaconState] = field(default_factory=dict) @@ -121,14 +121,14 @@ def get_forkchoice_store(anchor_state: BeaconState, anchor_block: BeaconBlock) - anchor_epoch = get_current_epoch(anchor_state) justified_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) finalized_checkpoint = Checkpoint(epoch=anchor_epoch, root=anchor_root) - proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) + proposer_boost_root = Root() return Store( time=uint64(anchor_state.genesis_time + SECONDS_PER_SLOT * anchor_state.slot), genesis_time=anchor_state.genesis_time, justified_checkpoint=justified_checkpoint, finalized_checkpoint=finalized_checkpoint, best_justified_checkpoint=justified_checkpoint, - proposer_score_boost=proposer_score_boost, + proposer_boost_root=proposer_boost_root, blocks={anchor_root: copy(anchor_block)}, block_states={anchor_root: copy(anchor_state)}, checkpoint_states={justified_checkpoint: copy(anchor_state)}, @@ -182,9 +182,9 @@ def get_latest_attesting_balance(store: Store, root: Root) -> Gwei: and get_ancestor(store, store.latest_messages[i].root, store.blocks[root].slot) == root) )) proposer_score = Gwei(0) - if store.proposer_score_boost.root != Root(): + if store.proposer_boost_root != Root(): block = store.blocks[root] - if get_ancestor(store, root, block.slot) == store.proposer_score_boost.root: + if get_ancestor(store, root, block.slot) == store.proposer_boost_root: num_validators = len(get_active_validator_indices(state, get_current_epoch(state))) avg_balance = get_total_active_balance(state) // num_validators committee_size = num_validators // SLOTS_PER_EPOCH @@ -371,9 +371,9 @@ def on_tick(store: Store, time: uint64) -> None: current_slot = get_current_slot(store) - # Reset store.proposer_score_boost if this is a new slot + # Reset store.proposer_boost_root if this is a new slot if current_slot > previous_slot: - store.proposer_score_boost = LatestMessage(root=Root(), epoch=Epoch(0)) + store.proposer_boost_root = Root() # Not a new epoch, return if not (current_slot > previous_slot and compute_slots_since_epoch_start(current_slot) == 0): @@ -416,10 +416,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: # Add proposer score boost if the block is timely is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT if get_current_slot(store) == block.slot and is_before_attesting_interval: - store.proposer_score_boost = LatestMessage( - root=hash_tree_root(block), - epoch=compute_epoch_at_slot(block.slot) - ) + store.proposer_boost_root = hash_tree_root(block) # Update justified checkpoint if state.current_justified_checkpoint.epoch > store.justified_checkpoint.epoch: diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index c4c9fffc11..f234125e04 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -707,7 +707,56 @@ def test_new_finalized_slot_is_justified_checkpoint_ancestor(spec, state): @with_all_phases @spec_state_test -def test_proposer_score_boost_same_slot_untimely_block(spec, state): +def test_proposer_boost(spec, state): + test_steps = [] + genesis_state = state.copy() + + # Initialization + store, anchor_block = get_genesis_forkchoice_store_and_block(spec, state) + yield 'anchor_state', state + yield 'anchor_block', anchor_block + + # Build block that serves as head ONLY on timely arrival, and ONLY in that slot + state = genesis_state.copy() + next_slots(spec, state, 3) + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + + # Process block on timely arrival just before end of boost interval + spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT - 1) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert store.proposer_boost_root == spec.hash_tree_root(block) + assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) > 0 + + # Ensure that boost is removed after slot is over + spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT) + assert store.proposer_boost_root == spec.Root() + assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) == 0 + + next_slots(spec, state, 3) + block = build_empty_block_for_next_slot(spec, state) + signed_block = state_transition_and_sign_block(spec, state, block) + + # Process block on timely arrival at start of boost interval + spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT) + yield from tick_and_add_block(spec, store, signed_block, test_steps) + assert store.proposer_boost_root == spec.hash_tree_root(block) + assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) > 0 + + # Ensure that boost is removed after slot is over + spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT) + assert store.proposer_boost_root == spec.Root() + assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) == 0 + + yield 'steps', test_steps + + +@with_all_phases +@spec_state_test +def test_proposer_boost_root_same_slot_untimely_block(spec, state): test_steps = [] genesis_state = state.copy() @@ -726,6 +775,6 @@ def test_proposer_score_boost_same_slot_untimely_block(spec, state): spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT) yield from tick_and_add_block(spec, store, signed_block, test_steps) - assert store.proposer_score_boost.root == spec.Root() + assert store.proposer_boost_root == spec.Root() yield 'steps', test_steps From d85d4399cbaf397b1c3e895618ace2da83b933d3 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Mon, 22 Nov 2021 14:45:20 -0800 Subject: [PATCH 35/48] Rename test --- .../test/phase0/fork_choice/test_get_head.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py index bb9cbee2e2..fccf1d636d 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py @@ -271,7 +271,7 @@ def test_filtered_block_tree(spec, state): @with_all_phases @spec_state_test -def test_proposer_score_boost_basic(spec, state): +def test_proposer_boost_correct_head(spec, state): test_steps = [] genesis_state = state.copy() @@ -297,9 +297,8 @@ def test_proposer_score_boost_basic(spec, state): state_2 = genesis_state.copy() next_slots(spec, state_2, 2) block_2 = build_empty_block_for_next_slot(spec, state_2) - block_2.body.graffiti = spec.Bytes32(hex(random.getrandbits(8 * 32))[2:].zfill(64)) signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2) - while spec.hash_tree_root(block_1) > spec.hash_tree_root(block_2): + while spec.hash_tree_root(block_1) >= spec.hash_tree_root(block_2): block_2.body.graffiti = spec.Bytes32(hex(random.getrandbits(8 * 32))[2:].zfill(64)) signed_block_2 = state_transition_and_sign_block(spec, state_2.copy(), block_2) assert spec.hash_tree_root(block_1) < spec.hash_tree_root(block_2) @@ -309,19 +308,18 @@ def test_proposer_score_boost_basic(spec, state): # Process block_2 yield from tick_and_add_block(spec, store, signed_block_2, test_steps) - assert store.proposer_score_boost.root == spec.Root() + assert store.proposer_boost_root == spec.Root() assert spec.get_head(store) == spec.hash_tree_root(block_2) # Process block_1 on timely arrival # The head should temporarily change to block_1 yield from tick_and_add_block(spec, store, signed_block_1, test_steps) - assert store.proposer_score_boost == spec.LatestMessage(root=spec.hash_tree_root(block_1), - epoch=spec.compute_epoch_at_slot(block_1.slot)) + assert store.proposer_boost_root == spec.hash_tree_root(block_1) assert spec.get_head(store) == spec.hash_tree_root(block_1) # After block_1.slot, the head should revert to block_2 spec.on_tick(store, store.genesis_time + (block_1.slot + 1) * spec.config.SECONDS_PER_SLOT) - assert store.proposer_score_boost.root == spec.Root() + assert store.proposer_boost_root == spec.Root() assert spec.get_head(store) == spec.hash_tree_root(block_2) test_steps.append({ From 9999331f81167b0baa022bcfb8430821d45d6ef2 Mon Sep 17 00:00:00 2001 From: Ori Pomerantz Date: Mon, 22 Nov 2021 17:30:12 -0600 Subject: [PATCH 36/48] How to write tests for the consensus layer (#2700) * Create README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Signed my name * Update tests/README.md Co-authored-by: Danny Ryan * Update tests/README.md Co-authored-by: Danny Ryan * Update tests/README.md Co-authored-by: Danny Ryan * Update tests/README.md Co-authored-by: Danny Ryan * Update tests/README.md Co-authored-by: Danny Ryan * Update tests/README.md Co-authored-by: Danny Ryan * Update README.md * Update README.md Co-authored-by: Danny Ryan --- tests/README.md | 473 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 473 insertions(+) create mode 100644 tests/README.md diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000000..b45faef249 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,473 @@ +# Getting Started with Consensus Spec Tests + +## Getting Started + +### Creating the environment + +Use an OS that has Python 3.8 or above. For example, Debian 11 (bullseye) + +1. Install the packages you need: + ```sh + sudo apt install -y make git wget python3-venv gcc python3-dev + ``` +1. Download the latest [consensus specs](https://github.com/ethereum/consensus-specs) + ```sh + git clone https://github.com/ethereum/consensus-specs.git + cd consensus-specs + ``` +1. Create the specifications and tests: + ```sh + make install_test + make pyspec + ``` + +To read more about creating the environment, [see here](core/pyspec/README.md). + +### Running your first test + + +1. Enter the virtual Python environment: + ```sh + cd ~/consensus-specs + . venv/bin/activate + ``` +1. Run a sanity check test: + ```sh + cd tests/core/pyspec + python -m pytest -k test_empty_block_transition --fork Merge eth2spec + ``` +1. The output should be similar to: + ``` + ============================= test session starts ============================== + platform linux -- Python 3.9.2, pytest-6.2.5, py-1.10.0, pluggy-1.0.0 + rootdir: /home/qbzzt1/consensus-specs + plugins: cov-2.12.1, forked-1.3.0, xdist-2.3.0 + collected 629 items / 626 deselected / 3 selected + + eth2spec/test/merge/sanity/test_blocks.py . [ 33%] + eth2spec/test/phase0/sanity/test_blocks.py .. [100%] + + =============================== warnings summary =============================== + ../../../venv/lib/python3.9/site-packages/cytoolz/compatibility.py:2 + /home/qbzzt1/consensus-specs/venv/lib/python3.9/site-packages/cytoolz/compatibility.py:2: + DeprecationWarning: The toolz.compatibility module is no longer needed in Python 3 and has + been deprecated. Please import these utilities directly from the standard library. This + module will be removed in a future release. + warnings.warn("The toolz.compatibility module is no longer " + + -- Docs: https://docs.pytest.org/en/stable/warnings.html + ================ 3 passed, 626 deselected, 1 warning in 16.81s ================= + ``` + + +## The "Hello, World" of Consensus Spec Tests + +One of the `test_empty_block_transition` tests is implemented by a function with the same +name located in +[`~/consensus-specs/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py). +To learn how consensus spec tests are written, let's go over the code: + +```python +@with_all_phases +``` + +This [decorator](https://book.pythontips.com/en/latest/decorators.html) specifies that this test +is applicable to all the phases of consensus layer development. These phases are similar to forks (Istanbul, +Berlin, London, etc.) in the execution blockchain. If you are interested, [you can see the definition of +this decorator here](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/context.py#L331-L335). + +```python +@spec_state_test +``` + +[This decorator](https://github.com/qbzzt/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/context.py#L232-L234) specifies +that this test is a state transition test, and that it does not include a transition between different forks. + +```python +def test_empty_block_transition(spec, state): +``` + +This type of test receives two parameters: + +* `specs`: The protocol specifications +* `state`: The genesis state before the test + +```python + pre_slot = state.slot +``` + +A slot is a unit of time (every 12 seconds in mainnet), for which a specific validator (selected randomly but in a +deterministic manner) is a proposer. The proposer can propose a block during that slot. + +```python + pre_eth1_votes = len(state.eth1_data_votes) + pre_mix = spec.get_randao_mix(state, spec.get_current_epoch(state)) +``` + +Store some values to check later that certain updates happened. + +```python + yield 'pre', state +``` + +In Python `yield` is used by [generators](https://wiki.python.org/moin/Generators). However, for our purposes +we can treat it as a partial return statement that doesn't stop the function's processing, only adds to a list +of return values. Here we add two values, the string `'pre'` and the initial state, to the list of return values. + +[You can read more about test generators and how the are used here](generators). + +```python + block = build_empty_block_for_next_slot(spec, state) +``` + +The state contains the last block, which is necessary for building up the next block (every block needs to +have the hash of the previous one in a blockchain). + +```python + signed_block = state_transition_and_sign_block(spec, state, block) +``` + +Create a block signed by the appropriate proposer and advance the state. + +```python + yield 'blocks', [signed_block] + yield 'post', state +``` + +More `yield` statements. The output of a consensus test is: + +1. `'pre'` +2. The state before the test was run +3. `'blocks'` +4. A list of signed blocks +5. `'post'` +6. The state after the test + + + +```python + # One vote for the eth1 + assert len(state.eth1_data_votes) == pre_eth1_votes + 1 + + # Check that the new parent root is correct + assert spec.get_block_root_at_slot(state, pre_slot) == signed_block.message.parent_root + + # Random data changed + assert spec.get_randao_mix(state, spec.get_current_epoch(state)) != pre_mix +``` + +Finally we assertions that test the transition was legitimate. In this case we have three assertions: + +1. One item was added to `eth1_data_votes` +2. The new block's `parent_root` is the same as the block in the previous location +3. The random data that every block includes was changed. + + +## New Tests + +The easiest way to write a new test is to copy and modify an existing one. For example, +lets write a test where the first slot of the beacon chain is empty (because the assigned +proposer is offline, for example), and then there's an empty block in the second slot. + +We already know how to accomplish most of what we need for this test, but the only way we know +to advance the state is `state_transition_and_sign_block`, a function that also puts a block +into the slot. So let's see if the function's definition tells us how to advance the state without +a block. + +First, we need to find out where the function is located. Run: + +```sh +find . -name '*.py' -exec grep 'def state_transition_and_sign_block' {} \; -print +``` + +And you'll find that the function is defined in +[`eth2spec/test/helpers/state.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/helpers/state.py). Looking +in that file, we see that the second function is: + +```python +def next_slot(spec, state): + """ + Transition to the next slot. + """ + spec.process_slots(state, state.slot + 1) +``` + +This looks like exactly what we need. So we add this call before we create the empty block: + + +```python +. +. +. + yield 'pre', state + + next_slot(spec, state) + + block = build_empty_block_for_next_slot(spec, state) +. +. +. +``` + +That's it. Our new test works (copy `test_empty_block_transition`, rename it, add the `next_slot` call, and then run it to +verify this). + + + +## Tests Designed to Fail + +It is important to make sure that the system rejects invalid input, so our next step is to deal with cases where the protocol +is supposed to reject something. To see such a test, look at `test_prev_slot_block_transition` (in the same +file we used previously, +[`~/consensus-specs/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py)). + +```python +@with_all_phases +@spec_state_test +def test_prev_slot_block_transition(spec, state): + spec.process_slots(state, state.slot + 1) + block = build_empty_block(spec, state, slot=state.slot) +``` + +Build an empty block for the current slot. + +```python + proposer_index = spec.get_beacon_proposer_index(state) +``` + +Get the identity of the current proposer, the one for *this* slot. + +```python + spec.process_slots(state, state.slot + 1) +``` + +Transition to the new slot, which naturally has a different proposer. + +```python + yield 'pre', state + expect_assertion_error(lambda: transition_unsigned_block(spec, state, block)) +``` + +Specify that the function `transition_unsigned_block` will cause an assertion error. +You can see this function in +[`~/consensus-specs/tests/core/pyspec/eth2spec/test/helpers/block.py`](https://github.com/ethereum/consensus-specs/blob/dev/tests/core/pyspec/eth2spec/test/helpers/block.py), +and one of the tests is that the block must be for this slot: +> ```python +> assert state.slot == block.slot +> ``` + +Because we use [lambda notation](https://www.w3schools.com/python/python_lambda.asp), the test +does not call `transition_unsigned_block` here. Instead, this is a function parameter that can +be called later. + +```python + block.state_root = state.hash_tree_root() +``` + +Set the block's state root to the current state hash tree root, which identifies this block as +belonging to this slot (even though it was created for the previous slot). + +```python + signed_block = sign_block(spec, state, block, proposer_index=proposer_index) +``` + +Notice that `proposer_index` is the variable we set earlier, *before* we advanced +the slot with `spec.process_slots(state, state.slot + 1)`. It is not the proposer +for the current state. + +```python + yield 'blocks', [signed_block] + yield 'post', None # No post state, signifying it errors out +``` + +This is the way we specify that a test is designed to fail - failed tests have no post state, +because the processing mechanism errors out before creating it. + + +## Attestation Tests + +The consensus layer doesn't provide any direct functionality to end users. It does +not execute EVM programs or store user data. It exists to provide a secure source of +information about the latest verified block hash of the execution layer. + +For every slot a validator is randomly selected as the proposer. The proposer proposes a block +for the current head of the consensus layer chain (built on the previous block). That block +includes the hash of the proposed new head of the execution layer. + +For every slot there is also a randomly selected committee of validators that needs to vote whether +the new consensus layer block is valid, which requires the proposed head of the execution chain to +also be a valid block. These votes are called [attestations](https://notes.ethereum.org/@hww/aggregation#112-Attestation), +and they are sent as independent messages. The proposer for a block is able to include attestations from previous slots, +which is how they get on chain to form consensus, reward honest validators, etc. + +[You can see a simple successful attestation test here](https://github.com/ethereum/consensus-specs/blob/926e5a3d722df973b9a12f12c015783de35cafa9/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py#L26-L30): +Lets go over it line by line. + + +```python +@with_all_phases +@spec_state_test +def test_success(spec, state): + attestation = get_valid_attestation(spec, state, signed=True) +``` + +[This function](https://github.com/ethereum/consensus-specs/blob/30fe7ba1107d976100eb0c3252ca7637b791e43a/tests/core/pyspec/eth2spec/test/helpers/attestations.py#L88-L120) +creates a valid attestation (which can then be modified to make it invalid if needed). +To see an attestion "from the inside" we need to follow it. + + +> ```python +> def get_valid_attestation(spec, +> state, +> slot=None, +> index=None, +> filter_participant_set=None, +> signed=False): +> ``` +> +> Only two parameters, `spec` and `state` are required. However, there are four other parameters that can affect +> the attestation created by this function. +> +> +> ```python +> # If filter_participant_set filters everything, the attestation has 0 participants, and cannot be signed. +> # Thus strictly speaking invalid when no participant is added later. +> if slot is None: +> slot = state.slot +> if index is None: +> index = 0 +> ``` +> +> Default values. Normally we want to choose the current slot, and out of the proposers and committees that it can have, +> we want the first one. +> +> ```python +> attestation_data = build_attestation_data( +> spec, state, slot=slot, index=index +> ) +> ``` +> +> Build the actual attestation. You can see this function +> [here](https://github.com/ethereum/consensus-specs/blob/30fe7ba1107d976100eb0c3252ca7637b791e43a/tests/core/pyspec/eth2spec/test/helpers/attestations.py#L53-L85) +> to see the exact data in an attestation. +> +> ```python +> beacon_committee = spec.get_beacon_committee( +> state, +> attestation_data.slot, +> attestation_data.index, +> ) +> ``` +> +> This is the committee that is supposed to approve or reject the proposed block. +> +> ```python +> +> committee_size = len(beacon_committee) +> aggregation_bits = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE](*([0] * committee_size)) +> ``` +> +> There's a bit for every committee member to see if it approves or not. +> +> ```python +> attestation = spec.Attestation( +> aggregation_bits=aggregation_bits, +> data=attestation_data, +> ) +> # fill the attestation with (optionally filtered) participants, and optionally sign it +> fill_aggregate_attestation(spec, state, attestation, signed=signed, filter_participant_set=filter_participant_set) +> +> return attestation +> ``` + +```python + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) +``` + +Attestations have to appear after the block they attest for, so we advance +`spec.MIN_ATTESTATION_INCLUSION_DELAY` slots before creating the block that includes the attestation. +Currently a single block is sufficient, but that may change in the future. + +```python + yield from run_attestation_processing(spec, state, attestation) +``` + +[This function](https://github.com/ethereum/consensus-specs/blob/30fe7ba1107d976100eb0c3252ca7637b791e43a/tests/core/pyspec/eth2spec/test/helpers/attestations.py#L13-L50) +processes the attestation and returns the result. + + +### Adding an Attestation Test + +Attestations can't happen in the same block as the one about which they are attesting, or in a block that is +after the block is finalized. This is specified as part of the specs, in the `process_attestation` function +(which is created from the spec by the `make pyspec` command you ran earlier). Here is the relevant code +fragment: + + +```python +def process_attestation(state: BeaconState, attestation: Attestation) -> None: + data = attestation.data + assert data.target.epoch in (get_previous_epoch(state), get_current_epoch(state)) + assert data.target.epoch == compute_epoch_at_slot(data.slot) + assert data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot <= data.slot + SLOTS_PER_EPOCH + ... +``` + +In the last line you can see two conditions being asserted: + +1. `data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot` which verifies that the attestation doesn't + arrive too early. +1. `state.slot <= data.slot + SLOTS_PER_EPOCH` which verifies that the attestation doesn't + arrive too late. + +This is how the consensus layer tests deal with edge cases, by asserting the conditions required for the +values to be legitimate. In the case of these particular conditions, they are tested +[here](https://github.com/ethereum/consensus-specs/blob/926e5a3d722df973b9a12f12c015783de35cafa9/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py#L87-L104). +One test checks what happens if the attestation is too early, and another if it is too late. + +However, it is not enough to ensure we reject invalid blocks. It is also necessary to ensure we accept all valid blocks. You saw earlier +a test (`test_success`) that tested that being `MIN_ATTESTATION_INCLUSION_DELAY` after the data for which we attest is enough. +Now we'll write a similar test that verifies that being `SLOTS_PER_EPOCH` away is still valid. To do this, we modify the +`test_after_epoch_slots` function. We need two changes: + +1. Call `transition_to_slot_via_block` with one less slot to advance +1. Don't tell `run_attestation_processing` to return an empty post state. + +The modified function is: + +```python +@with_all_phases +@spec_state_test +def test_almost_after_epoch_slots(spec, state): + attestation = get_valid_attestation(spec, state, signed=True) + + # increment to latest inclusion slot (not beyond it) + transition_to_slot_via_block(spec, state, state.slot + spec.SLOTS_PER_EPOCH) + + yield from run_attestation_processing(spec, state, attestation) +``` + +Add this function to the file `consensus-specs/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py`, +and run the test: + +```sh +cd ~/consensus-specs +. venv/bin/activate +cd tests/core/pyspec +python -m pytest -k almost_after --fork Merge eth2spec +``` + +You should see it ran successfully (although you might get a warning, you can ignore it) + +## How are These Tests Used? + +So far we've ran tests against the formal specifications. This is a way to check the specifications +are what we expect, but it doesn't actually check the beacon chain clients. The way these tests get applied +by clients is that every few weeks +[new test specifications are released](https://github.com/ethereum/consensus-spec-tests/releases), +in a format [documented here](https://github.com/ethereum/consensus-specs/tree/dev/tests/formats). +All the consensus layer clients implement test-runners that consume the test vectors in this standard format. + +--- + +Original version by [Ori Pomerantz](mailto:qbzzt1@gmail.com) From 64b4ca2950a81076f5904fd111fe639ef3bf6749 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 22 Nov 2021 17:02:46 -0700 Subject: [PATCH 37/48] add PROPOSER_SCORE_BOOST to configuration yaml files --- configs/mainnet.yaml | 5 +++++ configs/minimal.yaml | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 6d23073b7f..ce26970541 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -70,6 +70,11 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 CHURN_LIMIT_QUOTIENT: 65536 +# Fork choice +# --------------------------------------------------------------- +# 25% +PROPOSER_SCORE_BOOST: 25 + # Deposit contract # --------------------------------------------------------------- # Ethereum PoW Mainnet diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 4f99fce313..ef35b3a070 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -69,6 +69,12 @@ MIN_PER_EPOCH_CHURN_LIMIT: 4 CHURN_LIMIT_QUOTIENT: 32 +# Fork choice +# --------------------------------------------------------------- +# 25% +PROPOSER_SCORE_BOOST: 25 + + # Deposit contract # --------------------------------------------------------------- # Ethereum Goerli testnet From bdd7b0782ec72afd59e3cd188712b36f93aab863 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 23 Nov 2021 16:26:11 +0800 Subject: [PATCH 38/48] Add configuration value checks --- .../test/phase0/unittests/test_config_invariants.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py b/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py index 3183904c8b..8836d463ea 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py +++ b/tests/core/pyspec/eth2spec/test/phase0/unittests/test_config_invariants.py @@ -74,3 +74,10 @@ def test_time(spec, state): @spec_state_test def test_networking(spec, state): assert spec.RANDOM_SUBNETS_PER_VALIDATOR <= spec.ATTESTATION_SUBNET_COUNT + + +@with_all_phases +@spec_state_test +def test_fork_choice(spec, state): + assert spec.INTERVALS_PER_SLOT < spec.config.SECONDS_PER_SLOT + assert spec.config.PROPOSER_SCORE_BOOST <= 100 From a0b5a809d5513875ad7e2fcffab63110918eeece Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Tue, 23 Nov 2021 07:02:04 -0800 Subject: [PATCH 39/48] Apply HWW code's review - fix is_before_attesting_interval --- specs/merge/fork-choice.md | 3 ++- specs/phase0/fork-choice.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index cbfe33f4a6..dc392d2639 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -176,7 +176,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT if get_current_slot(store) == block.slot and is_before_attesting_interval: store.proposer_boost_root = hash_tree_root(block) diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index 6e50b99d4b..b6b7b0c838 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -414,7 +414,8 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: store.block_states[hash_tree_root(block)] = state # Add proposer score boost if the block is timely - is_before_attesting_interval = store.time % SECONDS_PER_SLOT < SECONDS_PER_SLOT // INTERVALS_PER_SLOT + time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT + is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT if get_current_slot(store) == block.slot and is_before_attesting_interval: store.proposer_boost_root = hash_tree_root(block) From 44fec310b370426ab070aacc4089544b9a6d9b16 Mon Sep 17 00:00:00 2001 From: Ben Edgington Date: Tue, 23 Nov 2021 15:20:26 +0000 Subject: [PATCH 40/48] Eth1 block hashes have type Hash32 (#2693) * Eth1 block hashes have type Hash32 * Modify other forks as well Co-authored-by: Hsiao-Wei Wang --- specs/altair/beacon-chain.md | 2 +- specs/merge/beacon-chain.md | 2 +- specs/phase0/beacon-chain.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/specs/altair/beacon-chain.md b/specs/altair/beacon-chain.md index b22f89a4db..3c7177a1b4 100644 --- a/specs/altair/beacon-chain.md +++ b/specs/altair/beacon-chain.md @@ -685,7 +685,7 @@ This helper function is only for initializing the state for pure Altair testnets *Note*: The function `initialize_beacon_state_from_eth1` is modified: (1) using `ALTAIR_FORK_VERSION` as the current fork version, (2) utilizing the Altair `BeaconBlockBody` when constructing the initial `latest_block_header`, and (3) adding initial sync committees. ```python -def initialize_beacon_state_from_eth1(eth1_block_hash: Bytes32, +def initialize_beacon_state_from_eth1(eth1_block_hash: Hash32, eth1_timestamp: uint64, deposits: Sequence[Deposit]) -> BeaconState: fork = Fork( diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index ac5b6d0ecf..661d333faa 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -406,7 +406,7 @@ Modifications include: Else, the Merge starts from genesis and the transition is incomplete. ```python -def initialize_beacon_state_from_eth1(eth1_block_hash: Bytes32, +def initialize_beacon_state_from_eth1(eth1_block_hash: Hash32, eth1_timestamp: uint64, deposits: Sequence[Deposit], execution_payload_header: ExecutionPayloadHeader=ExecutionPayloadHeader() diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index e2e235acf1..1618aa37c1 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -1175,7 +1175,7 @@ Before the Ethereum beacon chain genesis has been triggered, and for every Ether Proof-of-work blocks must only be considered once they are at least `SECONDS_PER_ETH1_BLOCK * ETH1_FOLLOW_DISTANCE` seconds old (i.e. `eth1_timestamp + SECONDS_PER_ETH1_BLOCK * ETH1_FOLLOW_DISTANCE <= current_unix_time`). Due to this constraint, if `GENESIS_DELAY < SECONDS_PER_ETH1_BLOCK * ETH1_FOLLOW_DISTANCE`, then the `genesis_time` can happen before the time/state is first known. Values should be configured to avoid this case. ```python -def initialize_beacon_state_from_eth1(eth1_block_hash: Bytes32, +def initialize_beacon_state_from_eth1(eth1_block_hash: Hash32, eth1_timestamp: uint64, deposits: Sequence[Deposit]) -> BeaconState: fork = Fork( From ecbe9190b942a45f5e7eb814dfc84ade3f4269ac Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Tue, 23 Nov 2021 07:20:54 -0800 Subject: [PATCH 41/48] Apply HWW code's review - properly update test steps --- .../test/phase0/fork_choice/test_get_head.py | 10 ++++--- .../test/phase0/fork_choice/test_on_block.py | 29 +++++++++++-------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py index fccf1d636d..83e342e08a 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py @@ -304,21 +304,23 @@ def test_proposer_boost_correct_head(spec, state): assert spec.hash_tree_root(block_1) < spec.hash_tree_root(block_2) # Tick to block_1 slot time - spec.on_tick(store, store.genesis_time + block_1.slot * spec.config.SECONDS_PER_SLOT) + time = store.genesis_time + block_1.slot * spec.config.SECONDS_PER_SLOT + on_tick_and_append_step(spec, store, time, test_steps) # Process block_2 - yield from tick_and_add_block(spec, store, signed_block_2, test_steps) + yield from add_block(spec, store, signed_block_2, test_steps) assert store.proposer_boost_root == spec.Root() assert spec.get_head(store) == spec.hash_tree_root(block_2) # Process block_1 on timely arrival # The head should temporarily change to block_1 - yield from tick_and_add_block(spec, store, signed_block_1, test_steps) + yield from add_block(spec, store, signed_block_1, test_steps) assert store.proposer_boost_root == spec.hash_tree_root(block_1) assert spec.get_head(store) == spec.hash_tree_root(block_1) # After block_1.slot, the head should revert to block_2 - spec.on_tick(store, store.genesis_time + (block_1.slot + 1) * spec.config.SECONDS_PER_SLOT) + time = store.genesis_time + (block_1.slot + 1) * spec.config.SECONDS_PER_SLOT + on_tick_and_append_step(spec, store, time, test_steps) assert store.proposer_boost_root == spec.Root() assert spec.get_head(store) == spec.hash_tree_root(block_2) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index f234125e04..5c3ba89ded 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -723,15 +723,17 @@ def test_proposer_boost(spec, state): signed_block = state_transition_and_sign_block(spec, state, block) # Process block on timely arrival just before end of boost interval - spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + - spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT - 1) - yield from tick_and_add_block(spec, store, signed_block, test_steps) + time = (store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT - 1) + on_tick_and_append_step(spec, store, time, test_steps) + yield from add_block(spec, store, signed_block, test_steps) assert store.proposer_boost_root == spec.hash_tree_root(block) assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) > 0 # Ensure that boost is removed after slot is over - spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + - spec.config.SECONDS_PER_SLOT) + time = (store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT) + on_tick_and_append_step(spec, store, time, test_steps) assert store.proposer_boost_root == spec.Root() assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) == 0 @@ -740,14 +742,16 @@ def test_proposer_boost(spec, state): signed_block = state_transition_and_sign_block(spec, state, block) # Process block on timely arrival at start of boost interval - spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT) - yield from tick_and_add_block(spec, store, signed_block, test_steps) + time = (store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT) + on_tick_and_append_step(spec, store, time, test_steps) + yield from add_block(spec, store, signed_block, test_steps) assert store.proposer_boost_root == spec.hash_tree_root(block) assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) > 0 # Ensure that boost is removed after slot is over - spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + - spec.config.SECONDS_PER_SLOT) + time = (store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT) + on_tick_and_append_step(spec, store, time, test_steps) assert store.proposer_boost_root == spec.Root() assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) == 0 @@ -772,9 +776,10 @@ def test_proposer_boost_root_same_slot_untimely_block(spec, state): signed_block = state_transition_and_sign_block(spec, state, block) # Process block on untimely arrival in the same slot - spec.on_tick(store, store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + - spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT) - yield from tick_and_add_block(spec, store, signed_block, test_steps) + time = (store.genesis_time + block.slot * spec.config.SECONDS_PER_SLOT + + spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT) + on_tick_and_append_step(spec, store, time, test_steps) + yield from add_block(spec, store, signed_block, test_steps) assert store.proposer_boost_root == spec.Root() yield 'steps', test_steps From 2a5c9d8dc5560a8ec5e784b4ed43569d7443d655 Mon Sep 17 00:00:00 2001 From: Aditya Asgaonkar Date: Tue, 23 Nov 2021 07:23:59 -0800 Subject: [PATCH 42/48] Set PROPOSER_SCORE_BOOST to 70% --- configs/mainnet.yaml | 2 +- configs/minimal.yaml | 2 +- specs/phase0/fork-choice.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index ce26970541..0a207b423e 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -73,7 +73,7 @@ CHURN_LIMIT_QUOTIENT: 65536 # Fork choice # --------------------------------------------------------------- # 25% -PROPOSER_SCORE_BOOST: 25 +PROPOSER_SCORE_BOOST: 70 # Deposit contract # --------------------------------------------------------------- diff --git a/configs/minimal.yaml b/configs/minimal.yaml index ef35b3a070..508587ee53 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -72,7 +72,7 @@ CHURN_LIMIT_QUOTIENT: 32 # Fork choice # --------------------------------------------------------------- # 25% -PROPOSER_SCORE_BOOST: 25 +PROPOSER_SCORE_BOOST: 70 # Deposit contract diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md index b6b7b0c838..d082ede306 100644 --- a/specs/phase0/fork-choice.md +++ b/specs/phase0/fork-choice.md @@ -75,7 +75,7 @@ Any of the above handlers that trigger an unhandled exception (e.g. a failed ass | Name | Value | | - | - | -| `PROPOSER_SCORE_BOOST` | `uint64(25)` | +| `PROPOSER_SCORE_BOOST` | `uint64(70)` | - The proposer score boost is worth `PROPOSER_SCORE_BOOST` percentage of the committee's weight, i.e., for slot with committee weight `committee_weight` the boost weight is equal to `(committee_weight * PROPOSER_SCORE_BOOST) // 100`. From 2ba0586c3de2e6bf1d2763366c822e923269dc40 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 Nov 2021 00:09:48 +0800 Subject: [PATCH 43/48] Add `proposer_boost_root` field to test vector "checks" step --- tests/core/pyspec/eth2spec/test/helpers/fork_choice.py | 1 + tests/formats/fork_choice/README.md | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 098c05bf9d..2fe0de90d7 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -157,6 +157,7 @@ def add_block(spec, 'epoch': int(store.best_justified_checkpoint.epoch), 'root': encode_hex(store.best_justified_checkpoint.root), }, + 'proposer_boost_root': encode_hex(store.proposer_boost_root), } }) diff --git a/tests/formats/fork_choice/README.md b/tests/formats/fork_choice/README.md index 48dde2fb11..e4da31a9b3 100644 --- a/tests/formats/fork_choice/README.md +++ b/tests/formats/fork_choice/README.md @@ -110,6 +110,7 @@ best_justified_checkpoint: { epoch: int, -- Integer value from store.best_justified_checkpoint.epoch root: string, -- Encoded 32-byte value from store.best_justified_checkpoint.root } +proposer_boost_root: string -- Encoded 32-byte value from store.proposer_boost_root ``` For example: @@ -120,6 +121,7 @@ For example: justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'} finalized_checkpoint: {epoch: 2, root: '0x40d32d6283ec11c53317a46808bc88f55657d93b95a1af920403187accf48f4f'} best_justified_checkpoint: {epoch: 3, root: '0xc25faab4acab38d3560864ca01e4d5cc4dc2cd473da053fbc03c2669143a2de4'} + proposer_boost_root: '0xdaa1d49d57594ced0c35688a6da133abb086d191a2ebdfd736fad95299325aeb' ``` *Note*: Each `checks` step may include one or multiple items. Each item has to be checked against the current store. From 975931b5cfc26f643221c3e043d23de539dd60e7 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 23 Nov 2021 11:19:22 -0700 Subject: [PATCH 44/48] pr feedback --- .../test/phase0/fork_choice/test_get_head.py | 7 ++++--- .../test/phase0/fork_choice/test_on_block.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py index 83e342e08a..d2c84fce79 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_get_head.py @@ -117,10 +117,11 @@ def test_split_tie_breaker_no_attestations(spec, state): signed_block_2 = state_transition_and_sign_block(spec, block_2_state, block_2) # Tick time past slot 1 so proposer score boost does not apply - spec.on_tick(store, store.genesis_time + (block_2.slot + 1) * spec.config.SECONDS_PER_SLOT) + time = store.genesis_time + (block_2.slot + 1) * spec.config.SECONDS_PER_SLOT + on_tick_and_append_step(spec, store, time, test_steps) - yield from tick_and_add_block(spec, store, signed_block_1, test_steps) - yield from tick_and_add_block(spec, store, signed_block_2, test_steps) + yield from add_block(spec, store, signed_block_1, test_steps) + yield from add_block(spec, store, signed_block_2, test_steps) highest_root = max(spec.hash_tree_root(block_1), spec.hash_tree_root(block_2)) assert spec.get_head(store) == highest_root diff --git a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py index 5c3ba89ded..f57522ad76 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py +++ b/tests/core/pyspec/eth2spec/test/phase0/fork_choice/test_on_block.py @@ -1,4 +1,5 @@ import random +from eth_utils import encode_hex from eth2spec.utils.ssz.ssz_impl import hash_tree_root from eth2spec.test.context import MINIMAL, spec_state_test, with_all_phases, with_presets @@ -755,6 +756,12 @@ def test_proposer_boost(spec, state): assert store.proposer_boost_root == spec.Root() assert spec.get_latest_attesting_balance(store, spec.hash_tree_root(block)) == 0 + test_steps.append({ + 'checks': { + 'proposer_boost_root': encode_hex(store.proposer_boost_root), + } + }) + yield 'steps', test_steps @@ -780,6 +787,13 @@ def test_proposer_boost_root_same_slot_untimely_block(spec, state): spec.config.SECONDS_PER_SLOT // spec.INTERVALS_PER_SLOT) on_tick_and_append_step(spec, store, time, test_steps) yield from add_block(spec, store, signed_block, test_steps) + assert store.proposer_boost_root == spec.Root() + test_steps.append({ + 'checks': { + 'proposer_boost_root': encode_hex(store.proposer_boost_root), + } + }) + yield 'steps', test_steps From 8050de384ee388cfbf4372a767474507502eb579 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Wed, 24 Nov 2021 02:19:43 +0800 Subject: [PATCH 45/48] Fix spec typo and add `test_prepare_execution_payload` unit tests --- specs/merge/validator.md | 2 +- .../unittests/validator/test_validator.py | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 27fb263528..dcb5a986f9 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -125,7 +125,7 @@ def prepare_execution_payload(state: BeaconState, execution_engine: ExecutionEngine) -> Optional[PayloadId]: if not is_merge_complete(state): is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() - is_activation_epoch_reached = get_current_epoch(state.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH + is_activation_epoch_reached = get_current_epoch(state) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH if is_terminal_block_hash_set and not is_activation_epoch_reached: # Terminal block hash is set but activation epoch is not yet reached, no prepare payload call is needed return None diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py b/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py index 9b710d2ead..d4acf04a65 100644 --- a/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/validator/test_validator.py @@ -1,3 +1,5 @@ +from copy import deepcopy + from eth2spec.test.helpers.pow_block import ( prepare_random_pow_chain, ) @@ -62,3 +64,99 @@ def test_get_pow_block_at_terminal_total_difficulty(spec, state): assert pow_block is None else: raise Exception('Something is wrong') + + +SAMPLE_PAYLOAD_ID = b'\x12' * 8 +# ('is_merge_complete', 'is_terminal_block_hash_set', 'is_activation_epoch_reached', +# 'terminal_pow_block_is_none', 'result_payload_id') +prepare_execution_payload_expected_results = [ + (False, False, False, False, SAMPLE_PAYLOAD_ID), + (False, False, False, True, None), + (False, False, True, False, SAMPLE_PAYLOAD_ID), + (False, False, True, True, None), + (False, True, False, False, None), + (False, True, False, True, None), + (False, True, True, False, SAMPLE_PAYLOAD_ID), + (False, True, True, True, None), + (True, False, False, False, SAMPLE_PAYLOAD_ID), + (True, False, False, True, SAMPLE_PAYLOAD_ID), + (True, False, True, False, SAMPLE_PAYLOAD_ID), + (True, False, True, True, SAMPLE_PAYLOAD_ID), + (True, True, False, False, SAMPLE_PAYLOAD_ID), + (True, True, False, True, SAMPLE_PAYLOAD_ID), + (True, True, True, False, SAMPLE_PAYLOAD_ID), + (True, True, True, True, SAMPLE_PAYLOAD_ID), +] + + +@with_merge_and_later +@spec_state_test +def test_prepare_execution_payload(spec, state): + for result in prepare_execution_payload_expected_results: + ( + is_merge_complete, + is_terminal_block_hash_set, + is_activation_epoch_reached, + terminal_pow_block_is_none, + result_payload_id, + ) = result + + # 1. Handle `is_merge_complete` + if is_merge_complete: + state.latest_execution_payload_header = spec.ExecutionPayloadHeader(random=b'\x12' * 32) + else: + state.latest_execution_payload_header = spec.ExecutionPayloadHeader() + + # 2. `is_terminal_block_hash_set` and `is_activation_epoch_reached` require mocking configs in runtime + config_overrides = {} + _mock_terminal_block_hash = b'\x34' * 32 + if is_terminal_block_hash_set: + config_overrides['TERMINAL_BLOCK_HASH'] = _mock_terminal_block_hash + else: + config_overrides['TERMINAL_BLOCK_HASH'] = spec.Hash32() + + # Default `TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH` is too big and too close to overflow + _mock_terminal_block_hash_activation_epoch = 3 + config_overrides['TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH'] = _mock_terminal_block_hash_activation_epoch + if is_activation_epoch_reached: + state.slot = _mock_terminal_block_hash_activation_epoch * spec.SLOTS_PER_EPOCH + else: + state.slot = (_mock_terminal_block_hash_activation_epoch - 1) * spec.SLOTS_PER_EPOCH + + # Logic from `with_config_overrides` + old_config = spec.config + tmp_config = deepcopy(old_config._asdict()) + tmp_config.update(config_overrides) + config_types = spec.Configuration.__annotations__ + test_config = {k: config_types[k](v) for k, v in tmp_config.items()} + spec.config = spec.Configuration(**test_config) + + # 3. Handle `terminal_pow_block_is_none` + pow_chain = prepare_random_pow_chain(spec, 2) + if terminal_pow_block_is_none: + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY - 1 + else: + if is_terminal_block_hash_set: + pow_chain.head().block_hash = _mock_terminal_block_hash + pow_chain.head().total_difficulty = spec.config.TERMINAL_TOTAL_DIFFICULTY + + # Dummy arguments + finalized_block_hash = b'\x56' * 32 + suggested_fee_recipient = b'\x78' * 20 + + # Mock execution_engine + class TestEngine(spec.NoopExecutionEngine): + def notify_forkchoice_updated(self, parent_hash, finalized_block_hash, payload_attributes) -> bool: + return SAMPLE_PAYLOAD_ID + + payload_id = spec.prepare_execution_payload( + state=state, + pow_chain=pow_chain.to_dict(), + finalized_block_hash=finalized_block_hash, + suggested_fee_recipient=suggested_fee_recipient, + execution_engine=TestEngine(), + ) + assert payload_id == result_payload_id + + # Restore config + spec.config = old_config From a0d008bb869d822a9f87933ea9f2e51935d21770 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 23 Nov 2021 14:31:35 -0700 Subject: [PATCH 46/48] is_merge -> is_merge_transition --- specs/merge/beacon-chain.md | 18 +++++++++--------- specs/merge/fork-choice.md | 2 +- specs/merge/validator.md | 2 +- .../eth2spec/test/helpers/fork_choice.py | 2 +- .../test/merge/genesis/test_initialization.py | 6 +++--- .../test/merge/unittests/test_transition.py | 10 +++++----- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/specs/merge/beacon-chain.md b/specs/merge/beacon-chain.md index 661d333faa..5b818d5a26 100644 --- a/specs/merge/beacon-chain.md +++ b/specs/merge/beacon-chain.md @@ -24,8 +24,8 @@ - [`ExecutionPayloadHeader`](#executionpayloadheader) - [Helper functions](#helper-functions) - [Predicates](#predicates) - - [`is_merge_complete`](#is_merge_complete) - - [`is_merge_block`](#is_merge_block) + - [`is_merge_transition_complete`](#is_merge_transition_complete) + - [`is_merge_transition_block`](#is_merge_transition_block) - [`is_execution_enabled`](#is_execution_enabled) - [Misc](#misc) - [`compute_timestamp_at_slot`](#compute_timestamp_at_slot) @@ -209,25 +209,25 @@ class ExecutionPayloadHeader(Container): ### Predicates -#### `is_merge_complete` +#### `is_merge_transition_complete` ```python -def is_merge_complete(state: BeaconState) -> bool: +def is_merge_transition_complete(state: BeaconState) -> bool: return state.latest_execution_payload_header != ExecutionPayloadHeader() ``` -#### `is_merge_block` +#### `is_merge_transition_block` ```python -def is_merge_block(state: BeaconState, body: BeaconBlockBody) -> bool: - return not is_merge_complete(state) and body.execution_payload != ExecutionPayload() +def is_merge_transition_block(state: BeaconState, body: BeaconBlockBody) -> bool: + return not is_merge_transition_complete(state) and body.execution_payload != ExecutionPayload() ``` #### `is_execution_enabled` ```python def is_execution_enabled(state: BeaconState, body: BeaconBlockBody) -> bool: - return is_merge_block(state, body) or is_merge_complete(state) + return is_merge_transition_block(state, body) or is_merge_transition_complete(state) ``` ### Misc @@ -346,7 +346,7 @@ def process_block(state: BeaconState, block: BeaconBlock) -> None: ```python def process_execution_payload(state: BeaconState, payload: ExecutionPayload, execution_engine: ExecutionEngine) -> None: # Verify consistency of the parent hash with respect to the previous execution payload header - if is_merge_complete(state): + if is_merge_transition_complete(state): assert payload.parent_hash == state.latest_execution_payload_header.block_hash # Verify random assert payload.random == get_randao_mix(state, get_current_epoch(state)) diff --git a/specs/merge/fork-choice.md b/specs/merge/fork-choice.md index 8c5bc13128..c4aa821632 100644 --- a/specs/merge/fork-choice.md +++ b/specs/merge/fork-choice.md @@ -167,7 +167,7 @@ def on_block(store: Store, signed_block: SignedBeaconBlock) -> None: state_transition(state, signed_block, True) # [New in Merge] - if is_merge_block(pre_state, block.body): + if is_merge_transition_block(pre_state, block.body): validate_merge_block(block) # Add new block to the store diff --git a/specs/merge/validator.md b/specs/merge/validator.md index 27fb263528..12f003f441 100644 --- a/specs/merge/validator.md +++ b/specs/merge/validator.md @@ -123,7 +123,7 @@ def prepare_execution_payload(state: BeaconState, finalized_block_hash: Hash32, suggested_fee_recipient: ExecutionAddress, execution_engine: ExecutionEngine) -> Optional[PayloadId]: - if not is_merge_complete(state): + if not is_merge_transition_complete(state): is_terminal_block_hash_set = TERMINAL_BLOCK_HASH != Hash32() is_activation_epoch_reached = get_current_epoch(state.slot) >= TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH if is_terminal_block_hash_set and not is_activation_epoch_reached: diff --git a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py index 098c05bf9d..39fafd388e 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py +++ b/tests/core/pyspec/eth2spec/test/helpers/fork_choice.py @@ -28,7 +28,7 @@ def tick_and_add_block(spec, store, signed_block, test_steps, valid=True, pre_state = store.block_states[signed_block.message.parent_root] block_time = pre_state.genesis_time + signed_block.message.slot * spec.config.SECONDS_PER_SLOT if merge_block: - assert spec.is_merge_block(pre_state, signed_block.message.body) + assert spec.is_merge_transition_block(pre_state, signed_block.message.body) if store.time < block_time: on_tick_and_append_step(spec, store, block_time, test_steps) diff --git a/tests/core/pyspec/eth2spec/test/merge/genesis/test_initialization.py b/tests/core/pyspec/eth2spec/test/merge/genesis/test_initialization.py index 78462263b2..9cd388698d 100644 --- a/tests/core/pyspec/eth2spec/test/merge/genesis/test_initialization.py +++ b/tests/core/pyspec/eth2spec/test/merge/genesis/test_initialization.py @@ -45,7 +45,7 @@ def test_initialize_pre_transition_no_param(spec): yield 'execution_payload_header', 'meta', False state = spec.initialize_beacon_state_from_eth1(eth1_block_hash, eth1_timestamp, deposits) - assert not spec.is_merge_complete(state) + assert not spec.is_merge_transition_complete(state) yield 'state', state @@ -79,7 +79,7 @@ def test_initialize_pre_transition_empty_payload(spec): execution_payload_header=execution_payload_header, ) - assert not spec.is_merge_complete(state) + assert not spec.is_merge_transition_complete(state) yield 'execution_payload_header', execution_payload_header @@ -117,6 +117,6 @@ def test_initialize_post_transition(spec): yield 'execution_payload_header', genesis_execution_payload_header - assert spec.is_merge_complete(state) + assert spec.is_merge_transition_complete(state) yield 'state', state diff --git a/tests/core/pyspec/eth2spec/test/merge/unittests/test_transition.py b/tests/core/pyspec/eth2spec/test/merge/unittests/test_transition.py index 05d3888daa..9c3b403ee1 100644 --- a/tests/core/pyspec/eth2spec/test/merge/unittests/test_transition.py +++ b/tests/core/pyspec/eth2spec/test/merge/unittests/test_transition.py @@ -13,17 +13,17 @@ @spec_state_test def test_fail_merge_complete(spec, state): state = build_state_with_incomplete_transition(spec, state) - assert not spec.is_merge_complete(state) + assert not spec.is_merge_transition_complete(state) @with_merge_and_later @spec_state_test def test_success_merge_complete(spec, state): state = build_state_with_complete_transition(spec, state) - assert spec.is_merge_complete(state) + assert spec.is_merge_transition_complete(state) -# with_complete_transition', 'with_execution_payload', 'is_merge_block', 'is_execution_enabled' +# with_complete_transition', 'with_execution_payload', 'is_merge_transition_block', 'is_execution_enabled' expected_results = [ (True, True, False, True), (True, False, False, True), @@ -39,7 +39,7 @@ def test_is_merge_block_and_is_execution_enabled(spec, state): ( with_complete_transition, with_execution_payload, - is_merge_block, + is_merge_transition_block, is_execution_enabled ) = result if with_complete_transition: @@ -51,5 +51,5 @@ def test_is_merge_block_and_is_execution_enabled(spec, state): if with_execution_payload: body.execution_payload = build_empty_execution_payload(spec, state) - assert spec.is_merge_block(state, body) == is_merge_block + assert spec.is_merge_transition_block(state, body) == is_merge_transition_block assert spec.is_execution_enabled(state, body) == is_execution_enabled From 4370b9c816b7a98ace84ca0b99f7101c14b7db54 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 23 Nov 2021 14:53:13 -0700 Subject: [PATCH 47/48] bump VERSION.txt to 1.1.6 --- tests/core/pyspec/eth2spec/VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/VERSION.txt b/tests/core/pyspec/eth2spec/VERSION.txt index 314c3d7175..ab679818ce 100644 --- a/tests/core/pyspec/eth2spec/VERSION.txt +++ b/tests/core/pyspec/eth2spec/VERSION.txt @@ -1 +1 @@ -1.1.5 \ No newline at end of file +1.1.6 \ No newline at end of file From d33a9391e31ec6dbb46a0216e7d038be724b220f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 23 Nov 2021 15:27:26 -0700 Subject: [PATCH 48/48] Apply suggestions from code review --- configs/mainnet.yaml | 2 +- configs/minimal.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configs/mainnet.yaml b/configs/mainnet.yaml index 0a207b423e..395e8d7177 100644 --- a/configs/mainnet.yaml +++ b/configs/mainnet.yaml @@ -72,7 +72,7 @@ CHURN_LIMIT_QUOTIENT: 65536 # Fork choice # --------------------------------------------------------------- -# 25% +# 70% PROPOSER_SCORE_BOOST: 70 # Deposit contract diff --git a/configs/minimal.yaml b/configs/minimal.yaml index 508587ee53..1b5433ec3a 100644 --- a/configs/minimal.yaml +++ b/configs/minimal.yaml @@ -71,7 +71,7 @@ CHURN_LIMIT_QUOTIENT: 32 # Fork choice # --------------------------------------------------------------- -# 25% +# 70% PROPOSER_SCORE_BOOST: 70