Remove optimistic slot from FinalityUpdate content key (#1810)

fluffy/network/beacon_light_client/beacon_light_client_content.nim

@@ -51,12 +51,11 @@ type
   # this causes them also to be included in a request, which makes perhaps less
   # sense?
   LightClientFinalityUpdateKey* = object
-    optimisticSlot*: uint64 ## slot of attested header of the update
     finalizedSlot*: uint64 ## slot of finalized header of the update
 
   # TODO: Same remark as for `LightClientFinalityUpdateKey`
   LightClientOptimisticUpdateKey* = object
-    optimisticSlot*: uint64 ## slot of attested header of the update
+    optimisticSlot*: uint64 ## signature_slot of the update
 
   ContentKey* = object
     case contentType*: ContentType
@@ -159,7 +158,8 @@ func decodeForkedLightClientObject(
 
   withLcDataFork(lcDataForkAtConsensusFork(contextFork)):
     when lcDataFork > LightClientDataFork.None:
-      let res = decodeSsz(data.toOpenArray(4, len(data) - 1), ObjType.Forky(lcDataFork))
+      let res = decodeSsz(
+        data.toOpenArray(4, len(data) - 1), ObjType.Forky(lcDataFork))
       if res.isOk:
         # TODO:
         # How can we verify the Epoch vs fork, e.g. with `consensusForkAtEpoch`?
@@ -235,12 +235,10 @@ func updateContentKey*(startPeriod: uint64, count: uint64): ContentKey =
       startPeriod: startPeriod, count: count)
   )
 
-func finalityUpdateContentKey*(
-    finalizedSlot: uint64, optimisticSlot: uint64): ContentKey =
+func finalityUpdateContentKey*(finalizedSlot: uint64): ContentKey =
   ContentKey(
     contentType: lightClientFinalityUpdate,
     lightClientFinalityUpdateKey: LightClientFinalityUpdateKey(
-      optimisticSlot: optimisticSlot,
       finalizedSlot: finalizedSlot
     )
   )

fluffy/network/beacon_light_client/beacon_light_client_manager.nim

@@ -25,9 +25,6 @@ logScope:
 type
   Nothing = object
   ResponseError = object of CatchableError
-  SlotInfo = object
-    finalizedSlot: Slot
-    optimisticSlot: Slot
 
   NetRes*[T] = Result[T, void]
   Endpoint[K, V] =
@@ -39,7 +36,7 @@ type
       tuple[startPeriod: SyncCommitteePeriod, count: uint64],
       ForkedLightClientUpdate]
   FinalityUpdate =
-    Endpoint[SlotInfo, ForkedLightClientFinalityUpdate]
+    Endpoint[Slot, ForkedLightClientFinalityUpdate]
   OptimisticUpdate =
     Endpoint[Slot, ForkedLightClientOptimisticUpdate]
 
