fix doppelganger detection logging

* update action tracker on dependent-root-changing reorg (instead of
epoch change)
* don't try to log duties while syncing - we're not tracking actions yet
* fix slot used for doppelganger loss detection

beacon_chain/nimbus_beacon_node.nim

@@ -927,13 +927,15 @@ proc updateGossipStatus(node: BeaconNode, slot: Slot) {.async.} =
     # it might also happen on a sufficiently fast restart
 
     # We "know" the actions for the current and the next epoch
-    if node.isSynced(head):
-      node.actionTracker.updateActions(
-        node.dag.getEpochRef(head, slot.epoch, false).expect(
-          "Getting head EpochRef should never fail"))
-      node.actionTracker.updateActions(
-        node.dag.getEpochRef(head, slot.epoch + 1, false).expect(
-          "Getting head EpochRef should never fail"))
+    if node.actionTracker.needsUpdate(slot.epoch, head, node.dag.tail):
+      let epochRef = node.dag.getEpochRef(head, slot.epoch, false).expect(
+        "Getting head EpochRef should never fail")
+      node.actionTracker.updateActions(epochRef, head, node.dag.tail)
+
+    if node.actionTracker.needsUpdate(slot.epoch + 1, head, node.dag.tail):
+      let epochRef = node.dag.getEpochRef(head, slot.epoch + 1, false).expect(
+        "Getting head EpochRef should never fail")
+      node.actionTracker.updateActions(epochRef, head, node.dag.tail)
 
   if node.gossipState.card > 0 and targetGossipState.card == 0:
     debug "Disabling topic subscriptions",
@@ -1002,16 +1004,12 @@ proc onSlotEnd(node: BeaconNode, slot: Slot) {.async.} =
     node.trackNextSyncCommitteeTopics(slot)
 
   # Update upcoming actions - we do this every slot in case a reorg happens
-  if node.isSynced(node.dag.head) and
-      node.actionTracker.lastCalculatedEpoch < slot.epoch + 1:
-    # TODO this is costly because we compute an EpochRef that likely will never
-    #      be used for anything else, due to the epoch ancestor being selected
-    #      pessimistically with respect to the shuffling - this needs fixing
-    #      at EpochRef level by not mixing balances and shufflings in the same
-    #      place
-    node.actionTracker.updateActions(node.dag.getEpochRef(
-      node.dag.head, slot.epoch + 1, false).expect(
-        "Getting head EpochRef should never fail"))
+  let head = node.dag.head
+  if node.isSynced(head):
+    if node.actionTracker.needsUpdate(slot.epoch + 1, head, node.dag.tail):
+      let epochRef = node.dag.getEpochRef(head, slot.epoch + 1, false).expect(
+        "Getting head EpochRef should never fail")
+      node.actionTracker.updateActions(epochRef, head, node.dag.tail)
 
   let
     nextAttestationSlot = node.actionTracker.getNextAttestationSlot(slot)
@@ -1036,7 +1034,7 @@ proc onSlotEnd(node: BeaconNode, slot: Slot) {.async.} =
         shortLog(nextActionWaitTime),
     nextAttestationSlot = displayInt64(nextAttestationSlot),
     nextProposalSlot = displayInt64(nextProposalSlot),
-    head = shortLog(node.dag.head)
+    head = shortLog(head)
 
   if nextAttestationSlot != FAR_FUTURE_SLOT:
     next_action_wait.set(nextActionWaitTime.toFloatSeconds)

beacon_chain/validators/action_tracker.nim

@@ -51,6 +51,10 @@ type
     proposingSlots*: array[2, uint32]
     lastCalculatedEpoch*: Epoch
 
+    dependentRoot*: Eth2Digest
+      ## The latest dependent root we used to compute attestation duties
+      ## for internal validators
+
     knownValidators*: Table[ValidatorIndex, Slot] ##\
       ## Validators that we've recently seen - we'll subscribe to one stability
       ## subnet for each such validator - the slot is used to expire validators
