From 94ba0a9bd138d961230f877970612ed4142333db Mon Sep 17 00:00:00 2001
From: Etan Kissling <etan@status.im>
Date: Wed, 7 Feb 2024 19:03:19 +0100
Subject: [PATCH] consider block availability when initializing LC data
 collector (#5860)

When using checkpoint sync, the initial block is missing in the DB.
Update the LC data collector initialization to account for that,
avoiding a spurious error message when it is incorrectly accessed:

```
ERR 2024-02-07 11:21:55.416+01:00 Block failed to load unexpectedly          topics="chaindag_lc" bid=d30517a7:8257504 tail=8257504
```

Also fixes a regression from #5691 that resulted in similar messages
while importing the first few blocks after checkpoint sync.

Thanks to @arnetheduck for reporting this.
---
 .../blockchain_dag_light_client.nim           | 40 +++++++++++++++++--
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim b/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim
index bf823aa8c..5aaad018a 100644
--- a/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim
+++ b/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim
@@ -631,7 +631,11 @@ proc createLightClientUpdate(
   # Check if light client data improved
   let
     finalized_slot = attested_data.finalized_slot
-    finalized_bsi = dag.getExistingBlockIdAtSlot(finalized_slot)
+    finalized_bsi =
+      if finalized_slot >= dag.tail.slot:
+        dag.getExistingBlockIdAtSlot(finalized_slot)
+      else:
+        Opt.none(BlockSlotId)
     has_finality =
       finalized_bsi.isSome and
       finalized_bsi.get.bid.slot >= dag.tail.slot
@@ -702,8 +706,36 @@ proc initLightClientDataCache*(dag: ChainDAGRef) =
   if not dag.shouldImportLcData:
     return
 
-  # Initialize tail slot
-  let targetTailSlot = max(dag.targetLightClientTailSlot, dag.tail.slot)
+  # Initialize tail slot.
+  # Both state and blocks must be available to construct light client data,
+  # see `cacheLightClientData`. If blocks are unavailable, most parts of the
+  # `LightClientHeader` could be reconstructed from the corresponding post-state
+  # but `execution_branch` (since Capella) requires full block availability.
+  # If the assumed-to-be-available range of states / blocks turns out to be
+  # actually unavailable (database inconsistencies), the `tailSlot` will adjust
+  # using `handleUnexpectedLightClientError`. This is unexpected and is logged,
+  # but recoverable by excluding the unavailable range from LC data collection.
+  let targetTailSlot = max(
+    # User configured horizon
+    dag.targetLightClientTailSlot,
+    max(
+      # State availability, needed for `cacheLightClientData`
+      dag.tail.slot,
+      # Block availability, needed for `LightClientHeader.execution_branch`
+      if dag.backfill.slot != GENESIS_SLOT:
+        let existing = dag.getBlockIdAtSlot(dag.backfill.slot)
+        if existing.isSome:
+          if dag.getForkedBlock(existing.get.bid).isNone:
+            # Special case: when starting with only a checkpoint state,
+            # we will not have the head block data in the database
+            dag.backfill.slot + 1
+          else:
+            dag.backfill.slot
+        else:
+          dag.backfill.slot
+      else:
+        dag.backfill.slot))
+
   dag.lcDataStore.cache.tailSlot = max(dag.head.slot, targetTailSlot)
   if dag.head.slot < dag.lcDataStore.cache.tailSlot:
     return
@@ -725,7 +757,7 @@ proc initLightClientDataCache*(dag: ChainDAGRef) =
   logScope:
     lightClientDataMaxPeriods = dag.lcDataStore.maxPeriods
     importMode = dag.lcDataStore.importMode
-  debug "Initializing cached LC data", res
+  debug "Initializing cached LC data", res, targetTailSlot
 
   proc isSyncAggregateCanonical(
       dag: ChainDAGRef, state: ForkyHashedBeaconState,