From a3229a6b9ba0cddcd5aedcf1d5c2b850760323c4 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Mon, 12 Jun 2023 14:22:32 +0200 Subject: [PATCH] extract helper for validating `UpdatesByRange` consistency (#5053) Reduce code duplication when checking response of `UpdatesByRange`. --- beacon_chain/sync/light_client_manager.nim | 40 +++------------ .../sync/light_client_sync_helpers.nim | 50 +++++++++++++++++++ beacon_chain/trusted_node_sync.nim | 26 ++-------- 3 files changed, 60 insertions(+), 56 deletions(-) create mode 100644 beacon_chain/sync/light_client_sync_helpers.nim diff --git a/beacon_chain/sync/light_client_manager.nim b/beacon_chain/sync/light_client_manager.nim index 8663be550..3de779880 100644 --- a/beacon_chain/sync/light_client_manager.nim +++ b/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: - raise newException(ResponseError, "Too many values in response" & - " (" & Base10.toString(response.get.lenu64) & - " > " & Base10.toString(reqCount.uint) & ")") - 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") + let e = distinctBase(response.get) + .checkLightClientUpdates(startPeriod, reqCount) + if e.isErr: + raise newException(ResponseError, e.error) return response # https://github.com/ethereum/consensus-specs/blob/v1.4.0-alpha.2/specs/altair/light-client/p2p-interface.md#getlightclientfinalityupdate diff --git a/beacon_chain/sync/light_client_sync_helpers.nim b/beacon_chain/sync/light_client_sync_helpers.nim new file mode 100644 index 000000000..aa3d59e01 --- /dev/null +++ b/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() diff --git a/beacon_chain/trusted_node_sync.nim b/beacon_chain/trusted_node_sync.nim index 0d5e15e89..56e0f06ed 100644 --- a/beacon_chain/trusted_node_sync.nim +++ b/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: - error "Malformed LC updates response: Too many values" + let e = updates.checkLightClientUpdates(startPeriod, count) + 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)