avoid bad blocks by filtering included attestations

* fix epoch violation during initial attestation validation
* fill in attestation fields (finalizes again!)

beacon_chain/attestation_pool.nim

@@ -67,11 +67,6 @@ proc validate(
     notice "Empty aggregation bitfield"
     return false
 
-  let crosslink_committee = mapIt(
-    filterIt(get_crosslink_committees_at_slot(state, attestation.data.slot),
-             it.shard == attestation.data.shard),
-    it.committee)[0]
-
   ## the rest; turns into expensive NOP until then.
   if skipValidation notin flags:
     let
@@ -238,8 +233,9 @@ proc add*(pool: var AttestationPool,
           attestation: attestation,
         )
 
-proc getAttestationsForBlock*(pool: AttestationPool,
-                              newBlockSlot: Slot): seq[Attestation] =
+proc getAttestationsForBlock*(
+  pool: AttestationPool, state: BeaconState,
+    newBlockSlot: Slot): seq[Attestation] =
   if newBlockSlot - GENESIS_SLOT < MIN_ATTESTATION_INCLUSION_DELAY:
     debug "Too early for attestations",
       newBlockSlot = humaneSlotNum(newBlockSlot)
@@ -280,6 +276,15 @@ proc getAttestationsForBlock*(pool: AttestationPool,
         aggregate_signature: a.validations[0].aggregate_signature
       )
 
+    # TODO what's going on here is that when producing a block, we need to
+    #      include only such attestations that will not cause block validation
+    #      to fail. How this interacts with voting and the acceptance of
+    #      attestations into the pool in general is an open question that needs
+    #      revisiting - for example, when attestations are added, against which
+    #      state should they be validated, if at all?
+    if not checkAttestation(state, attestation, {skipValidation, nextSlot}):
+      continue
+
     for v in a.validations[1..^1]:
       if not attestation.aggregation_bitfield.overlaps(
           v.aggregation_bitfield):

beacon_chain/beacon_node.nim

@@ -340,7 +340,8 @@ proc proposeBlock(node: BeaconNode,
     blockBody = BeaconBlockBody(
       randao_reveal: validator.genRandaoReveal(node.state.data, slot),
       eth1_data: node.mainchainMonitor.getBeaconBlockRef(),
-      attestations: node.attestationPool.getAttestationsForBlock(slot))
+      attestations:
+        node.attestationPool.getAttestationsForBlock(node.state.data, slot))
 
   var
     newBlock = BeaconBlock(
@@ -387,6 +388,12 @@ proc onAttestation(node: BeaconNode, attestation: Attestation) =
     attestationData = shortLog(attestation.data),
     signature = shortLog(attestation.aggregate_signature)
 
+  # TODO seems reasonable to use the latest head state here.. needs thinking
+  #      though - maybe we should use the state from the block pointed to by
+  #      the attestation for some of the check? Consider interop with block
+  #      production!
+  node.blockPool.updateState(node.state,
+    BlockSlot(blck: node.blockPool.head, slot: node.beaconClock.now().toSlot()))
   node.attestationPool.add(node.state.data, attestation)
 
 proc onBeaconBlock(node: BeaconNode, blck: BeaconBlock) =

beacon_chain/extras.nim

@@ -18,5 +18,12 @@ type
     ## primary use case for this flag is when a proposer must propose a new
     ## block - in order to do so, it needs to update the state as if the block
     ## was valid, before it can sign it.
+    nextSlot ##\
+    ## Perform the operation as if the next slot was being processed - this is
+    ## useful when using the state to verify data that will go in the next slot,
+    ## for example when proposing
+    ## TODO need to be careful here, easy to assume that slot number change is
+    ##      enough, vs advancing the state - however, making a full state copy
+    ##      is expensive also :/
 
   UpdateFlags* = set[UpdateFlag]

beacon_chain/spec/beaconstate.nim

@@ -490,58 +490,62 @@ func update_validator_registry*(state: var BeaconState) =
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.5.1/specs/core/0_beacon-chain.md#attestations
 proc checkAttestation*(
-    state: var BeaconState, attestation: Attestation, flags: UpdateFlags): bool =
+    state: BeaconState, attestation: Attestation, flags: UpdateFlags): bool =
   ## Check that an attestation follows the rules of being included in the state
   ## at the current slot. When acting as a proposer, the same rules need to
   ## be followed!
 
+  let stateSlot =
+    if nextSlot in flags: state.slot + 1
+    else: state.slot
+
   # Can't submit attestations that are too far in history (or in prehistory)
   if not (attestation.data.slot >= GENESIS_SLOT):
     warn("Attestation predates genesis slot",
       attestation_slot = attestation.data.slot,
-      state_slot = humaneSlotNum(state.slot))
+      state_slot = humaneSlotNum(stateSlot))
     return
 