@@ -195,13 +199,27 @@ func getNextProposalSlot*(tracker: ActionTracker, slot: Slot): Slot =
     tracker.proposingSlots,
     tracker.lastCalculatedEpoch, slot)
 
-proc updateActions*(tracker: var ActionTracker, epochRef: EpochRef) =
+proc dependentRoot(epoch: Epoch, head, tail: BlockRef): Eth2Digest =
+  head.prevDependentBlock(tail, epoch).root
+
+proc needsUpdate*(
+    tracker: ActionTracker, epoch: Epoch, head, tail: BlockRef): bool =
+  # Using prevDependentBlock here means we lock the action tracking to
+  # the dependent root for attestation duties and not block proposal -
+  # however, the risk of a proposer reordering in the last epoch is small
+  # and the action tracker is speculative in nature.
+  tracker.dependentRoot != dependentRoot(epoch, head, tail)
+
+proc updateActions*(
+    tracker: var ActionTracker, epochRef: EpochRef, head, tail: BlockRef) =
   # Updates the schedule for upcoming attestation and proposal work
   let
     epoch = epochRef.epoch
 
-  if tracker.lastCalculatedEpoch == epoch:
+  if not tracker.needsUpdate(epoch, head, tail):
     return
+
+  tracker.dependentRoot = dependentRoot(epoch, head, tail)
   tracker.lastCalculatedEpoch = epoch
 
   let validatorIndices = toHashSet(toSeq(tracker.knownValidators.keys()))

beacon_chain/validators/validator_duties.nim

@@ -1015,15 +1015,8 @@ proc handleValidatorDuties*(node: BeaconNode, lastSlot, slot: Slot) {.async.} =
   # await calls, thus we use a local variable to keep the logic straight here
   var head = node.dag.head
   if not node.isSynced(head):
-    let
-      nextAttestationSlot = node.actionTracker.getNextAttestationSlot(slot)
-      nextProposalSlot = node.actionTracker.getNextProposalSlot(slot)
-    if slot in [nextAttestationSlot, nextProposalSlot]:
-      notice "Syncing in progress; skipping validator duties for now",
-        slot, headSlot = head.slot
-    else:
-      debug "Syncing in progress; skipping validator duties for now",
-        slot, headSlot = head.slot
+    info "Syncing in progress; skipping validator duties for now",
+      slot, headSlot = head.slot
 
     # Rewards will be growing though, as we sync..
     updateValidatorMetrics(node)
@@ -1038,17 +1031,17 @@ proc handleValidatorDuties*(node: BeaconNode, lastSlot, slot: Slot) {.async.} =
         node.processor[].doppelgangerDetection.broadcastStartEpoch and
       node.config.doppelgangerDetection:
     let
-      nextAttestationSlot = node.actionTracker.getNextAttestationSlot(slot)
-      nextProposalSlot = node.actionTracker.getNextProposalSlot(slot)
+      nextAttestationSlot = node.actionTracker.getNextAttestationSlot(slot - 1)
+      nextProposalSlot = node.actionTracker.getNextProposalSlot(slot - 1)
 
     if slot in [nextAttestationSlot, nextProposalSlot]:
       notice "Doppelganger detection active - skipping validator duties while observing activity on the network",
-        slot, epoch = slot.epoch, nextAttestationSlot, nextProposalSlot,
+        slot, epoch = slot.epoch,
         broadcastStartEpoch =
           node.processor[].doppelgangerDetection.broadcastStartEpoch
     else:
       debug "Doppelganger detection active - skipping validator duties while observing activity on the network",
-        slot, epoch = slot.epoch, nextAttestationSlot, nextProposalSlot,
+        slot, epoch = slot.epoch,
         broadcastStartEpoch =
           node.processor[].doppelgangerDetection.broadcastStartEpoch