Handle empty-aggregation-bits case, and add tests. See #1713

specs/phase0/beacon-chain.md

@@ -684,11 +684,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]

specs/phase0/p2p-interface.md

@@ -293,7 +293,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.

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.

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

@@ -114,6 +114,9 @@ def get_valid_late_attestation(spec, state, slot=None, index=None, signed=False)
 
 
 def get_valid_attestation(spec, state, slot=None, index=None, empty=False, signed=False, on_time=True):
+    # If empty is true, the attestation has 0 participants, and will not be signed.
+    # Thus strictly speaking invalid when no participant is added later.
+
     if slot is None:
         slot = state.slot
     if index is None:

tests/core/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py

@@ -64,6 +64,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, empty=True)
+    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, empty=True)
+    # 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):

tests/core/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py

@@ -22,6 +22,9 @@ def run_process_rewards_and_penalties(spec, state):
 
 
 def prepare_state_with_full_attestations(spec, state, empty=False):
+    # If empty is true, attestations have 0 participants, and are not signed.
+    # Thus strictly speaking invalid when no participant is added later.
+
     # Go to start of next epoch to ensure can have full participation
     next_epoch(spec, state)