diff --git a/specs/phase0/beacon-chain.md b/specs/phase0/beacon-chain.md index 0b66cff3cb..899778fd9c 100644 --- a/specs/phase0/beacon-chain.md +++ b/specs/phase0/beacon-chain.md @@ -683,11 +683,11 @@ def is_slashable_attestation_data(data_1: AttestationData, data_2: AttestationDa ```python def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: IndexedAttestation) -> bool: """ - Check if ``indexed_attestation`` has sorted and unique indices and a valid aggregate signature. + Check if ``indexed_attestation`` is not empty, has sorted and unique indices and has a valid aggregate signature. """ # Verify indices are sorted and unique indices = indexed_attestation.attesting_indices - if not indices == sorted(set(indices)): + if len(indices) == 0 or not indices == sorted(set(indices)): return False # Verify aggregate signature pubkeys = [state.validators[i].pubkey for i in indices] diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index f76502bb42..d697e5da31 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -247,7 +247,7 @@ There are two primary global topics used to propagate beacon blocks and aggregat - The `aggregate` is the first valid aggregate received for the aggregator with index `aggregate_and_proof.aggregator_index` for the epoch `aggregate.data.target.epoch`. - The block being voted for (`aggregate.data.beacon_block_root`) passes validation. - `aggregate_and_proof.selection_proof` selects the validator as an aggregator for the slot -- i.e. `is_aggregator(state, aggregate.data.slot, aggregate.data.index, aggregate_and_proof.selection_proof)` returns `True`. - - The aggregator's validator index is within the aggregate's committee -- i.e. `aggregate_and_proof.aggregator_index in get_attesting_indices(state, aggregate.data, aggregate.aggregation_bits)`. + - The aggregator's validator index is within the aggregate's committee -- i.e. `aggregate_and_proof.aggregator_index in get_attesting_indices(state, aggregate.data, aggregate.aggregation_bits)`. This also means that it must never have an empty set of participants. - The `aggregate_and_proof.selection_proof` is a valid signature of the `aggregate.data.slot` by the validator with index `aggregate_and_proof.aggregator_index`. - The aggregator signature, `signed_aggregate_and_proof.signature`, is valid. - The signature of `aggregate` is valid. diff --git a/specs/phase1/beacon-chain.md b/specs/phase1/beacon-chain.md index fc5fe9e144..331243de6d 100644 --- a/specs/phase1/beacon-chain.md +++ b/specs/phase1/beacon-chain.md @@ -571,7 +571,8 @@ def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: Indexe attestation = indexed_attestation.attestation domain = get_domain(state, DOMAIN_BEACON_ATTESTER, attestation.data.target.epoch) aggregation_bits = attestation.aggregation_bits - assert len(aggregation_bits) == len(indexed_attestation.committee) + if not any(aggregation_bits) or len(aggregation_bits) != len(indexed_attestation.committee): + return False if len(attestation.custody_bits_blocks) == 0: # fall back on phase0 behavior if there is no shard data. diff --git a/tests/core/pyspec/eth2spec/test/helpers/attestations.py b/tests/core/pyspec/eth2spec/test/helpers/attestations.py index 9e98e83ea5..2c15a51365 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/attestations.py +++ b/tests/core/pyspec/eth2spec/test/helpers/attestations.py @@ -148,10 +148,12 @@ def get_valid_attestation(spec, state, slot=None, index=None, + filter_participant_set=None, shard_transition=None, - empty=False, signed=False, on_time=True): + # 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: @@ -173,10 +175,8 @@ def get_valid_attestation(spec, aggregation_bits=aggregation_bits, data=attestation_data, ) - if not empty: - fill_aggregate_attestation(spec, state, attestation) - if signed: - sign_attestation(spec, state, attestation) + # 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) if spec.fork == PHASE1 and on_time: attestation = convert_to_valid_on_time_attestation(spec, state, attestation, signed) @@ -269,16 +269,25 @@ def get_attestation_signature(spec, state, attestation_data, privkey): return bls.Sign(privkey, signing_root) -def fill_aggregate_attestation(spec, state, attestation, signed=False): +def fill_aggregate_attestation(spec, state, attestation, signed=False, filter_participant_set=None): + """ + `signed`: Signing is optional. + `filter_participant_set`: Optional, filters the full committee indices set (default) to a subset that participates + """ beacon_committee = spec.get_beacon_committee( state, attestation.data.slot, attestation.data.index, ) + # By default, have everyone participate + participants = set(beacon_committee) + # But optionally filter the participants to a smaller amount + if filter_participant_set is not None: + participants = filter_participant_set(participants) for i in range(len(beacon_committee)): - attestation.aggregation_bits[i] = True + attestation.aggregation_bits[i] = beacon_committee[i] in participants - if signed: + if signed and len(participants) > 0: sign_attestation(spec, state, attestation) diff --git a/tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py b/tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py index 8663391aa0..bb25a384e2 100644 --- a/tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py +++ b/tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py @@ -13,7 +13,6 @@ sign_attestation, ) from eth2spec.test.helpers.state import ( - next_slot, next_slots, next_epoch, transition_to, @@ -64,6 +63,29 @@ def test_invalid_attestation_signature(spec, state): yield from run_attestation_processing(spec, state, attestation, False) +@with_all_phases +@spec_state_test +@always_bls +def test_empty_participants_zeroes_sig(spec, state): + attestation = get_valid_attestation(spec, state, filter_participant_set=lambda comm: []) # 0 participants + attestation.signature = spec.BLSSignature(b'\x00' * 96) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + yield from run_attestation_processing(spec, state, attestation, False) + + +@with_all_phases +@spec_state_test +@always_bls +def test_empty_participants_seemingly_valid_sig(spec, state): + attestation = get_valid_attestation(spec, state, filter_participant_set=lambda comm: []) # 0 participants + # Special BLS value, valid for zero pubkeys on some (but not all) BLS implementations. + attestation.signature = spec.BLSSignature(b'\xc0' + b'\x00' * 95) + next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) + + yield from run_attestation_processing(spec, state, attestation, False) + + @with_all_phases @spec_state_test def test_before_inclusion_delay(spec, state): @@ -260,19 +282,6 @@ def test_bad_source_root(spec, state): yield from run_attestation_processing(spec, state, attestation, False) -@with_all_phases -@spec_state_test -def test_empty_aggregation_bits(spec, state): - next_slot(spec, state) - attestation = get_valid_attestation(spec, state, empty=True) - next_slots(spec, state, spec.MIN_ATTESTATION_INCLUSION_DELAY) - - assert attestation.aggregation_bits == Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE]( - *([0b0] * len(attestation.aggregation_bits))) - - yield from run_attestation_processing(spec, state, attestation) - - @with_all_phases @spec_state_test def test_too_many_aggregation_bits(spec, state): diff --git a/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py b/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py index af695fe697..337f7f25c3 100644 --- a/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py +++ b/tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py @@ -15,13 +15,20 @@ ) from eth2spec.test.helpers.attester_slashings import get_indexed_attestation_participants from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with +from random import Random def run_process_rewards_and_penalties(spec, state): yield from run_epoch_processing_with(spec, state, 'process_rewards_and_penalties') -def prepare_state_with_full_attestations(spec, state, empty=False): +def prepare_state_with_attestations(spec, state, participation_fn=None): + """ + Prepare state with attestations according to the ``participation_fn``. + If no ``participation_fn``, default to "full" -- max committee participation at each slot. + + participation_fn: (slot, committee_index, committee_indices_set) -> participants_indices_set + """ # Go to start of next epoch to ensure can have full participation next_epoch(spec, state) @@ -33,8 +40,15 @@ def prepare_state_with_full_attestations(spec, state, empty=False): # create an attestation for each index in each slot in epoch if state.slot < next_epoch_start_slot: for committee_index in range(spec.get_committee_count_at_slot(state, state.slot)): - attestation = get_valid_attestation(spec, state, index=committee_index, empty=empty, signed=True) - attestations.append(attestation) + def temp_participants_filter(comm): + if participation_fn is None: + return comm + else: + return participation_fn(state.slot, committee_index, comm) + attestation = get_valid_attestation(spec, state, index=committee_index, + filter_participant_set=temp_participants_filter, signed=True) + if any(attestation.aggregation_bits): # Only if there is at least 1 participant. + attestations.append(attestation) # fill each created slot in state after inclusion delay if state.slot >= start_slot + spec.MIN_ATTESTATION_INCLUSION_DELAY: inclusion_slot = state.slot - spec.MIN_ATTESTATION_INCLUSION_DELAY @@ -90,7 +104,7 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state): @with_all_phases @spec_state_test def test_full_attestations(spec, state): - attestations = prepare_state_with_full_attestations(spec, state) + attestations = prepare_state_with_attestations(spec, state) pre_state = state.copy() @@ -108,7 +122,7 @@ def test_full_attestations(spec, state): @with_all_phases @spec_state_test def test_full_attestations_random_incorrect_fields(spec, state): - attestations = prepare_state_with_full_attestations(spec, state) + attestations = prepare_state_with_attestations(spec, state) for i, attestation in enumerate(state.previous_epoch_attestations): if i % 3 == 0: # Mess up some head votes @@ -133,7 +147,7 @@ def test_full_attestations_random_incorrect_fields(spec, state): @with_custom_state(balances_fn=misc_balances, threshold_fn=lambda spec: spec.MAX_EFFECTIVE_BALANCE // 2) @single_phase def test_full_attestations_misc_balances(spec, state): - attestations = prepare_state_with_full_attestations(spec, state) + attestations = prepare_state_with_attestations(spec, state) pre_state = state.copy() @@ -165,7 +179,7 @@ def test_full_attestations_misc_balances(spec, state): @with_custom_state(balances_fn=low_single_balance, threshold_fn=zero_activation_threshold) @single_phase def test_full_attestations_one_validaor_one_gwei(spec, state): - attestations = prepare_state_with_full_attestations(spec, state) + attestations = prepare_state_with_attestations(spec, state) yield from run_process_rewards_and_penalties(spec, state) @@ -189,20 +203,55 @@ def test_no_attestations_all_penalties(spec, state): assert state.balances[index] < pre_state.balances[index] -@with_all_phases -@spec_state_test -def test_empty_attestations(spec, state): - attestations = prepare_state_with_full_attestations(spec, state, empty=True) +def run_with_participation(spec, state, participation_fn): + participated = set() + + def participation_tracker(slot, comm_index, comm): + att_participants = participation_fn(slot, comm_index, comm) + participated.update(att_participants) + return att_participants + + attestations = prepare_state_with_attestations(spec, state, participation_fn=participation_tracker) pre_state = state.copy() yield from run_process_rewards_and_penalties(spec, state) attesting_indices = spec.get_unslashed_attesting_indices(state, attestations) - assert len(attesting_indices) == 0 + assert len(attesting_indices) == len(participated) for index in range(len(pre_state.validators)): - assert state.balances[index] < pre_state.balances[index] + if index in participated: + assert state.balances[index] > pre_state.balances[index] + else: + assert state.balances[index] < pre_state.balances[index] + + +@with_all_phases +@spec_state_test +def test_almost_empty_attestations(spec, state): + rng = Random(1234) + yield from run_with_participation(spec, state, lambda slot, comm_index, comm: rng.sample(comm, 1)) + + +@with_all_phases +@spec_state_test +def test_random_fill_attestations(spec, state): + rng = Random(4567) + yield from run_with_participation(spec, state, lambda slot, comm_index, comm: rng.sample(comm, len(comm) // 3)) + + +@with_all_phases +@spec_state_test +def test_almost_full_attestations(spec, state): + rng = Random(8901) + yield from run_with_participation(spec, state, lambda slot, comm_index, comm: rng.sample(comm, len(comm) - 1)) + + +@with_all_phases +@spec_state_test +def test_full_attestation_participation(spec, state): + yield from run_with_participation(spec, state, lambda slot, comm_index, comm: comm) @with_all_phases @@ -247,7 +296,7 @@ def test_duplicate_attestation(spec, state): @spec_state_test # Case when some eligible attestations are slashed. Modifies attesting_balance and consequently rewards/penalties. def test_attestations_some_slashed(spec, state): - attestations = prepare_state_with_full_attestations(spec, state) + attestations = prepare_state_with_attestations(spec, state) attesting_indices_before_slashings = list(spec.get_unslashed_attesting_indices(state, attestations)) # Slash maximum amount of validators allowed per epoch.