@@ -142,11 +139,10 @@ proc doRequest(
 proc doRequest(
     e: typedesc[FinalityUpdate],
     n: LightClientNetwork,
-    slotInfo: SlotInfo
+    finalizedSlot: Slot
 ): Future[NetRes[ForkedLightClientFinalityUpdate]] =
   n.getLightClientFinalityUpdate(
-    distinctBase(slotInfo.finalizedSlot),
-    distinctBase(slotInfo.optimisticSlot)
+    distinctBase(finalizedSlot)
   )
 
 # https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/altair/light-client/p2p-interface.md#getlightclientoptimisticupdate
@@ -331,17 +327,8 @@ proc loop(self: LightClientManager) {.async.} =
           await self.query(UpdatesByRange,
             (startPeriod: syncTask.startPeriod, count: syncTask.count))
         of LcSyncKind.FinalityUpdate:
-          let
-            # TODO: This is tricky. The optimistic slot kinda depends on when
-            # the request for the finality update is send?
-            # How to resolve? Don't use the optimistic slot in the content key
-            # to begin with, does it add anything?
-            optimisticSlot = wallTime.slotOrZero() - 1
-            finalizedSlot = start_slot(epoch(wallTime.slotOrZero()) - 2)
-          await self.query(FinalityUpdate, SlotInfo(
-            finalizedSlot: finalizedSlot,
-            optimisticSlot: optimisticSlot
-          ))
+          let finalizedSlot = start_slot(epoch(wallTime.slotOrZero()) - 2)
+          await self.query(FinalityUpdate, finalizedSlot)
         of LcSyncKind.OptimisticUpdate:
           let optimisticSlot = wallTime.slotOrZero() - 1
           await self.query(OptimisticUpdate, optimisticSlot)

fluffy/network/beacon_light_client/beacon_light_client_network.nim

@@ -8,7 +8,6 @@
 {.push raises: [].}
 
 import
-  std/[options, tables],
   stew/results, chronos, chronicles,
   eth/p2p/discoveryv5/[protocol, enr],
   beacon_chain/spec/forks,
@@ -38,38 +37,44 @@ type
 func toContentIdHandler(contentKey: ByteList): results.Opt[ContentId] =
   ok(toContentId(contentKey))
 
+proc getContent(
+    n: LightClientNetwork, contentKey: ContentKey):
+    Future[results.Opt[seq[byte]]] {.async.} =
+  let
+    contentKeyEncoded = encode(contentKey)
+    contentId = toContentId(contentKeyEncoded)
+    contentRes = await n.portalProtocol.contentLookup(
+      contentKeyEncoded, contentId)
+
+  if contentRes.isNone():
+    warn "Failed fetching content from the beacon chain network",
+      contentKey = contentKeyEncoded
+    return Opt.none(seq[byte])
+  else:
+    return Opt.some(contentRes.value().content)
+
 proc getLightClientBootstrap*(
     n: LightClientNetwork,
     trustedRoot: Digest):
     Future[results.Opt[ForkedLightClientBootstrap]] {.async.} =
   let
-    bk = LightClientBootstrapKey(blockHash: trustedRoot)
-    ck = ContentKey(
-      contentType: lightClientBootstrap,
-      lightClientBootstrapKey: bk
-    )
-    keyEncoded = encode(ck)
-    contentID = toContentId(keyEncoded)
+    contentKey = bootstrapContentKey(trustedRoot)
+    contentResult = await n.getContent(contentKey)
 
-  let bootstrapContentLookup =
-      await n.portalProtocol.contentLookup(keyEncoded, contentId)
-
-  if bootstrapContentLookup.isNone():
-      warn "Failed fetching LightClientBootstrap from the network",
-        trustedRoot, contentKey = keyEncoded
-      return Opt.none(ForkedLightClientBootstrap)
+  if contentResult.isNone():
+    return Opt.none(ForkedLightClientBootstrap)
 
   let
-    bootstrap = bootstrapContentLookup.unsafeGet()
+    bootstrap = contentResult.value()
     decodingResult = decodeLightClientBootstrapForked(
-      n.forkDigests, bootstrap.content)
+      n.forkDigests, bootstrap)
 
-  if decodingResult.isErr:
+  if decodingResult.isErr():
     return Opt.none(ForkedLightClientBootstrap)
   else:
     # TODO Not doing validation for now, as probably it should be done by layer
     # above
-    return Opt.some(decodingResult.get())
+    return Opt.some(decodingResult.value())
 
 proc getLightClientUpdatesByRange*(
     n: LightClientNetwork,
@@ -77,96 +82,66 @@ proc getLightClientUpdatesByRange*(
     count: uint64):
     Future[results.Opt[ForkedLightClientUpdateList]] {.async.} =
   let
-    bk = LightClientUpdateKey(
-      startPeriod: distinctBase(startPeriod), count: count)
-    ck = ContentKey(
-      contentType: lightClientUpdate,
-      lightClientUpdateKey: bk
-    )
-    keyEncoded = encode(ck)
-    contentID = toContentId(keyEncoded)
+    contentKey = updateContentKey(distinctBase(startPeriod), count)
+    contentResult = await n.getContent(contentKey)
 
-  let updatesResult =
-      await n.portalProtocol.contentLookup(keyEncoded, contentId)
-
-  if updatesResult.isNone():
-      warn "Failed fetching updates network", contentKey = keyEncoded
-      return Opt.none(ForkedLightClientUpdateList)
+  if contentResult.isNone():
+    return Opt.none(ForkedLightClientUpdateList)
 
   let
-    updates = updatesResult.unsafeGet()
+    updates = contentResult.value()
     decodingResult = decodeLightClientUpdatesByRange(
-      n.forkDigests, updates.content)
+      n.forkDigests, updates)
 
-  if decodingResult.isErr:
+  if decodingResult.isErr():
     return Opt.none(ForkedLightClientUpdateList)
   else:
     # TODO Not doing validation for now, as probably it should be done by layer
     # above
-    return Opt.some(decodingResult.get())
+    return Opt.some(decodingResult.value())
 
