extended validation (#812)

* initial extended validation setup

* flesh out all TODO items for attestation and beaconblock verification

* fix finalization and add chronicles debugging messages

* directly use blockPool.headState rather than pointlessly updating it and document this constraint

* fix logic relating to first-attestation checking; support validating blocks across multiple forks

beacon_chain/attestation_aggregation.nim

@@ -30,10 +30,6 @@ import
 # TODO add tests, especially for validation
 # https://github.com/status-im/nim-beacon-chain/issues/122#issuecomment-562479965
 
-const
-  # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/p2p-interface.md#configuration
-  ATTESTATION_PROPAGATION_SLOT_RANGE = 32
-
 # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/validator.md#aggregation-selection
 func is_aggregator(state: BeaconState, slot: Slot, index: uint64,
     slot_signature: ValidatorSig): bool =

beacon_chain/attestation_pool.nim

@@ -1,5 +1,5 @@
 import
-  deques, sequtils, tables,
+  deques, sequtils, tables, options,
   chronicles, stew/[bitseqs, byteutils], json_serialization/std/sets,
   ./spec/[beaconstate, datatypes, crypto, digest, helpers, validator],
   ./extras, ./ssz, ./block_pool, ./beacon_node_types
@@ -35,6 +35,7 @@ proc combine*(tgt: var Attestation, src: Attestation, flags: UpdateFlags) =
   else:
     trace "Ignoring overlapping attestations"
 
+# TODO remove/merge with p2p-interface validation
 proc validate(
     state: BeaconState, attestation: Attestation): bool =
   # TODO what constitutes a valid attestation when it's about to be added to
@@ -265,26 +266,20 @@ proc add*(pool: var AttestationPool, attestation: Attestation) =
 
   pool.addResolved(blck, attestation)
 
-proc getAttestationsForBlock*(
-    pool: AttestationPool, state: BeaconState): seq[Attestation] =
-  ## Retrieve attestations that may be added to a new block at the slot of the
-  ## given state
-  logScope: pcs = "retrieve_attestation"
-
-  let newBlockSlot = state.slot
+proc getAttestationsForSlot(pool: AttestationPool, newBlockSlot: Slot):
+    Option[SlotData] =
   if newBlockSlot < (GENESIS_SLOT + MIN_ATTESTATION_INCLUSION_DELAY):
     debug "Too early for attestations",
       newBlockSlot = shortLog(newBlockSlot),
       cat = "query"
-    return
+    return none(SlotData)
 
   if pool.slots.len == 0: # startingSlot not set yet!
     info "No attestations found (pool empty)",
       newBlockSlot = shortLog(newBlockSlot),
       cat = "query"
-    return
+    return none(SlotData)
 
-  var cache = get_empty_per_epoch_cache()
   let
     # TODO in theory we could include attestations from other slots also, but
     # we're currently not tracking which attestations have already been included
@@ -300,12 +295,29 @@ proc getAttestationsForBlock*(
       startingSlot = shortLog(pool.startingSlot),
       endingSlot = shortLog(pool.startingSlot + pool.slots.len.uint64),
       cat = "query"
-    return
+    return none(SlotData)
 
+  let slotDequeIdx = int(attestationSlot - pool.startingSlot)
+  some(pool.slots[slotDequeIdx])
+
+proc getAttestationsForBlock*(
+    pool: AttestationPool, state: BeaconState): seq[Attestation] =
+  ## Retrieve attestations that may be added to a new block at the slot of the
+  ## given state
+  logScope: pcs = "retrieve_attestation"
+
+  # TODO this shouldn't really need state -- it's to recheck/validate, but that
+  # should be refactored
   let
-    slotDequeIdx = int(attestationSlot - pool.startingSlot)
-    slotData = pool.slots[slotDequeIdx]
+    newBlockSlot = state.slot
+    maybeSlotData = getAttestationsForSlot(pool, newBlockSlot)
 
+  if maybeSlotData.isNone:
+    # Logging done in getAttestationsForSlot(...)
+    return
+  let slotData = maybeSlotData.get
+
+  var cache = get_empty_per_epoch_cache()
   for a in slotData.attestations:
     var
       # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/validator.md#construct-attestation
@@ -438,3 +450,80 @@ proc selectHead*(pool: AttestationPool): BlockRef =
     lmdGhost(pool, pool.blockPool.justifiedState.data.data, justifiedHead.blck)
 
   newHead
+
+# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#attestation-subnets
+proc isValidAttestation*(
+    pool: AttestationPool, attestation: Attestation, current_slot: Slot,
+    topicCommitteeIndex: uint64, flags: UpdateFlags): bool =
+  # The attestation's committee index (attestation.data.index) is for the
+  # correct subnet.
+  if attestation.data.index != topicCommitteeIndex:
+    debug "isValidAttestation: attestation's committee index not for the correct subnet",
+      topicCommitteeIndex = topicCommitteeIndex,
+      attestation_data_index = attestation.data.index
+    return false
+
+  if not (attestation.data.slot + ATTESTATION_PROPAGATION_SLOT_RANGE >=
+      current_slot and current_slot >= attestation.data.slot):
+    debug "isValidAttestation: attestation.data.slot not within ATTESTATION_PROPAGATION_SLOT_RANGE"
+    return false
+
+  # The attestation is unaggregated -- that is, it has exactly one
+  # participating validator (len([bit for bit in attestation.aggregation_bits
+  # if bit == 0b1]) == 1).
+  # TODO a cleverer algorithm, along the lines of countOnes() in nim-stew
+  # But that belongs in nim-stew, since it'd break abstraction layers, to
+  # use details of its representation from nim-beacon-chain.
+  var onesCount = 0
+  for aggregation_bit in attestation.aggregation_bits:
+    if not aggregation_bit:
+      continue
+    onesCount += 1
+    if onesCount > 1:
+      debug "isValidAttestation: attestation has too many aggregation bits",
+        aggregation_bits = attestation.aggregation_bits
+      return false
+  if onesCount != 1:
+    debug "isValidAttestation: attestation has too few aggregation bits"
+    return false
+
+  # The attestation is the first valid attestation received for the
+  # participating validator for the slot, attestation.data.slot.
+  let maybeSlotData = getAttestationsForSlot(pool, attestation.data.slot)
+  if maybeSlotData.isSome:
+    for attestationEntry in maybeSlotData.get.attestations:
+      if attestation.data != attestationEntry.data:
+        continue
+      # Attestations might be aggregated eagerly or lazily; allow for both.
+      for validation in attestationEntry.validations:
+        if attestation.aggregation_bits.isSubsetOf(validation.aggregation_bits):
+          debug "isValidAttestation: attestation already exists at slot",
+            attestation_data_slot = attestation.data.slot,
+            attestation_aggregation_bits = attestation.aggregation_bits,
+            attestation_pool_validation = validation.aggregation_bits
+          return false
+
+  # The block being voted for (attestation.data.beacon_block_root) passes
+  # validation.
+  # We rely on the block pool to have been validated, so check for the
+  # existence of the block in the pool.
+  # TODO: consider a "slush pool" of attestations whose blocks have not yet
+  # propagated - i.e. imagine that attestations are smaller than blocks and
+  # therefore propagate faster, thus reordering their arrival in some nodes
+  if pool.blockPool.get(attestation.data.beacon_block_root).isNone():
+    debug "isValidAttestation: block doesn't exist in block pool",
+      attestation_data_beacon_block_root = attestation.data.beacon_block_root
+    return false
+
+  # The signature of attestation is valid.
+  # TODO need to know above which validator anyway, and this is too general
+  # as it supports aggregated attestations (which this can't be)
+  var cache = get_empty_per_epoch_cache()
+  if not is_valid_indexed_attestation(
+      pool.blockPool.headState.data.data,
+      get_indexed_attestation(
+        pool.blockPool.headState.data.data, attestation, cache), {}):
+    debug "isValidAttestation: signature verification failed"
+    return false
+
+  true

beacon_chain/beacon_node.nim

@@ -935,14 +935,31 @@ proc run*(node: BeaconNode) =
 
   waitFor node.network.subscribe(topicBeaconBlocks) do (signedBlock: SignedBeaconBlock):
     onBeaconBlock(node, signedBlock)
+  do (signedBlock: SignedBeaconBlock) -> bool:
+    let (afterGenesis, slot) = node.beaconClock.now.toSlot()
+    if not afterGenesis:
+      return false
+    node.blockPool.isValidBeaconBlock(signedBlock, slot, {})
 
-  waitFor allFutures(mapIt(
-    0'u64 ..< ATTESTATION_SUBNET_COUNT.uint64,
-    node.network.subscribe(getAttestationTopic(it)) do (attestation: Attestation):
-      # Avoid double-counting attestation-topic attestations on shared codepath
-      # when they're reflected through beacon blocks
-      beacon_attestations_received.inc()
-      node.onAttestation(attestation)))
+  proc attestationHandler(attestation: Attestation) =
+    # Avoid double-counting attestation-topic attestations on shared codepath
+    # when they're reflected through beacon blocks
+    beacon_attestations_received.inc()
+    node.onAttestation(attestation)
+
+  var attestationSubscriptions: seq[Future[void]] = @[]
+  for it in 0'u64 ..< ATTESTATION_SUBNET_COUNT.uint64:
+    closureScope:
+      let ci = it
+      attestationSubscriptions.add(node.network.subscribe(
+        getAttestationTopic(ci), attestationHandler,
+        proc(attestation: Attestation): bool =
+          # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#attestation-subnets
+          let (afterGenesis, slot) = node.beaconClock.now().toSlot()
+          if not afterGenesis:
+            return false
+          node.attestationPool.isValidAttestation(attestation, slot, ci, {})))
+  waitFor allFutures(attestationSubscriptions)
 
   let
     t = node.beaconClock.now().toSlot()

beacon_chain/beacon_node_types.nim

@@ -136,7 +136,10 @@ type
 
     inAdd*: bool
 
-    headState*: StateData ## State given by the head block
+    headState*: StateData ## \
+    ## State given by the head block; only update in `updateHead`, not anywhere
+    ## else via `withState`
+
     justifiedState*: StateData ## Latest justified state, as seen from the head
 
     tmpState*: StateData ## Scratchpad - may be any state

beacon_chain/block_pool.nim

@@ -956,3 +956,74 @@ proc getProposer*(pool: BlockPool, head: BlockRef, slot: Slot): Option[Validator
       return
 
     return some(state.validators[proposerIdx.get()].pubkey)
+
+# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#global-topics
+proc isValidBeaconBlock*(pool: BlockPool,
+    signed_beacon_block: SignedBeaconBlock, current_slot: Slot,
+    flags: UpdateFlags): bool =
+  # In general, checks are ordered from cheap to expensive. Especially, crypto
+  # verification could be quite a bit more expensive than the rest. This is an
+  # externally easy-to-invoke function by tossing network packets at the node.
+
+  # The block is not from a future slot
+  # TODO allow `MAXIMUM_GOSSIP_CLOCK_DISPARITY` leniency, especially towards
+  # seemingly future slots.
+  if not (signed_beacon_block.message.slot <= current_slot):
+    debug "isValidBeaconBlock: block is from a future slot",
+      signed_beacon_block_message_slot = signed_beacon_block.message.slot,
+      current_slot = current_slot
+    return false
+
+  # The block is from a slot greater than the latest finalized slot (with a
+  # MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance) -- i.e. validate that
+  # signed_beacon_block.message.slot >
+  # compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)
+  if not (signed_beacon_block.message.slot > pool.finalizedHead.slot):
+    debug "isValidBeaconBlock: block is not from a slot greater than the latest finalized slot"
+    return false
+
+  # The block is the first block with valid signature received for the proposer
+  # for the slot, signed_beacon_block.message.slot.
+  # TODO might check unresolved/orphaned blocks too, and this might not see all
+  # blocks at a given slot (though, in theory, those get checked elsewhere).
+  let slotBlockRef =
+    getBlockByPreciseSlot(pool, signed_beacon_block.message.slot)
+  if (not slotBlockRef.isNil) and
+      pool.get(slotBlockRef).data.message.proposer_index ==
+        signed_beacon_block.message.proposer_index:
+    debug "isValidBeaconBlock: block isn't first block with valid signature received for the proposer",
+      signed_beacon_block_message_slot = signed_beacon_block.message.slot,
+      blckRef = getBlockByPreciseSlot(pool, signed_beacon_block.message.slot)
+
+    return false
+
+  # The proposer signature, signed_beacon_block.signature, is valid with
+  # respect to the proposer_index pubkey.
+
+  # If this block doesn't have a parent we know about, we can't/don't really
+  # trace it back to a known-good state/checkpoint to verify its prevenance;
+  # while one could getOrResolve to queue up searching for missing parent it
+  # might not be the best place. As much as feasible, this function aims for
+  # answering yes/no, not queuing other action or otherwise altering state.
+  let parent_ref = pool.getRef(signed_beacon_block.message.parent_root)
+  if parent_ref.isNil:
+    return false
+
+  let bs =
+    BlockSlot(blck: parent_ref, slot: pool.get(parent_ref).data.message.slot)
+  pool.withState(pool.tmpState, bs):
+    let
+      blockRoot = hash_tree_root(signed_beacon_block.message)
+      domain = get_domain(pool.headState.data.data, DOMAIN_BEACON_PROPOSER,
+        compute_epoch_at_slot(signed_beacon_block.message.slot))
+      signing_root = compute_signing_root(blockRoot, domain)
+      proposer_index = signed_beacon_block.message.proposer_index
+
+    if proposer_index >= pool.headState.data.data.validators.len.uint64:
+      return false
+    if not blsVerify(pool.headState.data.data.validators[proposer_index].pubkey,
+        signing_root.data, signed_beacon_block.signature):
+      debug "isValidBeaconBlock: block failed signature verification"
+      return false
+
+  true

beacon_chain/eth2_network.nim

@@ -13,7 +13,7 @@ import
           multiaddress, multicodec, crypto/crypto,
           protocols/identify, protocols/protocol],
   libp2p/protocols/secure/[secure, secio],
-  libp2p/protocols/pubsub/[pubsub, floodsub],
+  libp2p/protocols/pubsub/[pubsub, floodsub, rpc/messages],
   libp2p/transports/[transport, tcptransport],
   libp2p/stream/lpstream,
   eth/[keys, async_utils], eth/p2p/[enode, p2p_protocol_dsl],
@@ -757,7 +757,7 @@ proc p2pProtocolBackendImpl*(p: P2PProtocol): Backend =
   result.afterProtocolInit = proc (p: P2PProtocol) =
     p.onPeerConnected.params.add newIdentDefs(streamVar, Connection)
 
-  result.implementMsg = proc (msg: Message) =
+  result.implementMsg = proc (msg: p2p_protocol_dsl.Message) =
     let
       protocol = msg.protocol
       msgName = $msg.ident
@@ -959,7 +959,8 @@ func peersCount*(node: Eth2Node): int =
 
 proc subscribe*[MsgType](node: Eth2Node,
                          topic: string,
-                         msgHandler: proc(msg: MsgType) {.gcsafe.} ) {.async, gcsafe.} =
+                         msgHandler: proc(msg: MsgType) {.gcsafe.},
+                         msgValidator: proc(msg: MsgType): bool {.gcsafe.} ) {.async, gcsafe.} =
   template execMsgHandler(peerExpr, gossipBytes, gossipTopic) =
     inc gossip_messages_received
     trace "Incoming pubsub message received",
@@ -967,6 +968,20 @@ proc subscribe*[MsgType](node: Eth2Node,
       message_id = `$`(sha256.digest(gossipBytes))
     msgHandler SSZ.decode(gossipBytes, MsgType)
 
+  # All message types which are subscribed to should be validated; putting
+  # this in subscribe(...) ensures that the default approach is correct.
+  template execMsgValidator(gossipBytes, gossipTopic): bool =
+    trace "Incoming pubsub message received for validation",
+      len = gossipBytes.len, topic = gossipTopic,
+      message_id = `$`(sha256.digest(gossipBytes))
+    msgValidator SSZ.decode(gossipBytes, MsgType)
+
+  # Validate messages as soon as subscribed
+  let incomingMsgValidator = proc(topic: string, message: messages.Message):
+      Future[bool] {.async, gcsafe.} =
+    return execMsgValidator(message.data, topic)
+  node.switch.addValidator(topic, incomingMsgValidator)
+
   let incomingMsgHandler = proc(topic: string,
                                 data: seq[byte]) {.async, gcsafe.} =
     execMsgHandler "unknown", data, topic

beacon_chain/spec/beaconstate.nim

@@ -412,7 +412,7 @@ func get_attesting_indices*(state: BeaconState,
       result.incl index
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/beacon-chain.md#get_indexed_attestation
-func get_indexed_attestation(state: BeaconState, attestation: Attestation,
+func get_indexed_attestation*(state: BeaconState, attestation: Attestation,
     stateCache: var StateCache): IndexedAttestation =
   # Return the indexed attestation corresponding to ``attestation``.
   let
@@ -420,14 +420,6 @@ func get_indexed_attestation(state: BeaconState, attestation: Attestation,
       get_attesting_indices(
         state, attestation.data, attestation.aggregation_bits, stateCache)
 
-  ## TODO No fundamental reason to do so many type conversions
-  ## verify_indexed_attestation checks for sortedness but it's
-  ## entirely a local artifact, seemingly; networking uses the
-  ## Attestation data structure, which can't be unsorted. That
-  ## the conversion here otherwise needs sorting is due to the
-  ## usage of HashSet -- order only matters in one place (that
-  ## 0.6.3 highlights and explicates) except in that the spec,
-  ## for no obvious reason, verifies it.
   IndexedAttestation(
     attesting_indices:
       sorted(mapIt(attesting_indices.toSeq, it.uint64), system.cmp),

beacon_chain/spec/datatypes.nim

@@ -74,6 +74,9 @@ const
     # TODO: This needs revisiting.
     # Why was the validator WITHDRAWAL_PERIOD altered in the spec?
 
+  # https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/p2p-interface.md#configuration
+  ATTESTATION_PROPAGATION_SLOT_RANGE* = 32
+
 template maxSize*(n: int) {.pragma.}
 
 type