extract helper for validating `UpdatesByRange` consistency (#5053)

Reduce code duplication when checking response of `UpdatesByRange`.

beacon_chain/sync/light_client_manager.nim

@@ -7,13 +7,13 @@
 
 {.push raises: [].}
 
-import chronos, chronicles, stew/base10
+import chronos, chronicles
 import
   eth/p2p/discoveryv5/random2,
   ../spec/network,
   ../networking/eth2_network,
   ../beacon_clock,
-  "."/sync_protocol, "."/sync_manager
+  "."/[light_client_sync_helpers, sync_protocol, sync_manager]
 export sync_manager
 
 logScope:
@@ -137,38 +137,10 @@ proc doRequest(
     reqCount = min(periods.len, MAX_REQUEST_LIGHT_CLIENT_UPDATES).uint64
   let response = await peer.lightClientUpdatesByRange(startPeriod, reqCount)
   if response.isOk:
-    if response.get.lenu64 > reqCount:
+    let e = distinctBase(response.get)
-      raise newException(ResponseError, "Too many values in response" &
+      .checkLightClientUpdates(startPeriod, reqCount)
-        " (" & Base10.toString(response.get.lenu64) &
+    if e.isErr:
-        " > " & Base10.toString(reqCount.uint) & ")")
+      raise newException(ResponseError, e.error)
-    var expectedPeriod = startPeriod
-    for update in response.get:
-      withForkyUpdate(update):
-        when lcDataFork > LightClientDataFork.None:
-          let
-            attPeriod =
-              forkyUpdate.attested_header.beacon.slot.sync_committee_period
-            sigPeriod = forkyUpdate.signature_slot.sync_committee_period
-          if attPeriod != sigPeriod:
-            raise newException(
-              ResponseError, "Conflicting sync committee periods" &
-              " (signature: " & Base10.toString(distinctBase(sigPeriod)) &
-              " != " & Base10.toString(distinctBase(attPeriod)) & ")")
-          if attPeriod < expectedPeriod:
-            raise newException(
-              ResponseError, "Unexpected sync committee period" &
-              " (" & Base10.toString(distinctBase(attPeriod)) &
-              " < " & Base10.toString(distinctBase(expectedPeriod)) & ")")
-          if attPeriod > expectedPeriod:
-            if attPeriod > lastPeriod:
-              raise newException(
-                ResponseError, "Sync committee period too high" &
-                " (" & Base10.toString(distinctBase(attPeriod)) &
-                " > " & Base10.toString(distinctBase(lastPeriod)) & ")")
-            expectedPeriod = attPeriod
-          inc expectedPeriod
-        else:
-          raise newException(ResponseError, "Invalid context bytes")
   return response
 
 # https://github.com/ethereum/consensus-specs/blob/v1.4.0-alpha.2/specs/altair/light-client/p2p-interface.md#getlightclientfinalityupdate

beacon_chain/sync/light_client_sync_helpers.nim

@@ -0,0 +1,50 @@
+# beacon_chain
+# Copyright (c) 2022-2023 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
+{.push raises: [].}
+
+import
+  std/typetraits,
+  chronos,
+  stew/base10,
+  ../spec/[forks_light_client, network]
+
+func checkLightClientUpdates*(
+    updates: openArray[ForkedLightClientUpdate],
+    startPeriod: SyncCommitteePeriod,
+    count: uint64): Result[void, string] =
+  if updates.lenu64 > count:
+    return err("Too many values in response" &
+      " (" & Base10.toString(updates.lenu64) &
+      " > " & Base10.toString(count.uint) & ")")
+  let lastPeriod = startPeriod + count - 1
+  var expectedPeriod = startPeriod
+  for update in updates:
+    withForkyUpdate(update):
+      when lcDataFork > LightClientDataFork.None:
+        let
+          attPeriod =
+            forkyUpdate.attested_header.beacon.slot.sync_committee_period
+          sigPeriod = forkyUpdate.signature_slot.sync_committee_period
+        if attPeriod != sigPeriod:
+          return err("Conflicting sync committee periods" &
+            " (signature: " & Base10.toString(distinctBase(sigPeriod)) &
+            " != " & Base10.toString(distinctBase(attPeriod)) & ")")
+        if attPeriod < expectedPeriod:
+          return err("Unexpected sync committee period" &
+            " (" & Base10.toString(distinctBase(attPeriod)) &
+            " < " & Base10.toString(distinctBase(expectedPeriod)) & ")")
+        if attPeriod > expectedPeriod:
+          if attPeriod > lastPeriod:
+            return err("Sync committee period too high" &
+              " (" & Base10.toString(distinctBase(attPeriod)) &
+              " > " & Base10.toString(distinctBase(lastPeriod)) & ")")
+          expectedPeriod = attPeriod
+        inc expectedPeriod
+      else:
+        return err("Invalid context bytes")
+  ok()

beacon_chain/trusted_node_sync.nim

@@ -9,7 +9,7 @@
 import
   stew/[base10, results],
   chronicles, chronos, eth/async_utils,
-  ./sync/sync_manager,
+  ./sync/[light_client_sync_helpers, sync_manager],
   ./consensus_object_pools/[block_clearance, blockchain_dag],
   ./spec/eth2_apis/rest_beacon_client,
   ./spec/[beaconstate, eth2_merkleization, forks, light_client_sync,
@@ -267,34 +267,16 @@ proc doTrustedNodeSync*(
             except CatchableError as exc:
               error "Unable to download LC updates", error = exc.msg
               quit 1
-          if updates.lenu64 > count:
+          let e = updates.checkLightClientUpdates(startPeriod, count)
-            error "Malformed LC updates response: Too many values"
+          if e.isErr:
+            error "Malformed LC updates response", resError = e.error
             quit 1
           if updates.len == 0:
             warn "Server does not appear to be fully synced"
             break
-          var expectedPeriod = startPeriod
           for i in 0 ..< updates.len:
             doAssert updates[i].kind > LightClientDataFork.None
             updates[i].migrateToDataFork(lcDataFork)
-            let
-              attPeriod = updates[i].forky(lcDataFork)
-                .attested_header.beacon.slot.sync_committee_period
-              sigPeriod = updates[i].forky(lcDataFork)
-                .signature_slot.sync_committee_period
-            if attPeriod != sigPeriod:
-              error "Malformed LC updates response: Conflicting periods"
-              quit 1
-            if attPeriod < expectedPeriod:
-              error "Malformed LC updates response: Unexpected period"
-              quit 1
-            if attPeriod > expectedPeriod:
-              if attPeriod > lastPeriod:
-                error "Malformed LC updates response: Period too high"
-                quit 1
-              expectedPeriod = attPeriod
-            inc expectedPeriod
-
             let res = process_light_client_update(
               store, updates[i].forky(lcDataFork),
               getBeaconTime().slotOrZero(), cfg, genesis_validators_root)