status-im · etan-status · Aug 25, 2022 · Aug 20, 2022 · Aug 20, 2022 · Aug 25, 2022
diff --git a/beacon_chain/consensus_object_pools/attestation_pool.nim b/beacon_chain/consensus_object_pools/attestation_pool.nim
@@ -733,26 +733,55 @@ func getAggregatedAttestation*(pool: var AttestationPool,
 
   res
 
+type BeaconHead* = object
+  blck*: BlockRef
+  safeExecutionPayloadHash*, finalizedExecutionPayloadHash*: Eth2Digest
+
+proc getBeaconHead*(
+    pool: var AttestationPool, headBlock: BlockRef): BeaconHead =
+  let
+    finalizedExecutionPayloadHash =
+      pool.dag.loadExecutionBlockRoot(pool.dag.finalizedHead.blck)
+
+    # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.3/fork_choice/safe-block.md#get_safe_execution_payload_hash
+    safeBlockRoot = pool.forkChoice.get_safe_beacon_block_root()
+    safeBlock = pool.dag.getBlockRef(safeBlockRoot)
+    safeExecutionPayloadHash =
+      if safeBlock.isErr:
+        # Safe block is currently the justified block determined by fork choice.
+        # If finality already advanced beyond the current justified checkpoint,
+        # e.g., because we have selected a head that did not yet realize the cp,
+        # the justified block may end up not having a `BlockRef` anymore.
+        # Because we know that a different fork already finalized a later point,
+        # let's just report the finalized execution payload hash instead.
+        finalizedExecutionPayloadHash
+      else:
+        pool.dag.loadExecutionBlockRoot(safeBlock.get)
+
+  BeaconHead(
+    blck: headBlock,
+    safeExecutionPayloadHash: safeExecutionPayloadHash,
+    finalizedExecutionPayloadHash: finalizedExecutionPayloadHash)
+
 proc selectOptimisticHead*(
-    pool: var AttestationPool, wallTime: BeaconTime): Opt[BlockRef] =
+    pool: var AttestationPool, wallTime: BeaconTime): Opt[BeaconHead] =
   ## Trigger fork choice and returns the new head block.
-  ## Can return `nil`
   # TODO rename this to get_optimistic_head
-  let newHead = pool.forkChoice.get_head(pool.dag, wallTime)
-
-  if newHead.isErr:
-    error "Couldn't select head", err = newHead.error
-    err()
-  else:
-    let ret = pool.dag.getBlockRef(newHead.get())
-    if ret.isErr():
-      # This should normally not happen, but if the chain dag and fork choice
-      # get out of sync, we'll need to try to download the selected head - in
-      # the meantime, return nil to indicate that no new head was chosen
-      warn "Fork choice selected unknown head, trying to sync", root = newHead.get()
-      pool.quarantine[].addMissing(newHead.get())
-
-    ret
+  let newHeadRoot = pool.forkChoice.get_head(pool.dag, wallTime)
+  if newHeadRoot.isErr:
+    error "Couldn't select head", err = newHeadRoot.error
+    return err()
+
+  let headBlock = pool.dag.getBlockRef(newHeadRoot.get()).valueOr:
+    # This should normally not happen, but if the chain dag and fork choice
+    # get out of sync, we'll need to try to download the selected head - in
+    # the meantime, return nil to indicate that no new head was chosen
+    warn "Fork choice selected unknown head, trying to sync",
+      root = newHeadRoot.get()
+    pool.quarantine[].addMissing(newHeadRoot.get())
+    return err()
+
+  ok pool.getBeaconHead(headBlock)
 
 proc prune*(pool: var AttestationPool) =
   if (let v = pool.forkChoice.prune(); v.isErr):

diff --git a/beacon_chain/consensus_object_pools/consensus_manager.nim b/beacon_chain/consensus_object_pools/consensus_manager.nim
@@ -118,7 +118,8 @@ from web3/engine_api_types import
 func `$`(h: BlockHash): string = $h.asEth2Digest
 
 proc runForkchoiceUpdated*(
-    eth1Monitor: Eth1Monitor, headBlockRoot, finalizedBlockRoot: Eth2Digest):
+    eth1Monitor: Eth1Monitor,
+    headBlockRoot, safeBlockRoot, finalizedBlockRoot: Eth2Digest):
     Future[PayloadExecutionStatus] {.async.} =
   # Allow finalizedBlockRoot to be 0 to avoid sync deadlocks.
   #
