From d8144c6de10604fa37b70e1f445c954331c4a7bd Mon Sep 17 00:00:00 2001
From: Etan Kissling <etan@status.im>
Date: Fri, 17 Nov 2023 17:17:26 -0800
Subject: [PATCH] cache new LC data after creating new LC updates (#5607)

When creating new LC updates, information about the parent block's post
state must be available (cached), but information about current block's
post state is not yet required. Caching information about the current
block's post state can be delayed, simplifying the LC data collection
logic a bit and allowing more future flexibility with the cache design.
---
 .../blockchain_dag_light_client.nim           | 34 ++++++++++---------
 1 file changed, 18 insertions(+), 16 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 f60db7a99..828140899 100644
--- a/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim
+++ b/beacon_chain/consensus_object_pools/blockchain_dag_light_client.nim
@@ -676,23 +676,26 @@ proc initLightClientDataCache*(dag: ChainDAGRef) =
   let targetTailSlot = max(dag.targetLightClientTailSlot, dag.tail.slot)
   dag.lcDataStore.cache.tailSlot = max(dag.head.slot, targetTailSlot)
 
-  # Import head state
+  # In `OnlyNew` mode, only head state needs to be cached
   if dag.head.slot < dag.lcDataStore.cache.tailSlot:
     return
-  withState(dag.headState):
-    when consensusFork >= ConsensusFork.Altair:
-      dag.cacheLightClientData(forkyState, dag.head.bid)
-    else: raiseAssert "Unreachable" # `tailSlot` cannot be before Altair
   if dag.lcDataStore.importMode == LightClientDataImportMode.OnlyNew:
+    withState(dag.headState):
+      when consensusFork >= ConsensusFork.Altair:
+        dag.cacheLightClientData(forkyState, dag.head.bid)
+      else: raiseAssert "Unreachable" # `tailSlot` cannot be before Altair
     return
 
   # Import light client data for finalized period through finalized head
-  let finalizedSlot = max(dag.finalizedHead.blck.slot, targetTailSlot)
-  if finalizedSlot >= dag.lcDataStore.cache.tailSlot:
-    return
-  dag.lcDataStore.cache.tailSlot = finalizedSlot
-  let finalizedPeriod = finalizedSlot.sync_committee_period
-  var res = dag.initLightClientDataForPeriod(finalizedPeriod)
+  let
+    finalizedSlot = max(dag.finalizedHead.blck.slot, targetTailSlot)
+    finalizedPeriod = finalizedSlot.sync_committee_period
+  var res =
+    if finalizedSlot < dag.lcDataStore.cache.tailSlot:
+      dag.lcDataStore.cache.tailSlot = finalizedSlot
+      dag.initLightClientDataForPeriod(finalizedPeriod)
+    else:
+      Opt[void].ok()
 
   let lightClientStartTick = Moment.now()
   logScope: lightClientDataMaxPeriods = dag.lcDataStore.maxPeriods
@@ -728,14 +731,13 @@ proc initLightClientDataCache*(dag: ChainDAGRef) =
       continue
     withStateAndBlck(dag.headState, bdata):
       when consensusFork >= ConsensusFork.Altair:
-        # Cache light client data (non-finalized blocks may refer to this)
-        if i != blocks.low:
-          dag.cacheLightClientData(forkyState, bid)  # `dag.head` already cached
-
         # Create `LightClientUpdate` instances
         if i < blocks.high:
           dag.createLightClientUpdates(
             forkyState, forkyBlck, parentBid = blocks[i + 1])
+
+        # Cache light client data (non-finalized blocks may refer to this)
+        dag.cacheLightClientData(forkyState, bid)
       else: raiseAssert "Unreachable"
 
   let lightClientEndTick = Moment.now()
@@ -775,8 +777,8 @@ proc processNewBlockForLightClient*(
   const consensusFork = typeof(signedBlock).kind
   when consensusFork >= ConsensusFork.Altair:
     template forkyState: untyped = state.forky(consensusFork)
-    dag.cacheLightClientData(forkyState, signedBlock.toBlockId())
     dag.createLightClientUpdates(forkyState, signedBlock, parentBid)
+    dag.cacheLightClientData(forkyState, signedBlock.toBlockId())
   else:
     raiseAssert "Unreachable"  # `tailSlot` cannot be before Altair