fall back to non-fcu fork choice on epoch boundaries (#5195)

* fall back to non-fcu fork choice on epoch boundaries

* Future[bool]

* fix

* Update beacon_chain/consensus_object_pools/consensus_manager.nim

Co-authored-by: Etan Kissling <etan@status.im>

* make things consistent with Opt[void] return

---------

Co-authored-by: Etan Kissling <etan@status.im>

beacon_chain/consensus_object_pools/consensus_manager.nim

@@ -340,11 +340,11 @@ proc getGasLimit*(
 from ../spec/datatypes/bellatrix import PayloadID
 
 proc runProposalForkchoiceUpdated*(
-    self: ref ConsensusManager, wallSlot: Slot) {.async.} =
+    self: ref ConsensusManager, wallSlot: Slot): Future[Opt[void]] {.async.} =
   let
     nextWallSlot = wallSlot + 1
     (validatorIndex, nextProposer) = self.checkNextProposer(wallSlot).valueOr:
-      return
+      return err()
   debug "runProposalForkchoiceUpdated: expected to be proposing next slot",
     nextWallSlot, validatorIndex, nextProposer
 
@@ -353,7 +353,7 @@ proc runProposalForkchoiceUpdated*(
   if nextWallSlot.is_epoch:
     debug "runProposalForkchoiceUpdated: not running early fcU for epoch-aligned proposal slot",
       nextWallSlot, validatorIndex, nextProposer
-    return
+    return err()
 
   # Approximately lines up with validator_duties version. Used optimistically/
   # opportunistically, so mismatches are fine if not too frequent.
@@ -382,7 +382,7 @@ proc runProposalForkchoiceUpdated*(
     headBlockHash = self.dag.loadExecutionBlockHash(beaconHead.blck)
 
   if headBlockHash.isZero:
-    return
+    return err()
 
   try:
     let safeBlockHash = beaconHead.safeExecutionPayloadHash
@@ -410,6 +410,8 @@ proc runProposalForkchoiceUpdated*(
   except CatchableError as err:
     error "Engine API fork-choice update failed", err = err.msg
 
+  ok()
+
 proc updateHeadWithExecution*(
     self: ref ConsensusManager, initialNewHead: BeaconHead,
     getBeaconTimeFn: GetBeaconTimeFn) {.async.} =
@@ -455,7 +457,7 @@ proc updateHeadWithExecution*(
     # needs while runProposalForkchoiceUpdated requires RANDAO information
     # from the head state corresponding to the `newHead` block, which only
     # self.dag.updateHead(...) sets up.
-    await self.runProposalForkchoiceUpdated(getBeaconTimeFn().slotOrZero)
+    discard await self.runProposalForkchoiceUpdated(getBeaconTimeFn().slotOrZero)
 
     self[].checkExpectedBlock()
   except CatchableError as exc:

beacon_chain/gossip_processing/block_processor.nim

@@ -638,30 +638,33 @@ proc storeBlock*(
         # `forkchoiceUpdated` necessary for EL client only.
         self.consensusManager[].updateHead(newHead.get.blck)
 
-        if self.consensusManager.checkNextProposer(wallSlot).isNone:
-          # No attached validator is next proposer, so use non-proposal fcU
-
-          template callForkchoiceUpdated(payloadAttributeType: untyped): auto =
-            await elManager.expectValidForkchoiceUpdated(
-              headBlockPayloadAttributesType = payloadAttributeType,
-              headBlockHash = headExecutionPayloadHash,
-              safeBlockHash = newHead.get.safeExecutionPayloadHash,
-              finalizedBlockHash = newHead.get.finalizedExecutionPayloadHash,
-              receivedBlock = signedBlock)
+        template callExpectValidFCU(payloadAttributeType: untyped): auto =
+          await elManager.expectValidForkchoiceUpdated(
+            headBlockPayloadAttributesType = payloadAttributeType,
+            headBlockHash = headExecutionPayloadHash,
+            safeBlockHash = newHead.get.safeExecutionPayloadHash,
+            finalizedBlockHash = newHead.get.finalizedExecutionPayloadHash,
+            receivedBlock = signedBlock)
 
+        template callForkChoiceUpdated: auto =
           case self.consensusManager.dag.cfg.consensusForkAtEpoch(
               newHead.get.blck.bid.slot.epoch)
           of ConsensusFork.Capella, ConsensusFork.Deneb:
-            callForkchoiceUpdated(payloadAttributeType = PayloadAttributesV2)
+            callExpectValidFCU(payloadAttributeType = PayloadAttributesV2)
           of  ConsensusFork.Phase0, ConsensusFork.Altair,
               ConsensusFork.Bellatrix:
-            callForkchoiceUpdated(payloadAttributeType = PayloadAttributesV1)
+            callExpectValidFCU(payloadAttributeType = PayloadAttributesV1)
+
+        if self.consensusManager.checkNextProposer(wallSlot).isNone:
+          # No attached validator is next proposer, so use non-proposal fcU
+          callForkChoiceUpdated()
         else:
           # Some attached validator is next proposer, so prepare payload. As
           # updateHead() updated the DAG head, runProposalForkchoiceUpdated,
           # which needs the state corresponding to that head block, can run.
-          await self.consensusManager.runProposalForkchoiceUpdated(
-            wallSlot)
+          if (await self.consensusManager.runProposalForkchoiceUpdated(
+              wallSlot)).isNone:
+            callForkChoiceUpdated()
       else:
         await self.consensusManager.updateHeadWithExecution(
           newHead.get, self.getBeaconTime)