@@ -138,15 +139,15 @@ proc runForkchoiceUpdated*(
 
     let fcuR = awaitWithTimeout(
       forkchoiceUpdated(
-        eth1Monitor, headBlockRoot, finalizedBlockRoot),
+        eth1Monitor, headBlockRoot, safeBlockRoot, finalizedBlockRoot),
       FORKCHOICEUPDATED_TIMEOUT):
         debug "runForkchoiceUpdated: forkchoiceUpdated timed out"
         ForkchoiceUpdatedResponse(
-          payloadStatus: PayloadStatusV1(status: PayloadExecutionStatus.syncing))
+          payloadStatus: PayloadStatusV1(
+            status: PayloadExecutionStatus.syncing))
 
     debug "runForkchoiceUpdated: ran forkchoiceUpdated",
-      headBlockRoot,
-      finalizedBlockRoot,
+      headBlockRoot, safeBlockRoot, finalizedBlockRoot,
       payloadStatus = $fcuR.payloadStatus.status,
       latestValidHash = $fcuR.payloadStatus.latestValidHash,
       validationError = $fcuR.payloadStatus.validationError
@@ -157,31 +158,32 @@ proc runForkchoiceUpdated*(
       err = err.msg
     return PayloadExecutionStatus.syncing
 
-proc updateExecutionClientHead(self: ref ConsensusManager, newHead: BlockRef)
+proc updateExecutionClientHead(self: ref ConsensusManager, newHead: BeaconHead)
     {.async.} =
   if self.eth1Monitor.isNil:
     return
 
-  let executionHeadRoot = self.dag.loadExecutionBlockRoot(newHead)
+  let headExecutionPayloadHash = self.dag.loadExecutionBlockRoot(newHead.blck)
 
-  if executionHeadRoot.isZero:
+  if headExecutionPayloadHash.isZero:
     # Blocks without execution payloads can't be optimistic.
-    self.dag.markBlockVerified(self.quarantine[], newHead.root)
+    self.dag.markBlockVerified(self.quarantine[], newHead.blck.root)
     return
 
   # Can't use dag.head here because it hasn't been updated yet
   let payloadExecutionStatus = await self.eth1Monitor.runForkchoiceUpdated(
-    executionHeadRoot,
-    self.dag.loadExecutionBlockRoot(self.dag.finalizedHead.blck))
+    headExecutionPayloadHash,
+    newHead.safeExecutionPayloadHash,
+    newHead.finalizedExecutionPayloadHash)
 
   case payloadExecutionStatus
   of PayloadExecutionStatus.valid:
-    self.dag.markBlockVerified(self.quarantine[], newHead.root)
+    self.dag.markBlockVerified(self.quarantine[], newHead.blck.root)
   of PayloadExecutionStatus.invalid, PayloadExecutionStatus.invalid_block_hash:
-    self.dag.markBlockInvalid(newHead.root)
-    self.quarantine[].addUnviable(newHead.root)
+    self.dag.markBlockInvalid(newHead.blck.root)
+    self.quarantine[].addUnviable(newHead.blck.root)
   of PayloadExecutionStatus.accepted, PayloadExecutionStatus.syncing:
-    self.dag.optimisticRoots.incl newHead.root
+    self.dag.optimisticRoots.incl newHead.blck.root
 
 proc updateHead*(self: var ConsensusManager, newHead: BlockRef) =
   ## Trigger fork choice and update the DAG with the new head block
@@ -206,11 +208,11 @@ proc updateHead*(self: var ConsensusManager, wallSlot: Slot) =
       head = shortLog(self.dag.head), wallSlot
     return
 
-  if self.dag.loadExecutionBlockRoot(newHead).isZero:
+  if self.dag.loadExecutionBlockRoot(newHead.blck).isZero:
     # Blocks without execution payloads can't be optimistic.
-    self.dag.markBlockVerified(self.quarantine[], newHead.root)
+    self.dag.markBlockVerified(self.quarantine[], newHead.blck.root)
 
-  self.updateHead(newHead)
+  self.updateHead(newHead.blck)
 
 proc checkNextProposer(dag: ChainDAGRef, slot: Slot):
     Opt[(ValidatorIndex, ValidatorPubKey)] =
@@ -247,18 +249,20 @@ proc runProposalForkchoiceUpdated*(self: ref ConsensusManager) {.async.} =
         get_randao_mix(state.data, get_current_epoch(state.data)).data
       feeRecipient = self.getFeeRecipient(
         nextProposer, validatorIndex, nextSlot.epoch)
