Beacon sync move fork choice logic to fc sub module (#3162)

* Move fc-update receptor call back function to `FC` sub-module

why
  The `FC` sub-module `chain_header_cache` keeps track of an header
  ancestry chain. As the `head` of the header chain is always set
  from the fork-choice update head, the call back function receiving
  the head is moved here. The client app using the`chain_header_cache`
  sub-module can be informed in an event driven way by a notification
  call back function (or fall back to polling.)

  This module also maintains the `finalized` hash value that is
  associated with the fork-choice update head. While the heads of
  subsequent fork-choice updates are ignored after a chain is
  initialised, the `finalized` hash values are kept in a cache for
  resolving headers over time while the `chain_header_cache` is
  active. Only the child-most finalised header is kept and available
  for further use by the client app.

* Rename `reqBeaconSyncerTarget()` to `fcHeaderClUpdate()`

why
  This naming does reflect the new owner of this hook. Currently, it
   is only used by the `FC` sub-module `chain_header_cache`.

* Move function calls `importBlocks()/forkChoice()` to `FC` sub-module

details
  This function pair is replaced by a single convenience wrapper
  function. This is a step into further integration into the `FC`
  module proper.

why
  This moves the details of when to call `forkChoice()` away from the
  syncer code. It also gets rid of handling the `finalized` variable
  within the syncer.

* Remove public exposure of `finalized` value from `FC` sub-module

why
  Is used internally only by the `fcHeaderImportBlock()` wrapper
  function.

* Remove crufty `syncReqNewHead()` hook

why
  Neither used not referenced since 29/09/2024 when the
  `beacon sync (#2666)` predecessor was removed

* Code cosmetics, docu update, logging update, etc.

Author: mjfh

execution_chain/beacon/api_handler/api_forkchoice.nim

@@ -108,11 +108,8 @@ proc forkchoiceUpdated*(ben: BeaconEngineRef,
       number = header.number,
       hash   = headHash.short
 
-    # Update sync header (if any)
-    com.syncReqNewHead(header)
-
-    # Ask the syncer to install missing blcks via RLPX
-    com.reqBeaconSyncerTarget(header, update.finalizedBlockHash)
+    # Inform the header cache (used by the syncer)
+    com.fcHeaderClUpdate(header, update.finalizedBlockHash)
 
     return simpleFCU(PayloadExecutionStatus.syncing)
 

execution_chain/beacon/beacon_engine.nim

@@ -259,9 +259,4 @@ proc delayPayloadImport*(ben: BeaconEngineRef, header: Header): PayloadStatusV1
   # at a later time.
   ben.put(blockHash, header)
 
-  # Although we don't want to trigger a sync, if there is one already in
-  # progress, try to extend it with the current payload request to relieve
-  # some strain from the forkchoice update.
-  ben.com.syncReqNewHead(header)
-
   PayloadStatusV1(status: PayloadExecutionStatus.syncing)

execution_chain/common/common.nim

@@ -36,11 +36,8 @@ type
     current: BlockNumber
     highest: BlockNumber
 
-  SyncReqNewHeadCB* = proc(header: Header) {.gcsafe, raises: [].}
-    ## Update head for syncing
-
-  ReqBeaconSyncerTargetCB* = proc(header: Header; finHash: Hash32) {.gcsafe, raises: [].}
-    ## Ditto (for beacon sync)
+  FcHeaderClUpdateCB* = proc(header: Header; finHash: Hash32) {.gcsafe, raises: [].}
+    ## Inform `CL` sub-module `chain_header_cache` about new head.
 
   BeaconSyncerProgressCB* = proc(): tuple[start, current, target: BlockNumber] {.gcsafe, raises: [].}
     ## Query syncer status
@@ -70,11 +67,7 @@ type
     # synchronizer need this
     syncProgress: SyncProgress
 
-    syncReqNewHead: SyncReqNewHeadCB
-      ## Call back function for the sync processor. This function stages
-      ## the arguent header to a private aerea for subsequent processing.
-
-    reqBeaconSyncerTargetCB: ReqBeaconSyncerTargetCB
+    fcHeaderClUpdateCB: FcHeaderClUpdateCB
       ## Call back function for a sync processor that returns the canonical
       ## header.
 
@@ -340,16 +333,10 @@ proc proofOfStake*(com: CommonRef, header: Header, txFrame: CoreDbTxRef): bool =
 func depositContractAddress*(com: CommonRef): Address =
   com.config.depositContractAddress.get(default(Address))
 
-proc syncReqNewHead*(com: CommonRef; header: Header)
-    {.gcsafe, raises: [].} =
+proc fcHeaderClUpdate*(com: CommonRef; header: Header; finHash: Hash32) =
   ## Used by RPC updater
-  if not com.syncReqNewHead.isNil:
-    com.syncReqNewHead(header)
-
-proc reqBeaconSyncerTarget*(com: CommonRef; header: Header; finHash: Hash32) =
-  ## Used by RPC updater
-  if not com.reqBeaconSyncerTargetCB.isNil:
-    com.reqBeaconSyncerTargetCB(header, finHash)
+  if not com.fcHeaderClUpdateCB.isNil:
+    com.fcHeaderClUpdateCB(header, finHash)
 
 proc beaconSyncerProgress*(com: CommonRef): tuple[start, current, target: BlockNumber] =
   ## Query syncer status
@@ -460,13 +447,9 @@ func setTTD*(com: CommonRef, ttd: Opt[DifficultyInt]) =
   # rebuild the MergeFork piece of the forkTransitionTable
   com.forkTransitionTable.mergeForkTransitionThreshold = com.config.mergeForkTransitionThreshold
 
-func `syncReqNewHead=`*(com: CommonRef; cb: SyncReqNewHeadCB) =
-  ## Activate or reset a call back handler for syncing.
-  com.syncReqNewHead = cb
-
-func `reqBeaconSyncerTarget=`*(com: CommonRef; cb: ReqBeaconSyncerTargetCB) =
+func `fcHeaderClUpdate=`*(com: CommonRef; cb: FcHeaderClUpdateCB) =
   ## Activate or reset a call back handler for syncing.
-  com.reqBeaconSyncerTargetCB = cb
+  com.fcHeaderClUpdateCB = cb
 
 func `beaconSyncerProgress=`*(com: CommonRef; cb: BeaconSyncerProgressCB) =
   ## Activate or reset a call back handler for querying syncer.

execution_chain/config.nim

@@ -339,13 +339,13 @@ type
       desc: "Number of worker threads (\"0\" = use as many threads as there are CPU cores available)"
       name: "num-threads" .}: int
 
-    beaconSyncScrumFile* {.
+    beaconSyncTargetFile* {.
       hidden
       desc: "Load a file containg an rlp-encoded object \"(Header,Hash32)\" " &
             "to be used " &
-            "as the first scrum target before any other request from the CL " &
-            " is accepted"
-      name: "debug-beacon-sync-scrum-file" .}: Option[InputFile]
+            "as the first target before any other request from the CL " &
+            "is accepted"
+      name: "debug-beacon-sync-target-file" .}: Option[InputFile]
 
     beaconSyncBlocksQueueHwm* {.
       hidden

execution_chain/core/chain.nim

@@ -9,9 +9,10 @@
 # according to those terms.
 
 import
-  ./chain/[persist_blocks, forked_chain]
+  ./chain/[persist_blocks, forked_chain, forked_chain/chain_header_cache]
 
 export
+  chain_header_cache,
   persist_blocks,
   forked_chain
 

execution_chain/core/chain/forked_chain.nim

@@ -19,7 +19,7 @@ import
   ../../evm/state,
   ../validate,
   ../executor/process_block,
-  ./forked_chain/[chain_desc, chain_header_cache, chain_branch]
+  ./forked_chain/[chain_desc, chain_branch]
 
 from std/sequtils import mapIt
 
@@ -29,7 +29,6 @@ logScope:
 export
   BlockDesc,
   ForkedChainRef,
-  chain_header_cache,
   common,
   core_db