Beacon sync update multi exe heads aware (#2861)

* Log/trace cancellation events in scheduler

* Provide `clear()` functions for explicitly flushing data objects

* Renaming header cache functions

why:
  More systematic, all functions start with prefix `dbHeader`

* Remove `danglingParent` from layout

why:
  Already provided by header cache

* Remove `couplerHash` and `headHash` from layout

why:
  No need to cache, `headHash` is unused and `couplerHash` used typically
  once, only.

* Remove `lastLayout` from sync descriptor

why:
  No need to compare changes, saving is always triggered after actively
  changing the sync layout state

* Early reject unsuitable head + finalised header from CL

why:
  The finalised header is only passed by its hash so the header must be
  fetched somewhere, e.g. from a peer via eth/xx.

  Also, finalised headers earlier than the `base` from `FC` cannot be
  handled due to the `Aristo` single state database architecture.

  Luckily, on a full node, the complete block history is available so
  unsuitable finalised headers are stored there already which is exploited
  here to avoid unnecessary network traffic.

* Code cosmetics, remove cruft, prettify logging, remove `final` metrics

detail:
  The `final` layout parameter will be deprecated and later removed

* Update/re-calibrate syncer logic documentation

why:
  The current implementation sucks if the `FC` module changes the
  canonical branch in the middle of completing a header chain (due
  to concurrent updates by the `newPayload()` logic.)

* Implement according to re-calibrated syncer docu

details:
  The implementation employs the notion of named layout states (see
  `SyncLayoutState` in `worker_desc.nim`) which are derived from the
  state parameter triple `(C,D,H)` as described in `README.md`.

Author: mjfh

nimbus/sync/beacon/README.md

@@ -66,58 +66,67 @@ Implementation, The Gory Details
 The following diagram depicts a most general state view of the sync and the
 *FC* modules and at a given point of time
 
-        0            C       L                                               (5)
-        o------------o-------o
+        0                    L                                               (5)
+        o--------------------o
         | <--- imported ---> |
-                     Y                     D                H
+                     C                     D                H
                      o---------------------o----------------o
                      | <-- unprocessed --> | <-- linked --> |
 
 where
 
-* *C* -- coupler, cached **base** entity of the **FC** module, reported at
-         the time when *H* was set. This determines the maximal way back length
-         of the *linked* ancestor chain starting at *H*.
-
-* *Y* -- has the same block number as *C* and is often, but not necessarily
-         equal to *C* (for notation *C~Y* see clause *(6)* below.)
+* *C* -- coupler, parent of the left endpoint of the chain of headers or blocks
+         to be fetched and imported.
 
-* *L* -- **latest**, current value of this entity of the **FC** module (i.e.
-         now, when looked up)
+* *L* -- **latest**, current value of this entity (with the same name) of the
+         **FC** module (i.e. the current value when looked up.) *L* need not
+         be a parent of any header of the linked chain `(C,H]` (see below for
+         notation). Both *L* and *H* might be heads of different forked chains.
 
-* *D* -- dangling, least block number of the linked chain in progress ending
-         at *H*. This variable is used to record the download state eventually
-         reaching *Y* (for notation *D<<H* see clause *(6)* below.)
+* *D* -- dangling, header with the least block number of the linked chain in
+         progress ending at *H*. This variable is used to record the download
+         state eventually reaching *Y* (for notation *D<<H* see clause *(6)*
+         below.)
 
-* *H* -- head, sync target which typically is the value of a *sync to new head*
-         request (via RPC)
+* *H* -- head, sync target header which typically was the value of a *sync to
+         new head* request (via RPC)
 
-The internal sync state (as opposed to the general state also including **FC**)
-is defined by the triple *(C,D,H)*. Other parameters *L* and *Y* mentioned in
-*(5)* are considered ephemeral to the sync state. They are always used by its
-latest value and are not cached by the syncer.
+The internal sync state (as opposed to the general state also including the
+state of **FC**) is defined by the triple *(C,D,H)*. Other parameters like *L*
+mentioned in *(5)* are considered ephemeral to the sync state. They are always
+seen by its latest values and not cached by the syncer.
 
 There are two order releations and some derivatives used to describe relations
-beween headers or blocks.
+between headers or blocks.
 
         For blocks or headers A and B, A is said less or equal B if the      (6)
         block numbers are less or equal. Notation: A <= B.
 
-        For blocks or headers A and B, A is said ancestor of, or equal to
-        B if B is linked to A following up the lineage of parentHash fields
-        of the block headers. Notation: A << B.
+        The notation A ~ B stands for A <= B <= A which makes <= an order
+        relation (relative to ~ rather than ==). If A ~ B does not hold
+        then the notation A !~ B is used.
+
+        The notation A < B stands for A <= B and A !~ B.
+
+        The notation B-1 stands for any block or header with block number of
+        B less one.
+
 
-        The relate notation A ~ B stands for A <= B <= A which is posh for
-        saying that A and B have the same block numer.
+        For blocks or headers A and B, writing A <- B stands for the block
+        A be parent of B (there can only be one parent of B.)
+
+        For blocks or headers A and B, A is said ancestor of, or equal to B
+        if A == B or there is a non-empty parent lineage A <- X <- Y <-..<- B.
+        Notation: A << B (note that << is an equivalence relation.)
 
         The compact interval notation [A,B] stands for the set {X|A<<X<<B}
         and the half open interval notation stands for [A,B]-{A} (i.e. the
         interval without the left end point.)
-     
+
 Note that *A<<B* implies *A<=B*. Boundary conditions that hold for the
 clause *(5)* diagram are
 
-        C ~ Y,  C in [0,L],  D in [Y,H]                                      (7)
+        there is a Z in [0,L] with C ~ Z, D is in [C,H]                      (7)
 
 
 ### Sync Processing
@@ -134,64 +143,70 @@ parameters *C* and *D* are irrelevant here.
 Following, there will be a request to advance *H* to a new position as
 indicated in the diagram below
 
-        0            C                                                       (9)
+        0            B                                                       (9)
         o------------o-------o
         | <--- imported ---> |                              D
-                     Y                                      H
+                     C                                      H
                      o--------------------------------------o
                      | <----------- unprocessed ----------> |
 
-with a new sync state *(C,D,H)*. The parameter *C* in clause *(9)* is set
-as the **base** entity of the **FC** module. *Y* is only known by its block
-number, *Y~C*. The parameter *D* is set to the download start position *H*.
+with a new sync state *(C,H,H)*. The parameter *B* is the **base** entity
+of the **FC** module. The parameter *C* is a placeholder with *C ~ B*. The
+parameter *D* is set to the download start position *H*.
 
-The syncer then fetches the header chain *(Y,H]* from the network. For the
-syncer state *(C,D,H)*, while iteratively fetching headers, only the parameter
-*D* will change each time a new header was fetched.
+The syncer then fetches the header chain *(C,H]* from the network. While
+iteratively fetching headers, the syncer state *(C,D,H)* will only change on
+its second position *D* time after a new header was fetched.
 
-Having finished dowlnoading *(Y,H]* one might end up with a situation
+Having finished downloading then *C~D-1*. The sync state is *(D-1,D,H)*. One
+will end up with a situation like
 
-        0             B  Z   L                                              (10)
-        o-------------o--o---o
+        0               Y    L                                              (10)
+        o---------------o----o
         | <--- imported ---> |
-                     Y   Z                                  H
-                     o---o----------------------------------o
+                     C    Z                                 H
+                     o----o---------------------------------o
                      | <-------------- linked ------------> |
 
-where *Z* is in the intersection of *[B,L]\*(Y,H]* with *B* the current
-**base** entity of the **FC** logic. It is only known that *0<<B<<L*
-although in many cases *B==C* holds.
+for some *Y* in *[0,L]* and *Z* in *(C,H]* where *Y<<Z* with *L* the **latest**
+entity of the **FC** logic.
 
-If there is no such *Z* then *(Y,H]* is discarded and sync processing restarts
-at clause *(8)* by resetting the sync state (e.g. to *(0,0,0)*.)
+If there are no such *Y* and *Z*, then *(C,H]* is discarded and sync processing
+restarts at clause *(8)* by resetting the sync state (e.g. to *(0,0,0)*.)
 
-Otherwise assume *Z* is the one with the largest block number of the
-intersection *[B,L]\*(Y,H]*. Then the headers *(Z,H]* will be completed to
-a lineage of blocks by downloading block bodies.
+Otherwise choose *Y* and *Z* with maximal block number of *Y* so that *Y<-Z*.
+Then complete *(Y,H]==[Z,H]* to a lineage of blocks by downloading missing
+block bodies.
 
-        0                Z                                                  (11)
-        o----------------o---o
+Having finished with block bodies, the sync state will be expressed as
+*(Y,Y,H)*. With the choice of the first two entries equal it is indicated that
+the lineage *(Y,H]* is fully populated with blocks.
+
+        0               Y                                                   (11)
+        o---------------o----o
         | <--- imported ---> |
-                         Z                                  H
-                         o----------------------------------o
-                         | <------------ blocks ----------> |
+                        Y                                   H
+                        o-----------------------------------o
+                        | <------------ blocks -----------> |
 
-The blocks *(Z,H]* will then be imported. While this happens, the internal
-state of the **FC** might change/reset so that further import becomes
-impossible. Even when starting import, the block *Z* might not be in *[0,L]*
+The blocks *(Y,H]* will then be imported and executed. While this happens, the
+internal state of the **FC** might change/reset so that further import becomes
+impossible. Even when starting import, the block *Y* might not be in *[0,L]*
 anymore due to some internal reset of the **FC** logic. In any of those
 cases, sync processing restarts at clause *(8)* by resetting the sync state.
 
-Otherwise the block import will end up at
+In case all blocks can be imported, one will will end up at
 
-        0                Z                                  H   L           (12)
-        o----------------o----------------------------------o---o
+        0                 Y                                 H   L           (12)
+        o-----------------o---------------------------------o---o
         | <--- imported --------------------------------------> |
 
 with *H<<L* for *L* the current value of the **latest** entity of the **FC**
-module. In many cases, *H==L* but there are other actors running that might
-import blocks quickly after importing *H* so that *H* is seen as ancestor,
-different from *L* when this stage is formally done with.
+module.
+
+In many cases, *H==L* but there are other actors which also might import blocks
+quickly after finishing import of *H* before formally committing this task. So
+*H* can become ancestor of *L*.
 
 Now clause *(12)* is equivalent to clause *(8)*.
 
@@ -295,7 +310,6 @@ be available if *nimbus* is compiled with the additional make flags
 | beacon_latest      | block height | **L**, *increasing* |
 | beacon_coupler     | block height | **C**, *increasing* |
 | beacon_dangling    | block height | **D**               |
-| beacon_final       | block height | **F**, *increasing* |
 | beacon_head        | block height | **H**, *increasing* |
 | beacon_target      | block height | **T**, *increasing* |
 |                            |      |                     |

nimbus/sync/beacon/TODO.md

@@ -1,9 +1,3 @@
-## Update sync state management to what is described in *README.md*
-
-1. For the moment, the events in *update.nim* need to be adjusted. This will fix an error where the CL forces the EL to fork internally by sending different head request headers with the same bock number.
-
-2. General scenario update. This is mostly error handling.
-
 ## General TODO items
 
 * Update/resolve code fragments which are tagged FIXME
@@ -25,3 +19,17 @@ which happened on several `holesky` tests immediately after loging somehing like
 or from another machine with literally the same exception text (but the stack-trace differs)
 
         NTC 2024-10-31 21:58:07.616 Finalized blocks persisted   file=forked_chain.nim:231 numberOfBlocks=129 last=9cbcc52953a8 baseNumber=2646857 baseHash=9db5c2ac537b
+
+### 3. Mem overflow possible on small breasted systems
+
+Running the exe client, a 1.5G response message was opbserved (on my 8G test system this kills the program as it has already 80% mem load. It happens while syncing holesky at around block #184160 and is reproducible on the 8G system but not yet on the an 80G system.)
+
+		[..]
+		DBG 2024-11-20 16:16:18.871+00:00 Processing JSON-RPC request  file=router.nim:135 id=178 name=eth_getLogs
+		DBG 2024-11-20 16:16:18.915+00:00 Returning JSON-RPC response  file=router.nim:137 id=178 name=eth_getLogs len=201631
+		TRC 2024-11-20 16:16:18.951+00:00 <<< find_node from           topics="eth p2p discovery" file=discovery.nim:248 node=Node[94.16.123.192:30303]
+		TRC 2024-11-20 16:16:18.951+00:00 Neighbours to                topics="eth p2p discovery" file=discovery.nim:161 node=Node[94.16.123.192:30303] nodes=[..]
+		TRC 2024-11-20 16:16:18.951+00:00 Neighbours to                topics="eth p2p discovery" file=discovery.nim:161 node=Node[94.16.123.192:30303] nodes=[..]
+		DBG 2024-11-20 16:16:19.027+00:00 Received JSON-RPC request    topics="JSONRPC-HTTP-SERVER" file=httpserver.nim:52 address=127.0.0.1:49746 len=239
+		DBG 2024-11-20 16:16:19.027+00:00 Processing JSON-RPC request  file=router.nim:135 id=179 name=eth_getLogs
+		DBG 2024-11-20 16:20:23.664+00:00 Returning JSON-RPC response  file=router.nim:137 id=179 name=eth_getLogs len=1630240149

nimbus/sync/beacon/worker.nim

@@ -54,7 +54,7 @@ proc napUnlessSomethingToFetch(
 
 proc setup*(ctx: BeaconCtxRef; info: static[string]): bool =
   ## Global set up
-  ctx.setupRpcMagic()
+  ctx.setupRpcMagic info
 
   # Load initial state from database if there is any
   ctx.setupDatabase info
@@ -109,12 +109,10 @@ proc runDaemon*(
   ## first usable request from the CL (via RPC) stumbles in.
   ##
   # Check for a possible header layout and body request changes
-  ctx.updateSyncStateLayout info
+  ctx.updateSyncState info
   if ctx.hibernate:
     return
 
-  ctx.updateBlockRequests info
-
   # Execute staged block records.
   if ctx.blocksStagedCanImportOk():
 

nimbus/sync/beacon/worker/blocks_staged.nim

@@ -57,13 +57,13 @@ proc fetchAndCheck(
   blk.blocks.setLen(offset + ivReq.len)
   var blockHash = newSeq[Hash32](ivReq.len)
   for n in 1u ..< ivReq.len:
-    let header = ctx.dbPeekHeader(ivReq.minPt + n).valueOr:
+    let header = ctx.dbHeaderPeek(ivReq.minPt + n).valueOr:
       # There is nothing one can do here
       raiseAssert info & " stashed header missing: n=" & $n &
         " ivReq=" & $ivReq & " nth=" & (ivReq.minPt + n).bnStr
     blockHash[n - 1] = header.parentHash
     blk.blocks[offset + n].header = header
-  blk.blocks[offset].header = ctx.dbPeekHeader(ivReq.minPt).valueOr:
+  blk.blocks[offset].header = ctx.dbHeaderPeek(ivReq.minPt).valueOr:
     # There is nothing one can do here
     raiseAssert info & " stashed header missing: n=0" &
       " ivReq=" & $ivReq & " nth=" & ivReq.minPt.bnStr
@@ -325,7 +325,7 @@ proc blocksStagedImport*(
 
   # Remove stashed headers for imported blocks
   for bn in iv.minPt .. maxImport:
-    ctx.dbUnstashHeader bn
+    ctx.dbHeaderUnstash bn
 
   # Update, so it can be followed nicely
   ctx.updateMetrics()

nimbus/sync/beacon/worker/blocks_staged/staged_queue.nim

@@ -31,6 +31,10 @@ func blocksStagedQueueIsEmpty*(ctx: BeaconCtxRef): bool =
 
 # ----------------
 
+func blocksStagedQueueClear*(ctx: BeaconCtxRef) =
+  ## Clear queue
+  ctx.blk.staged.clear
+
 func blocksStagedQueueInit*(ctx: BeaconCtxRef) =
   ## Constructor
   ctx.blk.staged = StagedBlocksQueue.init()

nimbus/sync/beacon/worker/blocks_unproc.nim

@@ -83,11 +83,6 @@ proc blocksUnprocCovered*(ctx: BeaconCtxRef; pt: BlockNumber): bool =
   ctx.blk.unprocessed.covered(pt, pt) == 1
 
 
-proc blocksUnprocTop*(ctx: BeaconCtxRef): BlockNumber =
-  let iv = ctx.blk.unprocessed.le().valueOr:
-    return BlockNumber(0)
-  iv.maxPt
-
 proc blocksUnprocBottom*(ctx: BeaconCtxRef): BlockNumber =
   let iv = ctx.blk.unprocessed.ge().valueOr:
     return high(BlockNumber)
@@ -112,14 +107,14 @@ proc blocksUnprocInit*(ctx: BeaconCtxRef) =
   ## Constructor
   ctx.blk.unprocessed = BnRangeSet.init()
 
-proc blocksUnprocSet*(ctx: BeaconCtxRef) =
+proc blocksUnprocClear*(ctx: BeaconCtxRef) =
   ## Clear
   ctx.blk.unprocessed.clear()
   ctx.blk.borrowed = 0u
 
 proc blocksUnprocSet*(ctx: BeaconCtxRef; minPt, maxPt: BlockNumber) =
   ## Set up new unprocessed range
-  ctx.blocksUnprocSet()
+  ctx.blocksUnprocClear()
   # Argument `maxPt` would be internally adjusted to `max(minPt,maxPt)`
   if minPt <= maxPt:
     discard ctx.blk.unprocessed.merge(minPt, maxPt)