-      headBlockRoot = self.dag.loadExecutionBlockRoot(self.dag.head)
-      finalizedBlockRoot =
-        self.dag.loadExecutionBlockRoot(self.dag.finalizedHead.blck)
+      beaconHead = self.attestationPool[].getBeaconHead(self.dag.head)
+      headBlockRoot = self.dag.loadExecutionBlockRoot(beaconHead.blck)
 
     if headBlockRoot.isZero:
       return
 
     try:
       let fcResult = awaitWithTimeout(
         forkchoiceUpdated(
-          self.eth1Monitor, headBlockRoot, finalizedBlockRoot, timestamp,
-          randomData, feeRecipient),
+          self.eth1Monitor,
+          headBlockRoot,
+          beaconHead.safeExecutionPayloadHash,
+          beaconHead.finalizedExecutionPayloadHash,
+          timestamp, randomData, feeRecipient),
         FORKCHOICEUPDATED_TIMEOUT):
           debug "runProposalForkchoiceUpdated: forkchoiceUpdated timed out"
           ForkchoiceUpdatedResponse(
@@ -271,13 +275,14 @@ proc runProposalForkchoiceUpdated*(self: ref ConsensusManager) {.async.} =
       self.forkchoiceUpdatedInfo = Opt.some ForkchoiceUpdatedInformation(
         payloadId: bellatrix.PayloadID(fcResult.payloadId.get),
         headBlockRoot: headBlockRoot,
-        finalizedBlockRoot: finalizedBlockRoot,
+        safeBlockRoot: beaconHead.safeExecutionPayloadHash,
+        finalizedBlockRoot: beaconHead.finalizedExecutionPayloadHash,
         timestamp: timestamp,
         feeRecipient: feeRecipient)
     except CatchableError as err:
       error "Engine API fork-choice update failed", err = err.msg
 
-proc updateHeadWithExecution*(self: ref ConsensusManager, newHead: BlockRef)
+proc updateHeadWithExecution*(self: ref ConsensusManager, newHead: BeaconHead)
     {.async.} =
   ## Trigger fork choice and update the DAG with the new head block
   ## This does not automatically prune the DAG after finalization
@@ -290,7 +295,7 @@ proc updateHeadWithExecution*(self: ref ConsensusManager, newHead: BlockRef)
 
     # Store the new head in the chain DAG - this may cause epochs to be
     # justified and finalized
-    self.dag.updateHead(newHead, self.quarantine[])
+    self.dag.updateHead(newHead.blck, self.quarantine[])
 
     # TODO after things stabilize with this, check for upcoming proposal and
     # don't bother sending first fcU, but initially, keep both in place

diff --git a/beacon_chain/eth1/eth1_monitor.nim b/beacon_chain/eth1/eth1_monitor.nim
@@ -496,7 +496,7 @@ proc newPayload*(p: Eth1Monitor, payload: engine_api.ExecutionPayloadV1):
   p.dataProvider.web3.provider.engine_newPayloadV1(payload)
 
 proc forkchoiceUpdated*(p: Eth1Monitor,
-                        headBlock, finalizedBlock: Eth2Digest):
+                        headBlock, safeBlock, finalizedBlock: Eth2Digest):
                         Future[engine_api.ForkchoiceUpdatedResponse] =
   # Eth1 monitor can recycle connections without (external) warning; at least,
   # don't crash.