-proc getUpdate(
-    n: LightClientNetwork, ck: ContentKey):
-    Future[results.Opt[seq[byte]]] {.async.} =
-  let
-    keyEncoded = encode(ck)
-    contentID = toContentId(keyEncoded)
-    updateLookup = await n.portalProtocol.contentLookup(keyEncoded, contentId)
-
-  if updateLookup.isNone():
-    warn "Failed fetching update from the network", contentKey = keyEncoded
-    return Opt.none(seq[byte])
-
-  return ok(updateLookup.get().content)
-
-# TODO:
-# Currently both getLightClientFinalityUpdate and getLightClientOptimisticUpdate
-# are implemented in naive way as finding first peer with any of those updates
-# and treating it as latest. This will probably need to get improved.
 proc getLightClientFinalityUpdate*(
     n: LightClientNetwork,
-    currentFinalSlot: uint64,
-    currentOptimisticSlot: uint64
+    finalizedSlot: uint64
   ): Future[results.Opt[ForkedLightClientFinalityUpdate]] {.async.} =
-
   let
-    ck = finalityUpdateContentKey(currentFinalSlot, currentOptimisticSlot)
-    lookupResult = await n.getUpdate(ck)
+    contentKey = finalityUpdateContentKey(finalizedSlot)
+    contentResult = await n.getContent(contentKey)
 
-  if lookupResult.isErr:
+  if contentResult.isNone():
     return Opt.none(ForkedLightClientFinalityUpdate)
 
   let
-    finalityUpdate = lookupResult.get()
+    finalityUpdate = contentResult.value()
     decodingResult = decodeLightClientFinalityUpdateForked(
       n.forkDigests, finalityUpdate)
 
-  if decodingResult.isErr:
+  if decodingResult.isErr():
     return Opt.none(ForkedLightClientFinalityUpdate)
   else:
-    return Opt.some(decodingResult.get())
+    return Opt.some(decodingResult.value())
 
 proc getLightClientOptimisticUpdate*(
     n: LightClientNetwork,
-    currentOptimisticSlot: uint64
+    optimisticSlot: uint64
   ): Future[results.Opt[ForkedLightClientOptimisticUpdate]] {.async.} =
 
   let
-    ck = optimisticUpdateContentKey(currentOptimisticSlot)
-    lookupResult = await n.getUpdate(ck)
+    contentKey = optimisticUpdateContentKey(optimisticSlot)
+    contentResult = await n.getContent(contentKey)
 
-  if lookupResult.isErr:
+  if contentResult.isNone():
     return Opt.none(ForkedLightClientOptimisticUpdate)
 
   let
-    optimisticUpdate = lookupResult.get()
+    optimisticUpdate = contentResult.value()
     decodingResult = decodeLightClientOptimisticUpdateForked(
       n.forkDigests, optimisticUpdate)
 
-  if decodingResult.isErr:
+  if decodingResult.isErr():
     return Opt.none(ForkedLightClientOptimisticUpdate)
   else:
-    return Opt.some(decodingResult.get())
+    return Opt.some(decodingResult.value())
 
 proc new*(
     T: type LightClientNetwork,
@@ -306,7 +281,7 @@ proc processContentLoop(n: LightClientNetwork) {.async.} =
     trace "processContentLoop canceled"
 
 proc start*(n: LightClientNetwork) =
-  info "Starting portal light client network"
+  info "Starting portal beacon chain network"
   n.portalProtocol.start()
   n.processContentLoop = processContentLoop(n)
 

fluffy/tests/beacon_light_client_tests/test_beacon_light_client_content.nim

@@ -136,12 +136,8 @@ suite "Beacon Light Client Content Encodings - Mainnet":
       let key = contentKey.value()
       withForkyObject(update):
         when lcDataFork > LightClientDataFork.None:
-          let attestedSlot = forkyObject.attested_header.beacon.slot
-          let finalizedSlot = forkyObject.finalized_header.beacon.slot
-
-          check:
-            attestedSlot == key.lightClientFinalityUpdateKey.optimisticSlot
-            finalizedSlot == key.lightClientFinalityUpdateKey.finalizedSlot
+          check forkyObject.finalized_header.beacon.slot ==
+            key.lightClientFinalityUpdateKey.finalizedSlot
 
       # re-encode content and content key
       let encoded = encodeForkedLightClientObject(update, forkDigests.capella)
@@ -174,10 +170,8 @@ suite "Beacon Light Client Content Encodings - Mainnet":
       let key = contentKey.value()
       withForkyObject(update):
         when lcDataFork > LightClientDataFork.None:
-          let attestedSlot = forkyObject.attested_header.beacon.slot
-
-          check:
-            attestedSlot == key.lightClientOptimisticUpdateKey.optimisticSlot
+          check forkyObject.attested_header.beacon.slot ==
+            key.lightClientOptimisticUpdateKey.optimisticSlot
 
       # re-encode content and content key
       let encoded = encodeForkedLightClientObject(update, forkDigests.capella)

fluffy/tests/beacon_light_client_tests/test_beacon_light_client_network.nim

@@ -93,8 +93,7 @@ procSuite "Beacon Light Client Content Network":
       optimisticHeaderSlot = optimisticUpdateData.attested_header.beacon.slot
 
       finalityUpdateKey = finalityUpdateContentKey(
-        distinctBase(finalizedHeaderSlot),
-        distinctBase(finalizedOptimisticHeaderSlot)
+        distinctBase(finalizedHeaderSlot)
       )
       finalityKeyEnc = encode(finalityUpdateKey)
       finalityUpdateId = toContentId(finalityKeyEnc)
@@ -123,7 +122,6 @@ procSuite "Beacon Light Client Content Network":
       finalityResult =
         await lcNode1.lightClientNetwork.getLightClientFinalityUpdate(
           distinctBase(finalizedHeaderSlot),
-          distinctBase(finalizedOptimisticHeaderSlot)
         )
       optimisticResult =
         await lcNode1.lightClientNetwork.getLightClientOptimisticUpdate(

fluffy/tools/beacon_chain_bridge/beacon_chain_bridge.nim

@@ -182,9 +182,7 @@ proc gossipLCFinalityUpdate*(
     when lcDataFork > LightClientDataFork.None:
       let
         finalizedSlot = forkyObject.finalized_header.beacon.slot
-        optimisticSlot = forkyObject.attested_header.beacon.slot
-        contentKey = encode(finalityUpdateContentKey(
-          finalizedSlot.uint64, optimisticSlot.uint64))
+        contentKey = encode(finalityUpdateContentKey(finalizedSlot.uint64))
         forkDigest = forkDigestAtEpoch(
           forkDigests[], epoch(forkyObject.attested_header.beacon.slot), cfg)
         content = encodeFinalityUpdateForked(
@@ -200,7 +198,7 @@ proc gossipLCFinalityUpdate*(
                 contentKeyHex,
                 content.toHex())
           info "Beacon LC finality update gossiped", peers,
-            contentKey = contentKeyHex, finalizedSlot, optimisticSlot
+            contentKey = contentKeyHex, finalizedSlot
           return ok()
         except CatchableError as e:
           return err("JSON-RPC error: " & $e.msg)

fluffy/tools/beacon_lc_bridge/beacon_lc_bridge.nim

@@ -650,9 +650,7 @@ proc run(config: BeaconBridgeConf) {.raises: [CatchableError].} =
           update, slot = forkyObject.attested_header.beacon.slot
         let
           finalizedSlot = forkyObject.finalized_header.beacon.slot
-          optimisticSlot = forkyObject.attested_header.beacon.slot
-          contentKey = encode(finalityUpdateContentKey(
-            finalizedSlot.uint64, optimisticSlot.uint64))
+          contentKey = encode(finalityUpdateContentKey(finalizedSlot.uint64))
           contentId = beacon_light_client_content.toContentId(contentKey)
           forkDigest = forkDigestAtEpoch(
             forkDigests[], epoch(forkyObject.attested_header.beacon.slot), cfg)

fluffy/tools/eth_data_exporter/cl_data_exporter.nim

@@ -208,9 +208,7 @@ proc exportLCFinalityUpdate*(
     when lcDataFork > LightClientDataFork.None:
       let
         finalizedSlot = forkyObject.finalized_header.beacon.slot
-        optimisticSlot = forkyObject.attested_header.beacon.slot
-        contentKey = encode(finalityUpdateContentKey(
-          finalizedSlot.uint64, optimisticSlot.uint64))
+        contentKey = encode(finalityUpdateContentKey(finalizedSlot.uint64))
         contentId = beacon_light_client_content.toContentId(contentKey)
         forkDigest = forkDigestAtEpoch(
           forkDigests[], epoch(forkyObject.attested_header.beacon.slot), cfg)
@@ -225,7 +223,7 @@ proc exportLCFinalityUpdate*(
       )
 
       var contentTable: JsonPortalContentTable
-      contentTable[$optimisticSlot] = portalContent
+      contentTable[$finalizedSlot] = portalContent
 
       writePortalContentToJson(fh, contentTable)
 

vendor/portal-spec-tests

@@ -1 +1 @@
-Subproject commit 529764e1df46f99899127b75097d5162e9ed7ed0
+Subproject commit 26edde52b942020ef38aba3795400d713072cf21