-  if not (state.slot <= attestation.data.slot + SLOTS_PER_EPOCH):
+  if not (stateSlot <= attestation.data.slot + SLOTS_PER_EPOCH):
     warn("Attestation too old",
       attestation_slot = humaneSlotNum(attestation.data.slot),
-      state_slot = humaneSlotNum(state.slot))
+      state_slot = humaneSlotNum(stateSlot))
     return
 
   # Can't submit attestations too quickly
   if not (
-      attestation.data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot):
+      attestation.data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot):
     warn("Can't submit attestations too quickly",
       attestation_slot = humaneSlotNum(attestation.data.slot),
-      state_slot = humaneSlotNum(state.slot))
+      state_slot = humaneSlotNum(stateSlot))
     return
 
   # # Verify that the justified epoch and root is correct
-  if slot_to_epoch(attestation.data.slot) >= get_current_epoch(state):
+  if slot_to_epoch(attestation.data.slot) >= stateSlot.slot_to_epoch():
     # Case 1: current epoch attestations
     if not (attestation.data.source_epoch == state.current_justified_epoch):
       warn("Source epoch is not current justified epoch",
         attestation_slot = humaneSlotNum(attestation.data.slot),
-        state_slot = humaneSlotNum(state.slot))
+        state_slot = humaneSlotNum(stateSlot))
       return
 
     if not (attestation.data.source_root == state.current_justified_root):
       warn("Source root is not current justified root",
         attestation_slot = humaneSlotNum(attestation.data.slot),
-        state_slot = humaneSlotNum(state.slot))
+        state_slot = humaneSlotNum(stateSlot))
       return
   else:
     # Case 2: previous epoch attestations
     if not (attestation.data.source_epoch == state.previous_justified_epoch):
       warn("Source epoch is not previous justified epoch",
         attestation_slot = humaneSlotNum(attestation.data.slot),
-        state_slot = humaneSlotNum(state.slot))
+        state_slot = humaneSlotNum(stateSlot))
       return
 
     if not (attestation.data.source_root == state.previous_justified_root):
       warn("Source root is not previous justified root",
         attestation_slot = humaneSlotNum(attestation.data.slot),
-        state_slot = humaneSlotNum(state.slot))
+        state_slot = humaneSlotNum(stateSlot))
       return
 
   # Check that the crosslink data is valid
@@ -641,20 +645,19 @@ proc makeAttestationData*(
   ## Head must be the head state during the slot that validator is
   ## part of committee - notably, it can't be a newer or older state (!)
 
-  # TODO update when https://github.com/ethereum/eth2.0-specs/issues/742
   let
     epoch_start_slot = get_epoch_start_slot(slot_to_epoch(state.slot))
-    epoch_boundary_root =
+    target_root =
       if epoch_start_slot == state.slot: beacon_block_root
       else: get_block_root(state, epoch_start_slot)
-    justified_slot = get_epoch_start_slot(state.current_justified_epoch)
-    justified_block_root = get_block_root(state, justified_slot)
 
   AttestationData(
     slot: state.slot,
     shard: shard,
     beacon_block_root: beacon_block_root,
+    target_root: target_root,
     crosslink_data_root: Eth2Digest(), # Stub in phase0
     previous_crosslink: state.latest_crosslinks[shard],
     source_epoch: state.current_justified_epoch,
+    source_root: state.current_justified_root
   )