From b029c75d88f911902228b92668b7d0d8f17a894e Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 1 Feb 2021 07:52:06 -0700 Subject: [PATCH 1/9] must be correct target to get correct head --- specs/lightclient/beacon-chain.md | 2 +- .../pyspec/eth2spec/test/helpers/rewards.py | 19 +++++++++++++------ .../test/phase0/rewards/test_random.py | 6 ++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index bb00a5c59..bcb90f756 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -374,7 +374,7 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: # Participation flags participation_flags = [] - if is_matching_head and state.slot <= data.slot + MIN_ATTESTATION_INCLUSION_DELAY: + if is_matching_head and is_matching_target and state.slot <= data.slot + MIN_ATTESTATION_INCLUSION_DELAY: participation_flags.append(TIMELY_HEAD_FLAG) if is_matching_source and state.slot <= data.slot + integer_squareroot(SLOTS_PER_EPOCH): participation_flags.append(TIMELY_SOURCE_FLAG) diff --git a/tests/core/pyspec/eth2spec/test/helpers/rewards.py b/tests/core/pyspec/eth2spec/test/helpers/rewards.py index 770519384..81eb1f955 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/rewards.py +++ b/tests/core/pyspec/eth2spec/test/helpers/rewards.py @@ -517,11 +517,18 @@ def run_test_full_random(spec, state, rng=Random(8020)): pending_attestation.inclusion_delay = rng.randint(1, spec.SLOTS_PER_EPOCH) else: for index in range(len(state.validators)): - # ~1/3 have bad target - state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] = rng.randint(0, 2) != 0 - # ~1/3 have bad head - state.previous_epoch_participation[index][spec.TIMELY_HEAD_FLAG] = rng.randint(0, 2) != 0 - # ~50% participation - state.previous_epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = rng.choice([True, False]) + # ~1/3 have bad head or bad target or not timely enough + is_timely_correct_head = rng.randint(0, 2) != 0 + state.previous_epoch_participation[index][spec.TIMELY_HEAD_FLAG] = is_timely_correct_head + if is_timely_correct_head: + # If timely head, then must be timely target + state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] = True + # If timely head, then must be timely source + state.previous_epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = True + else: + # ~50% of remaining have bad target or not timely enough + state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] = rng.choice([True, False]) + # ~50% of remaining have bad source or not timely enough + state.previous_epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = rng.choice([True, False]) yield from run_deltas(spec, state) diff --git a/tests/core/pyspec/eth2spec/test/phase0/rewards/test_random.py b/tests/core/pyspec/eth2spec/test/phase0/rewards/test_random.py index 83c7f7905..ae44c6640 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/rewards/test_random.py +++ b/tests/core/pyspec/eth2spec/test/phase0/rewards/test_random.py @@ -29,6 +29,12 @@ def test_full_random_2(spec, state): yield from rewards_helpers.run_test_full_random(spec, state, rng=Random(3030)) +@with_all_phases +@spec_state_test +def test_full_random_3(spec, state): + yield from rewards_helpers.run_test_full_random(spec, state, rng=Random(4040)) + + @with_all_phases @with_custom_state(balances_fn=low_balances, threshold_fn=lambda spec: spec.EJECTION_BALANCE) @spec_test From 1ba491711947b4f908bd68b0639584b11c20442c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 1 Feb 2021 08:35:58 -0700 Subject: [PATCH 2/9] add process_attestation tests to cover various timing and correctness scenarios --- .../eth2spec/test/helpers/attestations.py | 7 +- .../test_process_attestation.py | 159 +++++++++++++++++- 2 files changed, 160 insertions(+), 6 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/helpers/attestations.py b/tests/core/pyspec/eth2spec/test/helpers/attestations.py index cf62482da..571e19fef 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/attestations.py +++ b/tests/core/pyspec/eth2spec/test/helpers/attestations.py @@ -44,11 +44,8 @@ def run_attestation_processing(spec, state, attestation, valid=True): else: assert len(state.previous_epoch_attestations) == previous_epoch_count + 1 else: - for index in spec.get_attesting_indices(state, attestation.data, attestation.aggregation_bits): - if attestation.data.target.epoch == spec.get_current_epoch(state): - assert state.current_epoch_participation[index][spec.TIMELY_TARGET_FLAG] - else: - assert state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] + # After accounting reform, there are cases when processing an attestation does not result in any flag updates + pass # yield post-state yield 'post', state diff --git a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py index b31cf167c..71000763b 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py +++ b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py @@ -2,10 +2,13 @@ from eth2spec.test.context import ( spec_state_test, always_bls, never_bls, with_all_phases, + with_all_phases_except, spec_test, low_balances, with_custom_state, - single_phase) + single_phase, + PHASE1, +) from eth2spec.test.helpers.attestations import ( run_attestation_processing, get_valid_attestation, @@ -329,3 +332,157 @@ def test_too_few_aggregation_bits(spec, state): attestation.aggregation_bits = attestation.aggregation_bits[:-1] yield from run_attestation_processing(spec, state, attestation, False) + + +# +# Full correct atttestation contents at different slot inclusions +# + +@with_all_phases +@spec_state_test +def test_correct_min_inclusion_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=True) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_correct_sqrt_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=True, on_time=False) + next_slots(spec, state, spec.integer_squareroot(spec.SLOTS_PER_EPOCH)) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_correct_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=True, on_time=False) + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + yield from run_attestation_processing(spec, state, attestation) + + +# +# Incorrect head but correct source/target at different slot inclusions +# + +@with_all_phases_except([PHASE1]) +@spec_state_test +def test_incorrect_head_min_inclusion_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + attestation.data.beacon_block_root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_head_sqrt_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.integer_squareroot(spec.SLOTS_PER_EPOCH)) + + attestation.data.beacon_block_root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_head_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + attestation.data.beacon_block_root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +# +# Incorrect head and target but correct source at different slot inclusions +# + +@with_all_phases_except([PHASE1]) +@spec_state_test +def test_incorrect_head_and_target_min_inclusion_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + attestation.data.beacon_block_root = b'\x42' * 32 + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_head_and_target_sqrt_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.integer_squareroot(spec.SLOTS_PER_EPOCH)) + + attestation.data.beacon_block_root = b'\x42' * 32 + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_head_and_target_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + attestation.data.beacon_block_root = b'\x42' * 32 + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +# +# Correct head and source but incorrect target at different slot inclusions +# + +@with_all_phases_except([PHASE1]) +@spec_state_test +def test_incorrect_target_min_inclusion_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_target_sqrt_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.integer_squareroot(spec.SLOTS_PER_EPOCH)) + + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_target_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + next_slots(spec, state, spec.SLOTS_PER_EPOCH) + + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation) From 3677073812c02c2ed613e14d874e444b97a89210 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 1 Feb 2021 21:46:27 +0100 Subject: [PATCH 3/9] bitvector[8] -> uint8, for efficient packing in flags merkle tree --- specs/lightclient/beacon-chain.md | 53 +++++++++++++------ specs/lightclient/lightclient-fork.md | 4 +- .../pyspec/eth2spec/test/helpers/rewards.py | 29 ++++++---- ..._process_justification_and_finalization.py | 6 +-- .../test/phase0/sanity/test_blocks.py | 2 +- 5 files changed, 63 insertions(+), 31 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index bcb90f756..1ac6d2a2c 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -55,15 +55,26 @@ This is a patch implementing the first hard fork to the beacon chain, tentativel and [TODO] reducing the cost of processing chains that have very little or zero participation for a long span of epochs * Fork choice rule changes to address weaknesses recently discovered in the existing fork choice +## Custom types + +| Name | SSZ equivalent | Description | +| - | - | - | +| `ValidatorFlags` | `uint8` | Bitflags to track validator actions with | + ## Constants -### Participation flags +### Validator action flags + +This is formatted as an enum, with values `2**i` that can be combined as bit-flags. +The `0` value is reserved as default. Remaining bits in `ValidatorFlags` may be used in future hardforks. + +**Note**: unlike Phase0, a `TIMELY_TARGET_FLAG` does not imply a `TIMELY_SOURCE_FLAG`. | Name | Value | | - | - | -| `TIMELY_HEAD_FLAG` | `0` | -| `TIMELY_SOURCE_FLAG` | `1` | -| `TIMELY_TARGET_FLAG` | `2` | +| `TIMELY_HEAD_FLAG` | `ValidatorFlags(2**0)` (= 1) | +| `TIMELY_SOURCE_FLAG` | `ValidatorFlags(2**1)` (= 2) | +| `TIMELY_TARGET_FLAG` | `ValidatorFlags(2**2)` (= 4) | ### Participation rewards @@ -80,7 +91,6 @@ The reward fractions add up to 7/8, leaving the remaining 1/8 for proposer rewar | Name | Value | | - | - | -| `PARTICIPATION_FLAGS_LENGTH` | `8` | | `G2_POINT_AT_INFINITY` | `BLSSignature(b'\xc0' + b'\x00' * 95)` | ## Configuration @@ -146,8 +156,8 @@ class BeaconState(Container): # Slashings slashings: Vector[Gwei, EPOCHS_PER_SLASHINGS_VECTOR] # Per-epoch sums of slashed effective balances # Participation - previous_epoch_participation: List[Bitvector[PARTICIPATION_FLAGS_LENGTH], VALIDATOR_REGISTRY_LIMIT] - current_epoch_participation: List[Bitvector[PARTICIPATION_FLAGS_LENGTH], VALIDATOR_REGISTRY_LIMIT] + previous_epoch_participation: List[ValidatorFlags, VALIDATOR_REGISTRY_LIMIT] + current_epoch_participation: List[ValidatorFlags, VALIDATOR_REGISTRY_LIMIT] # Finality justification_bits: Bitvector[JUSTIFICATION_BITS_LENGTH] # Bit set for every recent justified epoch previous_justified_checkpoint: Checkpoint @@ -197,7 +207,15 @@ def get_flags_and_numerators() -> Sequence[Tuple[int, int]]: ) ``` +```python +def add_flags(flags: ValidatorFlags, add: ValidatorFlags) -> ValidatorFlags: + return flags | add +``` +```python +def has_flags(flags: ValidatorFlags, has: ValidatorFlags) -> bool: + return flags & has == has +``` ### Beacon state accessors @@ -257,7 +275,10 @@ def get_base_reward(state: BeaconState, index: ValidatorIndex) -> Gwei: #### `get_unslashed_participating_indices` ```python -def get_unslashed_participating_indices(state: BeaconState, flag: uint8, epoch: Epoch) -> Set[ValidatorIndex]: +def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlags, epoch: Epoch) -> Set[ValidatorIndex]: + """ + Retrieves the active validator indices of the given epoch, who are not slashed, and have all of the given flags. + """ assert epoch in (get_previous_epoch(state), get_current_epoch(state)) if epoch == get_current_epoch(state): epoch_participation = state.current_epoch_participation @@ -265,7 +286,7 @@ def get_unslashed_participating_indices(state: BeaconState, flag: uint8, epoch: epoch_participation = state.previous_epoch_participation participating_indices = [ index for index in get_active_validator_indices(state, epoch) - if epoch_participation[index][flag] + if has_flags(epoch_participation[index], flags) ] return set(filter(lambda index: not state.validators[index].slashed, participating_indices)) ``` @@ -273,7 +294,9 @@ def get_unslashed_participating_indices(state: BeaconState, flag: uint8, epoch: #### `get_flag_deltas` ```python -def get_flag_deltas(state: BeaconState, flag: uint8, numerator: uint64) -> Tuple[Sequence[Gwei], Sequence[Gwei]]: +def get_flag_deltas(state: BeaconState, + flag: ValidatorFlags, + numerator: uint64) -> Tuple[Sequence[Gwei], Sequence[Gwei]]: """ Computes the rewards and penalties associated with a particular duty, by scanning through the participation flags to determine who participated and who did not and assigning them the appropriate rewards and penalties. @@ -385,8 +408,8 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: proposer_reward_numerator = 0 for index in get_attesting_indices(state, data, attestation.aggregation_bits): for flag, numerator in get_flags_and_numerators(): - if flag in participation_flags and not epoch_participation[index][flag]: - epoch_participation[index][flag] = True + if flag in participation_flags and not has_flags(epoch_participation[index], flag): + epoch_participation[index] = add_flags(epoch_participation[index], flag) proposer_reward_numerator += get_base_reward(state, index) * numerator # Reward proposer @@ -432,8 +455,8 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: state.validators.append(get_validator_from_deposit(state, deposit)) state.balances.append(amount) # [Added in hf-1] Initialize empty participation flags for new validator - state.previous_epoch_participation.append(Bitvector[PARTICIPATION_FLAGS_LENGTH]()) - state.current_epoch_participation.append(Bitvector[PARTICIPATION_FLAGS_LENGTH]()) + state.previous_epoch_participation.append(ValidatorFlags(0)) + state.current_epoch_participation.append(ValidatorFlags(0)) else: # Increase balance by deposit amount index = ValidatorIndex(validator_pubkeys.index(pubkey)) @@ -572,5 +595,5 @@ def process_participation_flag_updates(state: BeaconState) -> None: Call to ``process_participation_flag_updates`` added to ``process_epoch`` in HF1 """ state.previous_epoch_participation = state.current_epoch_participation - state.current_epoch_participation = [Bitvector[PARTICIPATION_FLAGS_LENGTH]() for _ in range(len(state.validators))] + state.current_epoch_participation = [ValidatorFlags(0) for _ in range(len(state.validators))] ``` diff --git a/specs/lightclient/lightclient-fork.md b/specs/lightclient/lightclient-fork.md index a10e8c5f6..f5f3d1b7b 100644 --- a/specs/lightclient/lightclient-fork.md +++ b/specs/lightclient/lightclient-fork.md @@ -67,8 +67,8 @@ def upgrade_to_lightclient_patch(pre: phase0.BeaconState) -> BeaconState: # Slashings slashings=pre.slashings, # Attestations - previous_epoch_participation=[Bitvector[PARTICIPATION_FLAGS_LENGTH]() for _ in range(len(pre.validators))], - current_epoch_participation=[Bitvector[PARTICIPATION_FLAGS_LENGTH]() for _ in range(len(pre.validators))], + previous_epoch_participation=[ValidatorFlags(0) for _ in range(len(pre.validators))], + current_epoch_participation=[ValidatorFlags(0) for _ in range(len(pre.validators))], # Finality justification_bits=pre.justification_bits, previous_justified_checkpoint=pre.previous_justified_checkpoint, diff --git a/tests/core/pyspec/eth2spec/test/helpers/rewards.py b/tests/core/pyspec/eth2spec/test/helpers/rewards.py index 81eb1f955..feaac018b 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/rewards.py +++ b/tests/core/pyspec/eth2spec/test/helpers/rewards.py @@ -6,7 +6,7 @@ from eth2spec.test.context import is_post_lightclient_patch from eth2spec.test.helpers.attestations import cached_prepare_state_with_attestations from eth2spec.test.helpers.deposits import mock_deposit from eth2spec.test.helpers.state import next_epoch -from eth2spec.utils.ssz.ssz_typing import Container, uint64, List, Bitvector +from eth2spec.utils.ssz.ssz_typing import Container, uint64, List class Deltas(Container): @@ -314,7 +314,7 @@ def run_test_full_but_partial_participation(spec, state, rng=Random(5522)): else: for index in range(len(state.validators)): if rng.choice([True, False]): - state.previous_epoch_participation[index] = Bitvector[spec.PARTICIPATION_FLAGS_LENGTH]() + state.previous_epoch_participation[index] = spec.ValidatorFlags(0) yield from run_deltas(spec, state) @@ -328,7 +328,7 @@ def run_test_partial(spec, state, fraction_filled): state.previous_epoch_attestations = state.previous_epoch_attestations[:num_attestations] else: for index in range(int(len(state.validators) * fraction_filled)): - state.previous_epoch_participation[index] = Bitvector[spec.PARTICIPATION_FLAGS_LENGTH]() + state.previous_epoch_participation[index] = spec.ValidatorFlags(0) yield from run_deltas(spec, state) @@ -394,7 +394,7 @@ def run_test_some_very_low_effective_balances_that_did_not_attest(spec, state): else: index = 0 state.validators[index].effective_balance = 1 - state.previous_epoch_participation[index] = Bitvector[spec.PARTICIPATION_FLAGS_LENGTH]() + state.previous_epoch_participation[index] = spec.ValidatorFlags(0) yield from run_deltas(spec, state) @@ -519,16 +519,25 @@ def run_test_full_random(spec, state, rng=Random(8020)): for index in range(len(state.validators)): # ~1/3 have bad head or bad target or not timely enough is_timely_correct_head = rng.randint(0, 2) != 0 - state.previous_epoch_participation[index][spec.TIMELY_HEAD_FLAG] = is_timely_correct_head + flags = state.previous_epoch_participation[index] + + def set_flag(f, v): + nonlocal flags + if v: + flags |= f + else: + flags &= 0xff ^ f + + set_flag(spec.TIMELY_HEAD_FLAG, is_timely_correct_head) if is_timely_correct_head: # If timely head, then must be timely target - state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] = True + set_flag(spec.TIMELY_TARGET_FLAG, True) # If timely head, then must be timely source - state.previous_epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = True + set_flag(spec.TIMELY_SOURCE_FLAG, True) else: # ~50% of remaining have bad target or not timely enough - state.previous_epoch_participation[index][spec.TIMELY_TARGET_FLAG] = rng.choice([True, False]) + set_flag(spec.TIMELY_TARGET_FLAG, rng.choice([True, False])) # ~50% of remaining have bad source or not timely enough - state.previous_epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = rng.choice([True, False]) - + set_flag(spec.TIMELY_SOURCE_FLAG, rng.choice([True, False])) + state.previous_epoch_participation[index] = flags yield from run_deltas(spec, state) diff --git a/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_justification_and_finalization.py b/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_justification_and_finalization.py index 0d1741efb..89783f987 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_justification_and_finalization.py +++ b/tests/core/pyspec/eth2spec/test/phase0/epoch_processing/test_process_justification_and_finalization.py @@ -78,10 +78,10 @@ def add_mock_attestations(spec, state, epoch, source, target, sufficient_support else: for i, index in enumerate(committee): if aggregation_bits[i]: - epoch_participation[index][spec.TIMELY_HEAD_FLAG] = True - epoch_participation[index][spec.TIMELY_SOURCE_FLAG] = True + epoch_participation[index] |= spec.TIMELY_HEAD_FLAG + epoch_participation[index] |= spec.TIMELY_SOURCE_FLAG if not messed_up_target: - epoch_participation[index][spec.TIMELY_TARGET_FLAG] = True + epoch_participation[index] |= spec.TIMELY_TARGET_FLAG def get_checkpoints(spec, epoch): diff --git a/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py b/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py index 9cc8ab721..1834b290f 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py +++ b/tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py @@ -806,7 +806,7 @@ def test_attestation(spec, state): assert spec.hash_tree_root(state.previous_epoch_attestations) == pre_current_attestations_root else: for index in range(len(state.validators)): - assert state.current_epoch_participation[index] == spec.Bitvector[spec.PARTICIPATION_FLAGS_LENGTH]() + assert state.current_epoch_participation[index] == 0 assert spec.hash_tree_root(state.previous_epoch_participation) == pre_current_epoch_participation_root From e865670111c2fb89b7031f3ea6d67bf2426b03a9 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 1 Feb 2021 21:47:00 +0100 Subject: [PATCH 4/9] add missing decorators for testruns in no-bls mode --- .../block_processing/test_process_sync_committee.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py index a8be1af63..430c59b10 100644 --- a/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py +++ b/tests/core/pyspec/eth2spec/test/lightclient_patch/block_processing/test_process_sync_committee.py @@ -18,6 +18,7 @@ from eth2spec.test.context import ( with_all_phases_except, with_configs, spec_state_test, + always_bls, ) from eth2spec.utils.hash_function import hash @@ -196,6 +197,7 @@ def test_sync_committee_rewards_duplicate_committee(spec, state): @with_all_phases_except([PHASE0, PHASE1]) @spec_state_test +@always_bls def test_invalid_signature_past_block(spec, state): committee = spec.get_sync_committee_indices(state, spec.get_current_epoch(state)) @@ -237,6 +239,7 @@ def test_invalid_signature_past_block(spec, state): @with_all_phases_except([PHASE0, PHASE1]) @with_configs([MINIMAL], reason="to produce different committee sets") @spec_state_test +@always_bls def test_invalid_signature_previous_committee(spec, state): # NOTE: the `state` provided is at genesis and the process to select # sync committees currently returns the same committee for the first and second From 71c28e67a13469ffc0452687f4ce6fad99a73107 Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 1 Feb 2021 21:48:55 +0100 Subject: [PATCH 5/9] toc update --- specs/lightclient/beacon-chain.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 1ac6d2a2c..179aec6d1 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -7,8 +7,9 @@ - [Introduction](#introduction) +- [Custom types](#custom-types) - [Constants](#constants) - - [Participation flags](#participation-flags) + - [Validator action flags](#validator-action-flags) - [Participation rewards](#participation-rewards) - [Misc](#misc) - [Configuration](#configuration) From b4ba6c57de0bb2138379a2e1761d5ab2bc3ed90e Mon Sep 17 00:00:00 2001 From: protolambda Date: Mon, 1 Feb 2021 22:02:12 +0100 Subject: [PATCH 6/9] linter: first tuple element type is ValidatorFlags, not just int --- specs/lightclient/beacon-chain.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 179aec6d1..9f0572740 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -200,7 +200,7 @@ def eth2_fast_aggregate_verify(pubkeys: Sequence[BLSPubkey], message: Bytes32, s #### `flags_and_numerators` ```python -def get_flags_and_numerators() -> Sequence[Tuple[int, int]]: +def get_flags_and_numerators() -> Sequence[Tuple[ValidatorFlags, int]]: return ( (TIMELY_HEAD_FLAG, TIMELY_HEAD_NUMERATOR), (TIMELY_SOURCE_FLAG, TIMELY_SOURCE_NUMERATOR), From 1c1ba5cba291e725972601c62e97649565d1f299 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 2 Feb 2021 12:35:00 -0700 Subject: [PATCH 7/9] minor PR feedback --- specs/lightclient/beacon-chain.md | 7 ++- .../test_process_attestation.py | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 9f0572740..3508fe991 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -69,7 +69,8 @@ This is a patch implementing the first hard fork to the beacon chain, tentativel This is formatted as an enum, with values `2**i` that can be combined as bit-flags. The `0` value is reserved as default. Remaining bits in `ValidatorFlags` may be used in future hardforks. -**Note**: unlike Phase0, a `TIMELY_TARGET_FLAG` does not imply a `TIMELY_SOURCE_FLAG`. +**Note**: Unlike Phase0, a `TIMELY_TARGET_FLAG` does not necessarily imply a `TIMELY_SOURCE_FLAG` +due to the varying slot delay requirements of each. | Name | Value | | - | - | @@ -278,7 +279,7 @@ def get_base_reward(state: BeaconState, index: ValidatorIndex) -> Gwei: ```python def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlags, epoch: Epoch) -> Set[ValidatorIndex]: """ - Retrieves the active validator indices of the given epoch, who are not slashed, and have all of the given flags. + Retrieve the active validator indices of the given epoch, which are not slashed, and have all of the given flags. """ assert epoch in (get_previous_epoch(state), get_current_epoch(state)) if epoch == get_current_epoch(state): @@ -299,7 +300,7 @@ def get_flag_deltas(state: BeaconState, flag: ValidatorFlags, numerator: uint64) -> Tuple[Sequence[Gwei], Sequence[Gwei]]: """ - Computes the rewards and penalties associated with a particular duty, by scanning through the participation + Compute the rewards and penalties associated with a particular duty, by scanning through the participation flags to determine who participated and who did not and assigning them the appropriate rewards and penalties. """ rewards = [Gwei(0)] * len(state.validators) diff --git a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py index 71000763b..99a82879d 100644 --- a/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py +++ b/tests/core/pyspec/eth2spec/test/phase0/block_processing/test_process_attestation.py @@ -365,6 +365,17 @@ def test_correct_epoch_delay(spec, state): yield from run_attestation_processing(spec, state, attestation) +@with_all_phases +@spec_state_test +def test_correct_after_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=True, on_time=False) + + # increment past latest inclusion slot + next_slots(spec, state, spec.SLOTS_PER_EPOCH + 1) + + yield from run_attestation_processing(spec, state, attestation, False) + + # # Incorrect head but correct source/target at different slot inclusions # @@ -405,10 +416,27 @@ def test_incorrect_head_epoch_delay(spec, state): yield from run_attestation_processing(spec, state, attestation) +@with_all_phases +@spec_state_test +def test_incorrect_head_after_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + + # increment past latest inclusion slot + next_slots(spec, state, spec.SLOTS_PER_EPOCH + 1) + + attestation.data.beacon_block_root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation, False) + + # # Incorrect head and target but correct source at different slot inclusions # +# Note: current phase 1 spec checks +# `assert data.beacon_block_root == get_block_root_at_slot(state, compute_previous_slot(state.slot))` +# so this test can't pass that until phase 1 refactor is merged @with_all_phases_except([PHASE1]) @spec_state_test def test_incorrect_head_and_target_min_inclusion_delay(spec, state): @@ -448,6 +476,20 @@ def test_incorrect_head_and_target_epoch_delay(spec, state): yield from run_attestation_processing(spec, state, attestation) +@with_all_phases +@spec_state_test +def test_incorrect_head_and_target_after_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + # increment past latest inclusion slot + next_slots(spec, state, spec.SLOTS_PER_EPOCH + 1) + + attestation.data.beacon_block_root = b'\x42' * 32 + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation, False) + + # # Correct head and source but incorrect target at different slot inclusions # @@ -486,3 +528,16 @@ def test_incorrect_target_epoch_delay(spec, state): sign_attestation(spec, state, attestation) yield from run_attestation_processing(spec, state, attestation) + + +@with_all_phases +@spec_state_test +def test_incorrect_target_after_epoch_delay(spec, state): + attestation = get_valid_attestation(spec, state, signed=False, on_time=False) + # increment past latest inclusion slot + next_slots(spec, state, spec.SLOTS_PER_EPOCH + 1) + + attestation.data.target.root = b'\x42' * 32 + sign_attestation(spec, state, attestation) + + yield from run_attestation_processing(spec, state, attestation, False) From 34cea67b91cd0d36504a6376c93e13c589832a7d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 4 Feb 2021 08:45:25 -0700 Subject: [PATCH 8/9] ValidatorFlags -> ValidatorFlag --- specs/lightclient/beacon-chain.md | 30 +++++++++---------- specs/lightclient/lightclient-fork.md | 4 +-- .../pyspec/eth2spec/test/helpers/rewards.py | 6 ++-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index 3508fe991..ff69c1312 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -60,23 +60,23 @@ This is a patch implementing the first hard fork to the beacon chain, tentativel | Name | SSZ equivalent | Description | | - | - | - | -| `ValidatorFlags` | `uint8` | Bitflags to track validator actions with | +| `ValidatorFlag` | `uint8` | Bitflags to track validator actions with | ## Constants ### Validator action flags This is formatted as an enum, with values `2**i` that can be combined as bit-flags. -The `0` value is reserved as default. Remaining bits in `ValidatorFlags` may be used in future hardforks. +The `0` value is reserved as default. Remaining bits in `ValidatorFlag` may be used in future hardforks. **Note**: Unlike Phase0, a `TIMELY_TARGET_FLAG` does not necessarily imply a `TIMELY_SOURCE_FLAG` due to the varying slot delay requirements of each. | Name | Value | | - | - | -| `TIMELY_HEAD_FLAG` | `ValidatorFlags(2**0)` (= 1) | -| `TIMELY_SOURCE_FLAG` | `ValidatorFlags(2**1)` (= 2) | -| `TIMELY_TARGET_FLAG` | `ValidatorFlags(2**2)` (= 4) | +| `TIMELY_HEAD_FLAG` | `ValidatorFlag(2**0)` (= 1) | +| `TIMELY_SOURCE_FLAG` | `ValidatorFlag(2**1)` (= 2) | +| `TIMELY_TARGET_FLAG` | `ValidatorFlag(2**2)` (= 4) | ### Participation rewards @@ -158,8 +158,8 @@ class BeaconState(Container): # Slashings slashings: Vector[Gwei, EPOCHS_PER_SLASHINGS_VECTOR] # Per-epoch sums of slashed effective balances # Participation - previous_epoch_participation: List[ValidatorFlags, VALIDATOR_REGISTRY_LIMIT] - current_epoch_participation: List[ValidatorFlags, VALIDATOR_REGISTRY_LIMIT] + previous_epoch_participation: List[ValidatorFlag, VALIDATOR_REGISTRY_LIMIT] + current_epoch_participation: List[ValidatorFlag, VALIDATOR_REGISTRY_LIMIT] # Finality justification_bits: Bitvector[JUSTIFICATION_BITS_LENGTH] # Bit set for every recent justified epoch previous_justified_checkpoint: Checkpoint @@ -201,7 +201,7 @@ def eth2_fast_aggregate_verify(pubkeys: Sequence[BLSPubkey], message: Bytes32, s #### `flags_and_numerators` ```python -def get_flags_and_numerators() -> Sequence[Tuple[ValidatorFlags, int]]: +def get_flags_and_numerators() -> Sequence[Tuple[ValidatorFlag, int]]: return ( (TIMELY_HEAD_FLAG, TIMELY_HEAD_NUMERATOR), (TIMELY_SOURCE_FLAG, TIMELY_SOURCE_NUMERATOR), @@ -210,12 +210,12 @@ def get_flags_and_numerators() -> Sequence[Tuple[ValidatorFlags, int]]: ``` ```python -def add_flags(flags: ValidatorFlags, add: ValidatorFlags) -> ValidatorFlags: +def add_flags(flags: ValidatorFlag, add: ValidatorFlag) -> ValidatorFlag: return flags | add ``` ```python -def has_flags(flags: ValidatorFlags, has: ValidatorFlags) -> bool: +def has_flags(flags: ValidatorFlag, has: ValidatorFlag) -> bool: return flags & has == has ``` @@ -277,7 +277,7 @@ def get_base_reward(state: BeaconState, index: ValidatorIndex) -> Gwei: #### `get_unslashed_participating_indices` ```python -def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlags, epoch: Epoch) -> Set[ValidatorIndex]: +def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlag, epoch: Epoch) -> Set[ValidatorIndex]: """ Retrieve the active validator indices of the given epoch, which are not slashed, and have all of the given flags. """ @@ -297,7 +297,7 @@ def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlag ```python def get_flag_deltas(state: BeaconState, - flag: ValidatorFlags, + flag: ValidatorFlag, numerator: uint64) -> Tuple[Sequence[Gwei], Sequence[Gwei]]: """ Compute the rewards and penalties associated with a particular duty, by scanning through the participation @@ -457,8 +457,8 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: state.validators.append(get_validator_from_deposit(state, deposit)) state.balances.append(amount) # [Added in hf-1] Initialize empty participation flags for new validator - state.previous_epoch_participation.append(ValidatorFlags(0)) - state.current_epoch_participation.append(ValidatorFlags(0)) + state.previous_epoch_participation.append(ValidatorFlag(0)) + state.current_epoch_participation.append(ValidatorFlag(0)) else: # Increase balance by deposit amount index = ValidatorIndex(validator_pubkeys.index(pubkey)) @@ -597,5 +597,5 @@ def process_participation_flag_updates(state: BeaconState) -> None: Call to ``process_participation_flag_updates`` added to ``process_epoch`` in HF1 """ state.previous_epoch_participation = state.current_epoch_participation - state.current_epoch_participation = [ValidatorFlags(0) for _ in range(len(state.validators))] + state.current_epoch_participation = [ValidatorFlag(0) for _ in range(len(state.validators))] ``` diff --git a/specs/lightclient/lightclient-fork.md b/specs/lightclient/lightclient-fork.md index f5f3d1b7b..aa0171b86 100644 --- a/specs/lightclient/lightclient-fork.md +++ b/specs/lightclient/lightclient-fork.md @@ -67,8 +67,8 @@ def upgrade_to_lightclient_patch(pre: phase0.BeaconState) -> BeaconState: # Slashings slashings=pre.slashings, # Attestations - previous_epoch_participation=[ValidatorFlags(0) for _ in range(len(pre.validators))], - current_epoch_participation=[ValidatorFlags(0) for _ in range(len(pre.validators))], + previous_epoch_participation=[ValidatorFlag(0) for _ in range(len(pre.validators))], + current_epoch_participation=[ValidatorFlag(0) for _ in range(len(pre.validators))], # Finality justification_bits=pre.justification_bits, previous_justified_checkpoint=pre.previous_justified_checkpoint, diff --git a/tests/core/pyspec/eth2spec/test/helpers/rewards.py b/tests/core/pyspec/eth2spec/test/helpers/rewards.py index feaac018b..2499bcffe 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/rewards.py +++ b/tests/core/pyspec/eth2spec/test/helpers/rewards.py @@ -314,7 +314,7 @@ def run_test_full_but_partial_participation(spec, state, rng=Random(5522)): else: for index in range(len(state.validators)): if rng.choice([True, False]): - state.previous_epoch_participation[index] = spec.ValidatorFlags(0) + state.previous_epoch_participation[index] = spec.ValidatorFlag(0) yield from run_deltas(spec, state) @@ -328,7 +328,7 @@ def run_test_partial(spec, state, fraction_filled): state.previous_epoch_attestations = state.previous_epoch_attestations[:num_attestations] else: for index in range(int(len(state.validators) * fraction_filled)): - state.previous_epoch_participation[index] = spec.ValidatorFlags(0) + state.previous_epoch_participation[index] = spec.ValidatorFlag(0) yield from run_deltas(spec, state) @@ -394,7 +394,7 @@ def run_test_some_very_low_effective_balances_that_did_not_attest(spec, state): else: index = 0 state.validators[index].effective_balance = 1 - state.previous_epoch_participation[index] = spec.ValidatorFlags(0) + state.previous_epoch_participation[index] = spec.ValidatorFlag(0) yield from run_deltas(spec, state) From 9313815976803c2e93c8fbb4b2518340c4ce83c7 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 4 Feb 2021 08:47:46 -0700 Subject: [PATCH 9/9] put 'validator' in flags methods --- specs/lightclient/beacon-chain.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/specs/lightclient/beacon-chain.md b/specs/lightclient/beacon-chain.md index ff69c1312..e11f50076 100644 --- a/specs/lightclient/beacon-chain.md +++ b/specs/lightclient/beacon-chain.md @@ -210,12 +210,12 @@ def get_flags_and_numerators() -> Sequence[Tuple[ValidatorFlag, int]]: ``` ```python -def add_flags(flags: ValidatorFlag, add: ValidatorFlag) -> ValidatorFlag: +def add_validator_flags(flags: ValidatorFlag, add: ValidatorFlag) -> ValidatorFlag: return flags | add ``` ```python -def has_flags(flags: ValidatorFlag, has: ValidatorFlag) -> bool: +def has_validator_flags(flags: ValidatorFlag, has: ValidatorFlag) -> bool: return flags & has == has ``` @@ -288,7 +288,7 @@ def get_unslashed_participating_indices(state: BeaconState, flags: ValidatorFlag epoch_participation = state.previous_epoch_participation participating_indices = [ index for index in get_active_validator_indices(state, epoch) - if has_flags(epoch_participation[index], flags) + if has_validator_flags(epoch_participation[index], flags) ] return set(filter(lambda index: not state.validators[index].slashed, participating_indices)) ``` @@ -410,8 +410,8 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: proposer_reward_numerator = 0 for index in get_attesting_indices(state, data, attestation.aggregation_bits): for flag, numerator in get_flags_and_numerators(): - if flag in participation_flags and not has_flags(epoch_participation[index], flag): - epoch_participation[index] = add_flags(epoch_participation[index], flag) + if flag in participation_flags and not has_validator_flags(epoch_participation[index], flag): + epoch_participation[index] = add_validator_flags(epoch_participation[index], flag) proposer_reward_numerator += get_base_reward(state, index) * numerator # Reward proposer