simplify validator api (#573)

* avoid passing whole state to validator api (easier when it goes
out-of-process)
* clean up some cruft

beacon_chain/beacon_node.nim

@@ -287,7 +287,7 @@ proc updateHead(node: BeaconNode, slot: Slot): BlockRef =
   newHead
 
 proc sendAttestation(node: BeaconNode,
-                     state: BeaconState,
+                     fork: Fork,
                      validator: AttachedValidator,
                      attestationData: AttestationData,
                      committeeLen: int,
@@ -295,7 +295,7 @@ proc sendAttestation(node: BeaconNode,
   logScope: pcs = "send_attestation"
 
   let
-    validatorSignature = await validator.signAttestation(attestationData, state)
+    validatorSignature = await validator.signAttestation(attestationData, fork)
 
   var aggregationBits = CommitteeValidatorsBits.init(committeeLen)
   aggregationBits.raiseBit indexInCommittee
@@ -374,8 +374,9 @@ proc proposeBlock(node: BeaconNode,
     # To create a block, we'll first apply a partial block to the state, skipping
     # some validations.
     let
+      fork = state.fork
       blockBody = BeaconBlockBody(
-        randao_reveal: validator.genRandaoReveal(state, slot),
+        randao_reveal: validator.genRandaoReveal(fork, slot),
         eth1_data: eth1data,
         attestations:
           node.attestationPool.getAttestationsForBlock(state, slot),
@@ -388,10 +389,7 @@ proc proposeBlock(node: BeaconNode,
         body: blockBody,
         # TODO: This shouldn't be necessary if OpaqueBlob is the default
         signature: ValidatorSig(kind: OpaqueBlob))
-
-    var
       tmpState = hashedState
-
     discard state_transition(tmpState, newBlock, {skipValidation})
     # TODO only enable in fast-fail debugging situations
     # otherwise, bad attestations can bring down network
@@ -402,8 +400,11 @@ proc proposeBlock(node: BeaconNode,
     let blockRoot = signing_root(newBlock)
 
     # Careful, state no longer valid after here..
+    # We use the fork from the pre-newBlock state which should be fine because
+    # fork carries two epochs, so even if it's a fork block, the right thing
+    # will happen here
     newBlock.signature =
-      await validator.signBlockProposal(state, slot, blockRoot)
+      await validator.signBlockProposal(fork, slot, blockRoot)
 
     (blockRoot, newBlock)
 
@@ -551,7 +552,7 @@ proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) =
 
     for a in attestations:
       traceAsyncErrors sendAttestation(
-        node, state, a.validator, a.data, a.committeeLen, a.indexInCommittee)
+        node, state.fork, a.validator, a.data, a.committeeLen, a.indexInCommittee)
 
 proc handleProposal(node: BeaconNode, head: BlockRef, slot: Slot):
     Future[BlockRef] {.async.} =
@@ -1052,9 +1053,6 @@ when isMainModule:
         config.depositPrivateKey)
 
   of query:
-    var
-      trieDB = trieDB newChainDb(config.databaseDir)
-
     case config.queryCmd
     of QueryCmd.nimQuery:
       # TODO: This will handle a simple subset of Nim using

beacon_chain/spec/beaconstate.nim

@@ -345,15 +345,17 @@ proc is_valid_indexed_attestation*(
     return false
 
   # Verify aggregate signature
-  result = bls_verify(
+  if not bls_verify(
     bls_aggregate_pubkeys(mapIt(indices, state.validators[it.int].pubkey)),
     hash_tree_root(indexed_attestation.data).data,
     indexed_attestation.signature,
     get_domain(
       state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
-  )
+  ):
-  if not result:
     notice "indexed attestation: signature verification failure"
+    return false
+
+  true
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.9.1/specs/core/0_beacon-chain.md#get_attesting_indices
 func get_attesting_indices*(state: BeaconState,

beacon_chain/spec/helpers.nim

@@ -122,17 +122,24 @@ func compute_domain*(
   result[4..7] = fork_version
 
 # https://github.com/ethereum/eth2.0-specs/blob/v0.9.1/specs/core/0_beacon-chain.md#get_domain
+
+func get_domain*(
+    fork: Fork, domain_type: DomainType, epoch: Epoch): Domain =
+  ## Return the signature domain (fork version concatenated with domain type)
+  ## of a message.
+  let
+    fork_version =
+      if epoch < fork.epoch:
+        fork.previous_version
+      else:
+        fork.current_version
+  compute_domain(domain_type, fork_version)
+
 func get_domain*(
     state: BeaconState, domain_type: DomainType, message_epoch: Epoch): Domain =
   ## Return the signature domain (fork version concatenated with domain type)
   ## of a message.
-  let
+  get_domain(state.fork, domain_type, message_epoch)
-    epoch = message_epoch
-    fork_version = if epoch < state.fork.epoch:
-        state.fork.previous_version
-      else:
-        state.fork.current_version
-  compute_domain(domain_type, fork_version)
 
 func get_domain*(state: BeaconState, domain_type: DomainType): Domain =
   get_domain(state, domain_type, get_current_epoch(state))

beacon_chain/validator_pool.nim

@@ -26,7 +26,7 @@ func getValidator*(pool: ValidatorPool,
                    validatorKey: ValidatorPubKey): AttachedValidator =
   pool.validators.getOrDefault(validatorKey)
 
-proc signBlockProposal*(v: AttachedValidator, state: BeaconState, slot: Slot,
+proc signBlockProposal*(v: AttachedValidator, fork: Fork, slot: Slot,
                         blockRoot: Eth2Digest): Future[ValidatorSig] {.async.} =
 
   if v.kind == inProcess:
@@ -34,7 +34,7 @@ proc signBlockProposal*(v: AttachedValidator, state: BeaconState, slot: Slot,
     #      care about this in here
     let
       domain =
-        get_domain(state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(slot))
+        get_domain(fork, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(slot))
     # TODO this is an ugly hack to fake a delay and subsequent async reordering
     #      for the purpose of testing the external validator delay - to be
     #      replaced by something more sensible
@@ -47,11 +47,11 @@ proc signBlockProposal*(v: AttachedValidator, state: BeaconState, slot: Slot,
 
 proc signAttestation*(v: AttachedValidator,
                       attestation: AttestationData,
-                      state: BeaconState): Future[ValidatorSig] {.async.} =
+                      fork: Fork): Future[ValidatorSig] {.async.} =
   if v.kind == inProcess:
     let
       attestationRoot = hash_tree_root(attestation)
-      domain = get_domain(state, DOMAIN_BEACON_ATTESTER, attestation.target.epoch)
+      domain = get_domain(fork, DOMAIN_BEACON_ATTESTER, attestation.target.epoch)
 
     # TODO this is an ugly hack to fake a delay and subsequent async reordering
     #      for the purpose of testing the external validator delay - to be
@@ -63,14 +63,14 @@ proc signAttestation*(v: AttachedValidator,
     error "Unimplemented"
     quit 1
 
-func genRandaoReveal*(k: ValidatorPrivKey, state: BeaconState, slot: Slot):
+func genRandaoReveal*(k: ValidatorPrivKey, fork: Fork, slot: Slot):
     ValidatorSig =
   let
-    domain = get_domain(state, DOMAIN_RANDAO, compute_epoch_at_slot(slot))
+    domain = get_domain(fork, DOMAIN_RANDAO, compute_epoch_at_slot(slot))
     root = hash_tree_root(compute_epoch_at_slot(slot).uint64).data
 
   bls_sign(k, root, domain)
 
-func genRandaoReveal*(v: AttachedValidator, state: BeaconState, slot: Slot):
+func genRandaoReveal*(v: AttachedValidator, fork: Fork, slot: Slot):
     ValidatorSig =
-  genRandaoReveal(v.privKey, state, slot)
+  genRandaoReveal(v.privKey, fork, slot)

tests/testutil.nim

@@ -75,17 +75,6 @@ func makeInitialDeposits*(
   for i in 0..<n.int:
     result.add makeDeposit(i, flags)
 
-func getNextBeaconProposerIndex*(state: BeaconState): ValidatorIndex =
-  # TODO: This is a special version of get_beacon_proposer_index that takes into
-  #       account the partial update done at the start of slot processing -
-  #       see get_shard_committees_index
-  var
-    next_state = state
-    cache = get_empty_per_epoch_cache()
-
-  next_state.slot += 1
-  get_beacon_proposer_index(next_state, cache)
-
 proc addBlock*(
     state: var BeaconState, previous_block_root: Eth2Digest,
     body: BeaconBlockBody, flags: UpdateFlags = {}): BeaconBlock =
@@ -106,7 +95,7 @@ proc addBlock*(
 
   # TODO ugly hack; API needs rethinking
   var new_body = body
-  new_body.randao_reveal = privKey.genRandaoReveal(state, state.slot + 1)
+  new_body.randao_reveal = privKey.genRandaoReveal(state.fork, state.slot + 1)
   new_body.eth1_data = Eth1Data()
 
   var
@@ -128,21 +117,22 @@ proc addBlock*(
   # can set the state root in order to be able to create a valid signature
   new_block.state_root = hash_tree_root(state)
 
-  let proposerPrivkey = hackPrivKey(proposer)
+  doAssert privKey.pubKey() == proposer.pubkey,
-  doAssert proposerPrivkey.pubKey() == proposer.pubkey,
     "signature key should be derived from private key! - wrong privkey?"
 
   if skipValidation notin flags:
     let block_root = signing_root(new_block)
     # We have a signature - put it in the block and we should be done!
     new_block.signature =
-      bls_sign(proposerPrivkey, block_root.data,
+      bls_sign(privKey, block_root.data,
-               get_domain(state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(new_block.slot)))
+               get_domain(state, DOMAIN_BEACON_PROPOSER,
+               compute_epoch_at_slot(new_block.slot)))
 
     doAssert bls_verify(
       proposer.pubkey,
       block_root.data, new_block.signature,
-      get_domain(state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(new_block.slot))),
+      get_domain(
+        state, DOMAIN_BEACON_PROPOSER, compute_epoch_at_slot(new_block.slot))),
       "we just signed this message - it should pass verification!"
 
   new_block