rework process_attestation and work through tests

specs/phase1/beacon-chain.md

@@ -575,16 +575,16 @@ def process_operations(state: BeaconState, body: BeaconBlockBody) -> None:
 
     for_ops(body.proposer_slashings, process_proposer_slashing)
     for_ops(body.attester_slashings, process_attester_slashing)
-
     # New attestation processing
-    process_attestations(state, body, body.attestations)
-
+    for_ops(body.attestations, process_attestation)
     for_ops(body.deposits, process_deposit)
     for_ops(body.voluntary_exits, process_voluntary_exit)
 
     # See custody game spec.
     process_custody_game_operations(state, body)
 
+    process_crosslinks(state, body.shard_transitions, body.attestations)
+
     # TODO process_operations(body.shard_receipt_proofs, process_shard_receipt_proofs)
 ```
 
@@ -598,6 +598,7 @@ def validate_attestation(state: BeaconState, attestation: Attestation) -> None:
     assert data.index < get_committee_count_at_slot(state, data.slot)
     assert data.index < get_active_shard_count(state)
     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
 
     committee = get_beacon_committee(state, data.slot, data.index)
@@ -740,43 +741,40 @@ def process_crosslink_for_shard(state: BeaconState,
 
 ```python
 def process_crosslinks(state: BeaconState,
-                       block_body: BeaconBlockBody,
-                       attestations: Sequence[Attestation]) -> Set[Tuple[Shard, Root]]:
-    winners: Set[Tuple[Shard, Root]] = set()
+                       shard_transitions: Sequence[ShardTransition],
+                       attestations: Sequence[Attestation]) -> None:
     committee_count = get_committee_count_at_slot(state, state.slot)
     for committee_index in map(CommitteeIndex, range(committee_count)):
         shard = compute_shard_from_committee_index(state, committee_index, state.slot)
-        # All attestations in the block for this shard
+        # All attestations in the block for this committee/shard and current slot
         shard_attestations = [
             attestation for attestation in attestations
-            if get_shard(state, attestation) == shard and attestation.data.slot == state.slot
+            if attestation.data.index == committee_index and attestation.data.slot == state.slot
         ]
-        shard_transition = block_body.shard_transitions[shard]
+        shard_transition = shard_transitions[shard]
         winning_root = process_crosslink_for_shard(state, shard, shard_transition, shard_attestations)
         if winning_root != Root():
-            winners.add((shard, winning_root))
-    return winners
+            # Mark relevant pending attestations as creating a successful crosslink
+            for pending_attestation in state.current_epoch_attestations:
+                if (
+                    pending_attestation.slot == state.slot and pending_attestation
+                    and pending_attestation.data.index == committee_index
+                    and pending_attestation.data.shard_transition_root == winning_root
+                ):
+                    pending_attestation.crosslink_success = True
 ```
 
-###### `process_attestations`
+###### `process_attestation`
 
 ```python
-def process_attestations(state: BeaconState, block_body: BeaconBlockBody, attestations: Sequence[Attestation]) -> None:
-    # Basic validation
-    for attestation in attestations:
+def process_attestation(state: BeaconState, attestation: Attestation) -> None:
     validate_attestation(state, attestation)
-
-    # Process crosslinks
-    winners = process_crosslinks(state, block_body, attestations)
-
-    # Store pending attestations for epoch processing
-    for attestation in attestations:
-        is_winning_transition = (get_shard(state, attestation), attestation.data.shard_transition_root) in winners
+    # Store pending attestation for epoch processing
     pending_attestation = PendingAttestation(
         aggregation_bits=attestation.aggregation_bits,
         data=attestation.data,
         inclusion_delay=state.slot - attestation.data.slot,
-            crosslink_success=is_winning_transition and attestation.data.slot == state.slot,
+        crosslink_success=False,  # To be filled in during process_crosslinks
         proposer_index=get_beacon_proposer_index(state),
     )
     if attestation.data.target.epoch == get_current_epoch(state):

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

@@ -0,0 +1,80 @@
+from eth2spec.test.context import (
+    with_all_phases_except,
+    expect_assertion_error,
+    spec_state_test,
+    always_bls,
+)
+from eth2spec.test.helpers.attestations import (
+    get_valid_attestation,
+    sign_attestation,
+)
+
+from eth2spec.utils.ssz.ssz_typing import Bitlist
+
+
+def run_attestation_processing(spec, state, attestation, valid=True):
+    """
+    Run ``process_attestation``, yielding:
+      - pre-state ('pre')
+      - attestation ('attestation')
+      - post-state ('post').
+    If ``valid == False``, run expecting ``AssertionError``
+    """
+    # yield pre-state
+    yield 'pre', state
+
+    yield 'attestation', attestation
+
+    # If the attestation is invalid, processing is aborted, and there is no post-state.
+    if not valid:
+        expect_assertion_error(lambda: spec.process_attestation(state, attestation))
+        yield 'post', None
+        return
+
+    current_epoch_count = len(state.current_epoch_attestations)
+    previous_epoch_count = len(state.previous_epoch_attestations)
+
+    # process attestation
+    spec.process_attestation(state, attestation)
+
+    # Make sure the attestation has been processed
+    if attestation.data.target.epoch == spec.get_current_epoch(state):
+        assert len(state.current_epoch_attestations) == current_epoch_count + 1
+    else:
+        assert len(state.previous_epoch_attestations) == previous_epoch_count + 1
+
+    # yield post-state
+    yield 'post', state
+
+
+@with_all_phases_except(['phase0'])
+@spec_state_test
+@always_bls
+def test_success_empty_custody_bits_blocks(spec, state):
+    attestation = get_valid_attestation(spec, state)
+    attestation.custody_bits_blocks = []
+    sign_attestation(spec, state, attestation)
+
+    state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY
+
+    yield from run_attestation_processing(spec, state, attestation)
+
+
+@with_all_phases_except(['phase0'])
+@spec_state_test
+@always_bls
+def test_fail_custody_bits_blocks_incorrect_slot(spec, state):
+    attestation = get_valid_attestation(spec, state)
+    committee = spec.get_beacon_committee(
+        state,
+        attestation.data.slot,
+        attestation.data.index,
+    )
+    bitlist = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE]([0 for _ in range(len(committee))])
+    bitlist[0] = 1
+    attestation.custody_bits_blocks = [bitlist]
+    sign_attestation(spec, state, attestation)
+
+    state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY + 1
+
+    yield from run_attestation_processing(spec, state, attestation)