can finalize successfully with external VCs! + some other cleanup

beacon_chain/spec/eth2_apis/validator_callsigs.nim

@@ -26,7 +26,10 @@ proc post_v1_beacon_blocks(body: SignedBeaconBlock): bool
 
 proc get_v1_validator_attestation_data(slot: Slot, committee_index: CommitteeIndex): AttestationData
 
-proc get_v1_validator_aggregate_attestation(attestation_data_root: Eth2Digest): Attestation
+# TODO at the time of writing (10.06.2020) the API specifies this call to have a hash of
+# the attestation data instead of the object itself but we also need the slot.. see here:
+# https://docs.google.com/spreadsheets/d/1kVIx6GvzVLwNYbcd-Fj8YUlPf4qGrWUlS35uaTnIAVg/edit?disco=AAAAGh7r_fQ
+proc get_v1_validator_aggregate_attestation(attestation_data: AttestationData): Attestation
 
 # TODO returns a bool even though in the API there is no return type - because of nim-json-rpc
 proc post_v1_validator_aggregate_and_proof(payload: SignedAggregateAndProof): bool

beacon_chain/spec/validator.nim

@@ -232,16 +232,6 @@ func get_beacon_proposer_index*(state: BeaconState, cache: var StateCache):
     Option[ValidatorIndex] =
   get_beacon_proposer_index(state, cache, state.slot)
 
-# Not from spec
-# TODO: cache the results from this and reuse in subsequent calls to get_beacon_proposer_index
-func get_beacon_proposer_indexes_for_epoch*(state: BeaconState, epoch: Epoch,
-    stateCache: var StateCache): seq[tuple[s: Slot, i: ValidatorIndex]] =
-  for i in 0 ..< SLOTS_PER_EPOCH:
-    let currSlot = (compute_start_slot_at_epoch(epoch).int + i).Slot
-    let idx = get_beacon_proposer_index(state, stateCache, currSlot)
-    if idx.isSome:
-      result.add (currSlot, idx.get)
-
 # https://github.com/ethereum/eth2.0-specs/blob/v0.11.3/specs/phase0/validator.md#validator-assignments
 func get_committee_assignment*(
     state: BeaconState, epoch: Epoch, validator_index: ValidatorIndex):

beacon_chain/validator_api.nim

@@ -7,7 +7,7 @@
 
 import
   # Standard library
-  tables, strutils, sequtils,
+  tables, strutils,
 
   # Nimble packages
   stew/[objects],
@@ -15,7 +15,7 @@ import
   chronicles,
 
   # Local modules
-  spec/[datatypes, digest, crypto, validator, beaconstate],
+  spec/[datatypes, digest, crypto, validator, beaconstate, helpers],
   block_pool, ssz/merkleization,
   beacon_node_common, beacon_node_types,
   validator_duties, eth2_network,
@@ -25,6 +25,8 @@ import
 type
   RpcServer* = RpcHttpServer
 
+logScope: topics = "valapi"
+
 proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
 
   # TODO Probably the `beacon` ones (and not `validator`) should be defined elsewhere...
@@ -75,9 +77,7 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
       node, valInfo, proposer.get()[0], graffiti, head, slot)
 
     # TODO how do we handle the case when we cannot return a meaningful block? 404...
-    # currently this fails often - perhaps because the block has already been
-    # processed and signed with the inProcess validator...
-    # doAssert(res.message.isSome())
+    doAssert(res.message.isSome())
     return res.message.get(BeaconBlock()) # returning a default if empty
 
   rpcServer.rpc("post_v1_beacon_blocks") do (body: SignedBeaconBlock) -> bool:
@@ -106,13 +106,13 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
       return makeAttestationData(state, slot, committee_index.uint64, blck.root)
 
   rpcServer.rpc("get_v1_validator_aggregate_attestation") do (
-      attestation_data_root: Eth2Digest)-> Attestation:
+      attestation_data: AttestationData)-> Attestation:
     notice "== get_v1_validator_aggregate_attestation"
-    # TODO look at attestation.data.beacon_block_root
 
   rpcServer.rpc("post_v1_validator_aggregate_and_proof") do (
       payload: SignedAggregateAndProof) -> bool:
     notice "== post_v1_validator_aggregate_and_proof"
+    # TODO is this enough?
     node.network.broadcast(node.topicAggregateAndProofs, payload)
     return true
 