@@ -510,17 +510,12 @@ proc forkchoiceUpdated*(p: Eth1Monitor,
   p.dataProvider.web3.provider.engine_forkchoiceUpdatedV1(
     ForkchoiceStateV1(
       headBlockHash: headBlock.asBlockHash,
-
-      # https://hackmd.io/@n0ble/kintsugi-spec#Engine-API
-      # "CL client software MUST use headBlockHash value as a stub for the
-      # safeBlockHash parameter"
-      safeBlockHash: headBlock.asBlockHash,
-
+      safeBlockHash: safeBlock.asBlockHash,
       finalizedBlockHash: finalizedBlock.asBlockHash),
     none(engine_api.PayloadAttributesV1))
 
 proc forkchoiceUpdated*(p: Eth1Monitor,
-                        headBlock, finalizedBlock: Eth2Digest,
+                        headBlock, safeBlock, finalizedBlock: Eth2Digest,
                         timestamp: uint64,
                         randomData: array[32, byte],
                         suggestedFeeRecipient: Eth1Address):
@@ -537,12 +532,7 @@ proc forkchoiceUpdated*(p: Eth1Monitor,
   p.dataProvider.web3.provider.engine_forkchoiceUpdatedV1(
     ForkchoiceStateV1(
       headBlockHash: headBlock.asBlockHash,
-
-      # https://hackmd.io/@n0ble/kintsugi-spec#Engine-API
-      # "CL client software MUST use headBlockHash value as a stub for the
-      # safeBlockHash parameter"
-      safeBlockHash: headBlock.asBlockHash,
-
+      safeBlockHash: safeBlock.asBlockHash,
       finalizedBlockHash: finalizedBlock.asBlockHash),
     some(engine_api.PayloadAttributesV1(
       timestamp: Quantity timestamp,

diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim
@@ -413,6 +413,11 @@ proc get_head*(self: var ForkChoice,
     self.checkpoints.justified.balances,
     self.checkpoints.proposer_boost_root)
 
+# https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.3/fork_choice/safe-block.md#get_safe_beacon_block_root
+func get_safe_beacon_block_root*(self: var ForkChoice): Eth2Digest =
+  # Use most recent justified block as a stopgap
+  self.checkpoints.justified.checkpoint.root
+
 func prune*(
        self: var ForkChoiceBackend, finalized_root: Eth2Digest
      ): FcResult[void] =

diff --git a/beacon_chain/gossip_processing/block_processor.nim b/beacon_chain/gossip_processing/block_processor.nim
@@ -173,20 +173,20 @@ from ../eth1/eth1_monitor import
   Eth1Monitor, asEngineExecutionPayload, ensureDataProvider, newPayload
 
 proc expectValidForkchoiceUpdated(
-    eth1Monitor: Eth1Monitor, headBlockRoot, finalizedBlockRoot: Eth2Digest):
-    Future[void] {.async.} =
+    eth1Monitor: Eth1Monitor,
+    headBlockRoot, safeBlockRoot, finalizedBlockRoot: Eth2Digest
+): Future[void] {.async.} =
   let payloadExecutionStatus =
-    await eth1Monitor.runForkchoiceUpdated(headBlockRoot, finalizedBlockRoot)
+    await eth1Monitor.runForkchoiceUpdated(
+      headBlockRoot, safeBlockRoot, finalizedBlockRoot)
   if payloadExecutionStatus != PayloadExecutionStatus.valid:
     # Only called when expecting this to be valid because `newPayload` or some
     # previous `forkchoiceUpdated` had already marked it as valid.
     warn "expectValidForkchoiceUpdate: forkChoiceUpdated not `VALID`",
-      payloadExecutionStatus,
-      headBlockRoot,
-      finalizedBlockRoot
+      payloadExecutionStatus, headBlockRoot, safeBlockRoot, finalizedBlockRoot
 
 from ../consensus_object_pools/attestation_pool import
-  addForkChoice, selectOptimisticHead
+  addForkChoice, selectOptimisticHead, BeaconHead
 from ../consensus_object_pools/blockchain_dag import
   is_optimistic, loadExecutionBlockRoot, markBlockVerified
 from ../consensus_object_pools/block_dag import shortLog
@@ -294,20 +294,20 @@ proc storeBlock*(
         wallSlot.start_beacon_time)
 
   if newHead.isOk:
-    let executionHeadRoot =
-      self.consensusManager.dag.loadExecutionBlockRoot(newHead.get)
-    if executionHeadRoot.isZero:
+    let headExecutionPayloadHash =
+      self.consensusManager.dag.loadExecutionBlockRoot(newHead.get.blck)
+    if headExecutionPayloadHash.isZero:
       # Blocks without execution payloads can't be optimistic.
-      self.consensusManager[].updateHead(newHead.get)
-    elif not self.consensusManager.dag.is_optimistic newHead.get.root:
+      self.consensusManager[].updateHead(newHead.get.blck)
+    elif not self.consensusManager.dag.is_optimistic newHead.get.blck.root:
       # Not `NOT_VALID`; either `VALID` or `INVALIDATED`, but latter wouldn't
       # be selected as head, so `VALID`. `forkchoiceUpdated` necessary for EL
       # client only.
-      self.consensusManager[].updateHead(newHead.get)
+      self.consensusManager[].updateHead(newHead.get.blck)
       asyncSpawn self.consensusManager.eth1Monitor.expectValidForkchoiceUpdated(
-        executionHeadRoot,
-        self.consensusManager.dag.loadExecutionBlockRoot(
-          self.consensusManager.dag.finalizedHead.blck))
+        headExecutionPayloadHash,
+        newHead.get.safeExecutionPayloadHash,
+        newHead.get.finalizedExecutionPayloadHash)
 
       # TODO remove redundant fcU in case of proposal
       asyncSpawn self.consensusManager.runProposalForkchoiceUpdated()

diff --git a/beacon_chain/nimbus_light_client.nim b/beacon_chain/nimbus_light_client.nim
@@ -90,6 +90,7 @@ programMain:
               # engine_forkchoiceUpdatedV1
               discard await eth1Monitor.runForkchoiceUpdated(
                 headBlockRoot = payload.block_hash,
+                safeBlockRoot = payload.block_hash,  # stub value
                 finalizedBlockRoot = ZERO_HASH)
         else: discard
       return

diff --git a/beacon_chain/validators/validator_duties.nim b/beacon_chain/validators/validator_duties.nim
@@ -313,6 +313,7 @@ from web3/engine_api import ForkchoiceUpdatedResponse
 # TODO: This copies the entire BeaconState on each call
 proc forkchoice_updated(state: bellatrix.BeaconState,
                         head_block_hash: Eth2Digest,
+                        safe_block_hash: Eth2Digest,
                         finalized_block_hash: Eth2Digest,
                         fee_recipient: ethtypes.Address,
                         execution_engine: Eth1Monitor):
@@ -328,8 +329,8 @@ proc forkchoice_updated(state: bellatrix.BeaconState,
       try:
         awaitWithTimeout(
           execution_engine.forkchoiceUpdated(
-            head_block_hash, finalized_block_hash, timestamp, random.data,
-            fee_recipient),
+            head_block_hash, safe_block_hash, finalized_block_hash,
+            timestamp, random.data, fee_recipient),
           FORKCHOICEUPDATED_TIMEOUT):
             error "Engine API fork-choice update timed out"
             default(ForkchoiceUpdatedResponse)
@@ -398,14 +399,15 @@ proc getExecutionPayload(
           node.eth1Monitor.terminalBlockHash.get.asEth2Digest
         else:
           default(Eth2Digest)
-      executionBlockRoot = node.dag.loadExecutionBlockRoot(node.dag.head)
+      beaconHead = node.attestationPool[].getBeaconHead(node.dag.head)
+      executionBlockRoot = node.dag.loadExecutionBlockRoot(beaconHead.blck)
       latestHead =
         if not executionBlockRoot.isZero:
           executionBlockRoot
         else:
           terminalBlockHash
-      latestFinalized =
-        node.dag.loadExecutionBlockRoot(node.dag.finalizedHead.blck)
+      latestSafe = beaconHead.safeExecutionPayloadHash
+      latestFinalized = beaconHead.finalizedExecutionPayloadHash
       feeRecipient = node.getFeeRecipient(pubkey, validator_index, epoch)
       lastFcU = node.consensusManager.forkchoiceUpdatedInfo
       timestamp = compute_timestamp_at_slot(
@@ -414,14 +416,14 @@ proc getExecutionPayload(
       payload_id =
         if  lastFcU.isSome and
             lastFcU.get.headBlockRoot == latestHead and
+            lastFcU.get.safeBlockRoot == latestSafe and
             lastFcU.get.finalizedBlockRoot == latestFinalized and
             lastFcU.get.timestamp == timestamp and
             lastFcU.get.feeRecipient == feeRecipient:
           some bellatrix.PayloadID(lastFcU.get.payloadId)
         else:
           debug "getExecutionPayload: didn't find payloadId, re-querying",
-            latestHead,
-            latestFinalized,
+            latestHead, latestSafe, latestFinalized,
             timestamp,
             feeRecipient,
             cachedHeadBlockRoot = lastFcU.get.headBlockRoot,
@@ -430,7 +432,8 @@ proc getExecutionPayload(
             cachedFeeRecipient = lastFcU.get.feeRecipient
 
           (await forkchoice_updated(
-           proposalState.bellatrixData.data, latestHead, latestFinalized,
+           proposalState.bellatrixData.data,
+           latestHead, latestSafe, latestFinalized,
            feeRecipient, node.consensusManager.eth1Monitor))
       payload = try:
           awaitWithTimeout(