@@ -137,15 +137,15 @@ proc installValidatorApiHandlers*(rpcServer: RpcServer, node: BeaconNode) =
   rpcServer.rpc("get_v1_validator_duties_proposer") do (
       epoch: Epoch) -> seq[ValidatorPubkeySlotPair]:
     notice "== get_v1_validator_duties_proposer", epoch = epoch
-    discard node.updateHead() # TODO do we need this?
-    var cache = get_empty_per_epoch_cache()
-    result = get_beacon_proposer_indexes_for_epoch(node.blockPool.headState.data.data,
-      epoch, cache).mapIt(ValidatorPubkeySlotPair(
-        public_key: node.blockPool.headState.data.data.validators[it.i].pubkey,
-        slot: it.s
-      ))
+    let head = node.updateHead()
+    for i in 0 ..< SLOTS_PER_EPOCH:
+      let currSlot = (compute_start_slot_at_epoch(epoch).int + i).Slot
+      let proposer = node.blockPool.getProposer(head, currSlot)
+      if proposer.isSome():
+        result.add(ValidatorPubkeySlotPair(public_key: proposer.get()[1], slot: currSlot))
 
   rpcServer.rpc("post_v1_validator_beacon_committee_subscription") do (
       committee_index: CommitteeIndex, slot: Slot, aggregator: bool,
       validator_pubkey: ValidatorPubKey, slot_signature: ValidatorSig):
     notice "== post_v1_validator_beacon_committee_subscription"
+    # TODO

beacon_chain/validator_client.nim

@@ -25,6 +25,8 @@ import
   spec/eth2_apis/validator_callsigs_types,
   eth2_json_rpc_serialization
 
+logScope: topics = "vc"
+
 template sourceDir: string = currentSourcePath.rsplit(DirSep, 1)[0]
 
 ## Generate client convenience marshalling wrappers from forward declarations
@@ -41,11 +43,6 @@ type
     attestationsForEpoch: Table[Slot, seq[AttesterDuties]]
     beaconGenesis: BeaconGenesisTuple
 
-# TODO remove this and move to real logging once done experimenting - it's much
-# easier to distinguish such output from the one from chronicles with timestamps
-proc port_logged(vc: ValidatorClient, msg: string) =
-  echo "== ", vc.config.rpcPort, " ", msg
-
 proc getValidatorDutiesForEpoch(vc: ValidatorClient, epoch: Epoch) {.gcsafe, async.} =
   let proposals = await vc.client.get_v1_validator_duties_proposer(epoch)
   # update the block proposal duties this VC should do during this epoch
@@ -80,7 +77,12 @@ proc onSlotStart(vc: ValidatorClient, lastSlot, scheduledSlot: Slot) {.gcsafe, a
     slot = wallSlot.slot # afterGenesis == true!
     nextSlot = slot + 1
 
-  vc.port_logged "WAKE UP! scheduledSlot " & $scheduledSlot & " slot " & $slot
+  info "Slot start",
+    lastSlot = shortLog(lastSlot),
+    scheduledSlot = shortLog(scheduledSlot),
+    beaconTime = shortLog(beaconTime),
+    portBN = vc.config.rpcPort,
+    cat = "scheduling"
 
   try:
     # at the start of each epoch - request all validator duties
@@ -138,8 +140,22 @@ proc onSlotStart(vc: ValidatorClient, lastSlot, scheduledSlot: Slot) {.gcsafe, a
   let
     nextSlotStart = saturate(vc.beaconClock.fromNow(nextSlot))
 
-  # it's much easier to wake up on every slot compared to scheduling the start of each
-  # epoch and only the precise slots when the VC should sign/propose/attest with a key
+  info "Slot end",
+    slot = shortLog(slot),
+    nextSlot = shortLog(nextSlot),
+    portBN = vc.config.rpcPort,
+    cat = "scheduling"
+
+  when declared(GC_fullCollect):
+    # The slots in the validator client work as frames in a game: we want to make
+    # sure that we're ready for the next one and don't get stuck in lengthy
+    # garbage collection tasks when time is of essence in the middle of a slot -
+    # while this does not guarantee that we'll never collect during a slot, it
+    # makes sure that all the scratch space we used during slot tasks (logging,
+    # temporary buffers etc) gets recycled for the next slot that is likely to
+    # need similar amounts of memory.
+    GC_fullCollect()
+
   addTimer(nextSlotStart) do (p: pointer):
     asyncCheck vc.onSlotStart(slot, nextSlot)