From cdac3ae12b8f1861e9560df77251e2013f583216 Mon Sep 17 00:00:00 2001 From: dadepo Date: Wed, 7 Sep 2022 22:48:09 +0200 Subject: [PATCH] Updating light client proof logic to root next sync committee in attested header (#4478) * Update assertValidSyncCommitteeProof to align with updated spec * fix validation test * updated more tests * merged in unstable * move nextSyncCommittee to syncAttestedState * having the attester block root as sync committee. Should fix e2e test * some variable renaming * proper renaming of variables etc --- .../src/chain/lightClient/index.ts | 4 +- packages/light-client/src/index.ts | 37 ++++++------- packages/light-client/src/validation.ts | 23 ++------ .../test/unit/syncInMemory.test.ts | 4 +- .../light-client/test/unit/validation.test.ts | 47 ++++++++-------- .../test/utils/prepareUpdateNaive.ts | 25 ++------- packages/light-client/test/utils/utils.ts | 55 ++++++++----------- 7 files changed, 76 insertions(+), 119 deletions(-) diff --git a/packages/beacon-node/src/chain/lightClient/index.ts b/packages/beacon-node/src/chain/lightClient/index.ts index 1ac8f6065949..6dd9ede0e7e0 100644 --- a/packages/beacon-node/src/chain/lightClient/index.ts +++ b/packages/beacon-node/src/chain/lightClient/index.ts @@ -282,9 +282,7 @@ export class LightClientServer { throw Error(`No partialUpdate available for period ${period}`); } - const syncCommitteeWitnessBlockRoot = partialUpdate.isFinalized - ? (partialUpdate.finalizedCheckpoint.root as Uint8Array) - : partialUpdate.blockRoot; + const syncCommitteeWitnessBlockRoot = partialUpdate.blockRoot; const syncCommitteeWitness = await this.db.syncCommitteeWitness.get(syncCommitteeWitnessBlockRoot); if (!syncCommitteeWitness) { diff --git a/packages/light-client/src/index.ts b/packages/light-client/src/index.ts index f78d639920b5..02ee1afef210 100644 --- a/packages/light-client/src/index.ts +++ b/packages/light-client/src/index.ts @@ -15,12 +15,7 @@ import {isValidMerkleBranch} from "./utils/verifyMerkleBranch.js"; import {SyncCommitteeFast} from "./types.js"; import {chunkifyInclusiveRange} from "./utils/chunkify.js"; import {LightclientEmitter, LightclientEvent} from "./events.js"; -import { - assertValidSignedHeader, - assertValidLightClientUpdate, - activeHeader, - assertValidFinalityProof, -} from "./validation.js"; +import {assertValidSignedHeader, assertValidLightClientUpdate, assertValidFinalityProof} from "./validation.js"; import {getLcLoggerConsole, ILcLogger} from "./utils/logger.js"; import {computeSyncPeriodAtEpoch, computeSyncPeriodAtSlot, computeEpochAtSlot} from "./utils/clock.js"; @@ -398,24 +393,24 @@ export class Lightclient { * This headerUpdate may update the head if there's enough participation. */ private processOptimisticUpdate(headerUpdate: routes.events.LightclientOptimisticHeaderUpdate): void { - const {attestedHeader: header, syncAggregate} = headerUpdate; + const {attestedHeader, syncAggregate} = headerUpdate; // Prevent registering updates for slots to far ahead - if (header.slot > slotWithFutureTolerance(this.config, this.genesisTime, MAX_CLOCK_DISPARITY_SEC)) { - throw Error(`header.slot ${header.slot} is too far in the future, currentSlot: ${this.currentSlot}`); + if (attestedHeader.slot > slotWithFutureTolerance(this.config, this.genesisTime, MAX_CLOCK_DISPARITY_SEC)) { + throw Error(`header.slot ${attestedHeader.slot} is too far in the future, currentSlot: ${this.currentSlot}`); } - const period = computeSyncPeriodAtSlot(header.slot); + const period = computeSyncPeriodAtSlot(attestedHeader.slot); const syncCommittee = this.syncCommitteeByPeriod.get(period); if (!syncCommittee) { // TODO: Attempt to fetch committee update for period if it's before the current clock period throw Error(`No syncCommittee for period ${period}`); } - const headerBlockRoot = ssz.phase0.BeaconBlockHeader.hashTreeRoot(header); + const headerBlockRoot = ssz.phase0.BeaconBlockHeader.hashTreeRoot(attestedHeader); const headerBlockRootHex = toHexString(headerBlockRoot); - assertValidSignedHeader(this.config, syncCommittee, syncAggregate, headerBlockRoot, header.slot); + assertValidSignedHeader(this.config, syncCommittee, syncAggregate, headerBlockRoot, attestedHeader.slot); // Valid header, check if has enough bits. // Only accept headers that have at least half of the max participation seen in this period @@ -441,32 +436,32 @@ export class Lightclient { // Maybe update the head if ( // Advance head - header.slot > this.head.header.slot || + attestedHeader.slot > this.head.header.slot || // Replace same slot head - (header.slot === this.head.header.slot && participation > this.head.participation) + (attestedHeader.slot === this.head.header.slot && participation > this.head.participation) ) { // TODO: Do metrics for each case (advance vs replace same slot) const prevHead = this.head; - this.head = {header, participation, blockRoot: headerBlockRootHex}; + this.head = {header: attestedHeader, participation, blockRoot: headerBlockRootHex}; // This is not an error, but a problematic network condition worth knowing about - if (header.slot === prevHead.header.slot && prevHead.blockRoot !== headerBlockRootHex) { + if (attestedHeader.slot === prevHead.header.slot && prevHead.blockRoot !== headerBlockRootHex) { this.logger.warn("Head update on same slot", { prevHeadSlot: prevHead.header.slot, prevHeadRoot: prevHead.blockRoot, }); } this.logger.info("Head updated", { - slot: header.slot, + slot: attestedHeader.slot, root: headerBlockRootHex, }); // Emit to consumers - this.emitter.emit(LightclientEvent.head, header); + this.emitter.emit(LightclientEvent.head, attestedHeader); } else { this.logger.debug("Received valid head update did not update head", { currentHead: `${this.head.header.slot} ${this.head.blockRoot}`, - eventHead: `${header.slot} ${headerBlockRootHex}`, + eventHead: `${attestedHeader.slot} ${headerBlockRootHex}`, }); } } @@ -531,12 +526,12 @@ export class Lightclient { * period 0 period 1 period 2 * -|----------------|----------------|----------------|-> time * | now - * - active current_sync_committee: period 0 + * - current_sync_committee: period 0 * - known next_sync_committee, signed by current_sync_committee */ private processSyncCommitteeUpdate(update: altair.LightClientUpdate): void { // Prevent registering updates for slots too far in the future - const updateSlot = activeHeader(update).slot; + const updateSlot = update.attestedHeader.slot; if (updateSlot > slotWithFutureTolerance(this.config, this.genesisTime, MAX_CLOCK_DISPARITY_SEC)) { throw Error(`updateSlot ${updateSlot} is too far in the future, currentSlot ${this.currentSlot}`); } diff --git a/packages/light-client/src/validation.ts b/packages/light-client/src/validation.ts index 92a5b3222525..a3b39ac7a1ae 100644 --- a/packages/light-client/src/validation.ts +++ b/packages/light-client/src/validation.ts @@ -1,4 +1,4 @@ -import {altair, phase0, Root, Slot, ssz} from "@lodestar/types"; +import {altair, Root, Slot, ssz} from "@lodestar/types"; import bls from "@chainsafe/bls/switchable"; import type {PublicKey, Signature} from "@chainsafe/bls/types"; import { @@ -18,8 +18,9 @@ import {computeSyncPeriodAtSlot} from "./utils/clock.js"; /** * - * @param syncCommittee SyncPeriod that signed this update: `computeSyncPeriodAtSlot(update.header.slot) - 1` - * @param forkVersion ForkVersion that was used to sign the update + * @param config the beacon node config + * @param syncCommittee the sync committee update + * @param update the light client update for validation */ export function assertValidLightClientUpdate( config: IBeaconConfig, @@ -101,27 +102,13 @@ export function assertValidSyncCommitteeProof(update: altair.LightClientUpdate): update.nextSyncCommitteeBranch, NEXT_SYNC_COMMITTEE_DEPTH, NEXT_SYNC_COMMITTEE_INDEX, - activeHeader(update).stateRoot + update.attestedHeader.stateRoot ) ) { throw Error("Invalid next sync committee merkle branch"); } } -/** - * The "active header" is the header that the update is trying to convince us - * to accept. If a finalized header is present, it's the finalized header, - * otherwise it's the attested header - * @param update - */ -export function activeHeader(update: altair.LightClientUpdate): phase0.BeaconBlockHeader { - if (!isEmptyHeader(update.finalizedHeader)) { - return update.finalizedHeader; - } - - return update.attestedHeader; -} - /** * Assert valid signature for `signedHeader` with provided `syncCommittee`. * diff --git a/packages/light-client/test/unit/syncInMemory.test.ts b/packages/light-client/test/unit/syncInMemory.test.ts index a715bf8da3b4..e7291bcd1d8b 100644 --- a/packages/light-client/test/unit/syncInMemory.test.ts +++ b/packages/light-client/test/unit/syncInMemory.test.ts @@ -54,13 +54,13 @@ describe("syncInMemory", function () { const headerBlockSlot = finalizedBlockSlot + 1; const finalizedState = ssz.altair.BeaconState.defaultValue(); - finalizedState.nextSyncCommittee = getSyncCommittee(syncCommitteesKeys, 0).syncCommittee; const finalizedBlockHeader = ssz.phase0.BeaconBlockHeader.defaultValue(); finalizedBlockHeader.slot = finalizedBlockSlot; finalizedBlockHeader.stateRoot = ssz.altair.BeaconState.hashTreeRoot(finalizedState); - // Create a state that has the finalizedState as finalized checkpoint + // Create a state that has the next sync committee and finalizedState as finalized checkpoint const syncAttestedState = ssz.altair.BeaconState.defaultValue(); + syncAttestedState.nextSyncCommittee = getSyncCommittee(syncCommitteesKeys, 0).syncCommittee; syncAttestedState.finalizedCheckpoint = { epoch: 0, // Checkpoint { epoch, blockRoot } root: ssz.phase0.BeaconBlockHeader.hashTreeRoot(finalizedBlockHeader), diff --git a/packages/light-client/test/unit/validation.test.ts b/packages/light-client/test/unit/validation.test.ts index a826fff7202c..35386143ea16 100644 --- a/packages/light-client/test/unit/validation.test.ts +++ b/packages/light-client/test/unit/validation.test.ts @@ -56,33 +56,32 @@ describe("validation", function () { aggregatePubkey: bls.aggregatePublicKeys(pubkeys), }; - // finalizedCheckpointState must have `nextSyncCommittee` - const finalizedCheckpointState = ssz.altair.BeaconState.defaultViewDU(); - finalizedCheckpointState.nextSyncCommittee = ssz.altair.SyncCommittee.toViewDU(nextSyncCommittee); - // Prove it - const nextSyncCommitteeBranch = new Tree(finalizedCheckpointState.node).getSingleProof( - BigInt(NEXT_SYNC_COMMITTEE_GINDEX) - ); - - // update.header must have stateRoot to finalizedCheckpointState + const finalizedState = ssz.altair.BeaconState.defaultViewDU(); + + // finalized header must have stateRoot to finalizedState const finalizedHeader = defaultBeaconBlockHeader(updateHeaderSlot); - finalizedHeader.stateRoot = finalizedCheckpointState.hashTreeRoot(); + finalizedHeader.stateRoot = finalizedState.hashTreeRoot(); - // syncAttestedState must have `header` as finalizedCheckpoint - const syncAttestedState = ssz.altair.BeaconState.defaultViewDU(); - syncAttestedState.finalizedCheckpoint = ssz.phase0.Checkpoint.toViewDU({ + // attestedState must have `finalizedHeader` as finalizedCheckpoint + const attestedState = ssz.altair.BeaconState.defaultViewDU(); + attestedState.finalizedCheckpoint = ssz.phase0.Checkpoint.toViewDU({ epoch: 0, root: ssz.phase0.BeaconBlockHeader.hashTreeRoot(finalizedHeader), }); - // Prove it - const finalityBranch = new Tree(syncAttestedState.node).getSingleProof(BigInt(FINALIZED_ROOT_GINDEX)); - // finalityHeader must have stateRoot to syncAttestedState - const syncAttestedBlockHeader = defaultBeaconBlockHeader(attestedHeaderSlot); - syncAttestedBlockHeader.stateRoot = syncAttestedState.hashTreeRoot(); + // attested state must contain next sync committees + attestedState.nextSyncCommittee = ssz.altair.SyncCommittee.toViewDU(nextSyncCommittee); + + // attestedHeader must have stateRoot to attestedState + const attestedHeader = defaultBeaconBlockHeader(attestedHeaderSlot); + attestedHeader.stateRoot = attestedState.hashTreeRoot(); + + // Creates proofs for nextSyncCommitteeBranch and finalityBranch rooted in attested state + const nextSyncCommitteeBranch = new Tree(attestedState.node).getSingleProof(BigInt(NEXT_SYNC_COMMITTEE_GINDEX)); + const finalityBranch = new Tree(attestedState.node).getSingleProof(BigInt(FINALIZED_ROOT_GINDEX)); const forkVersion = ssz.Bytes4.defaultValue(); - const signingRoot = getSyncAggregateSigningRoot(config, syncAttestedBlockHeader); + const signingRoot = getSyncAggregateSigningRoot(config, attestedHeader); const syncAggregate = signAndAggregate(signingRoot, sks); const syncCommittee: SyncCommitteeFast = { @@ -91,11 +90,11 @@ describe("validation", function () { }; update = { - attestedHeader: syncAttestedBlockHeader, - nextSyncCommittee: nextSyncCommittee, - nextSyncCommitteeBranch: nextSyncCommitteeBranch, - finalizedHeader: finalizedHeader, - finalityBranch: finalityBranch, + attestedHeader, + nextSyncCommittee, + nextSyncCommitteeBranch, + finalizedHeader, + finalityBranch, syncAggregate, forkVersion, }; diff --git a/packages/light-client/test/utils/prepareUpdateNaive.ts b/packages/light-client/test/utils/prepareUpdateNaive.ts index 70e8b6ff3e17..36cffe7c4d76 100644 --- a/packages/light-client/test/utils/prepareUpdateNaive.ts +++ b/packages/light-client/test/utils/prepareUpdateNaive.ts @@ -28,7 +28,7 @@ export async function prepareUpdateNaive( // // Then the lightclient will verify it signs over `signedHeader`, where // ```js - // signedHeader = finalityHeaderSpecified ? update.finalityHeader : update.header + // signedHeader = update.header // ``` // So if we have a finalized block with `finalityHeader` we need to find a state such that // `state.getBlockRootAtSlot(state.slot - 1) == finalityHeader.root`, then find the block at `state.slot` @@ -55,18 +55,6 @@ export async function prepareUpdateNaive( // │ syncAttestedState │ // └───────────────────────────────────────────┘ // │ - // │ state.finalizedCheckpoint - // ▼ - // ┌───────────────────────────────────────────┐ - // │ finalizedCheckpointBlock <<<< │ - // └───────────────────────────────────────────┘ - // │ - // │ block.stateRoot - // ▼ - // ┌───────────────────────────────────────────┐ - // │ finalizedCheckpointState │ - // └───────────────────────────────────────────┘ - // │ // │ state.nextSyncCommittee // ▼ // ┌───────────────────────────────────────────┐ @@ -100,16 +88,15 @@ export async function prepareUpdateNaive( const syncAttestedStateTree = new Tree(syncAttestedState.node); const finalityBranch = syncAttestedStateTree.getSingleProof(BigInt(FINALIZED_ROOT_GINDEX)); - // Get `nextSyncCommittee` from a finalized state so the lightclient can safely transition to the next committee - const finalizedCheckpointState = await chain.getStateByRoot(finalizedCheckpointBlockHeader.stateRoot); + // Get `nextSyncCommittee` from an attested state so the lightclient can safely transition to the next committee // Prove that the `nextSyncCommittee` is included in a finalized state "attested" by the current sync committee - finalizedCheckpointState.commit(); - const finalizedCheckpointStateTree = new Tree(finalizedCheckpointState.node); - const nextSyncCommitteeBranch = finalizedCheckpointStateTree.getSingleProof(BigInt(NEXT_SYNC_COMMITTEE_GINDEX)); + syncAttestedState.commit(); + const syncAttestedStateStateTree = new Tree(syncAttestedState.node); + const nextSyncCommitteeBranch = syncAttestedStateStateTree.getSingleProof(BigInt(NEXT_SYNC_COMMITTEE_GINDEX)); return { attestedHeader: syncAttestedBlockHeader, - nextSyncCommittee: finalizedCheckpointState.nextSyncCommittee.toValue(), + nextSyncCommittee: syncAttestedState.nextSyncCommittee.toValue(), nextSyncCommitteeBranch: nextSyncCommitteeBranch, finalizedHeader: finalizedCheckpointBlockHeader, finalityBranch: finalityBranch, diff --git a/packages/light-client/test/utils/utils.ts b/packages/light-client/test/utils/utils.ts index f3b10d7ee339..77735568cfad 100644 --- a/packages/light-client/test/utils/utils.ts +++ b/packages/light-client/test/utils/utils.ts @@ -1,16 +1,14 @@ import bls from "@chainsafe/bls/switchable"; import {PointFormat, PublicKey, SecretKey} from "@chainsafe/bls/types"; -import {hash} from "@chainsafe/persistent-merkle-tree"; +import {hash, Tree} from "@chainsafe/persistent-merkle-tree"; import {BitArray, fromHexString} from "@chainsafe/ssz"; import {routes} from "@lodestar/api"; import {IBeaconConfig} from "@lodestar/config"; import { DOMAIN_SYNC_COMMITTEE, EPOCHS_PER_SYNC_COMMITTEE_PERIOD, - FINALIZED_ROOT_DEPTH, - FINALIZED_ROOT_INDEX, - NEXT_SYNC_COMMITTEE_DEPTH, - NEXT_SYNC_COMMITTEE_INDEX, + FINALIZED_ROOT_GINDEX, + NEXT_SYNC_COMMITTEE_GINDEX, SLOTS_PER_EPOCH, SYNC_COMMITTEE_SIZE, } from "@lodestar/params"; @@ -121,37 +119,30 @@ export function computeLightclientUpdate(config: IBeaconConfig, period: SyncPeri const nextSyncCommittee = committeeNext.syncCommittee; - const {root: headerStateRoot, proof: nextSyncCommitteeBranch} = computeMerkleBranch( - ssz.altair.SyncCommittee.hashTreeRoot(nextSyncCommittee), - NEXT_SYNC_COMMITTEE_DEPTH, - NEXT_SYNC_COMMITTEE_INDEX - ); + const finalizedState = ssz.altair.BeaconState.defaultViewDU(); - // finalized header's state root is used to to validate sync committee branch - const finalizedHeader: phase0.BeaconBlockHeader = { - slot: updateSlot, - proposerIndex: 0, - parentRoot: SOME_HASH, - stateRoot: headerStateRoot, - bodyRoot: SOME_HASH, - }; + // finalized header must have stateRoot to finalizedState + const finalizedHeader = defaultBeaconBlockHeader(updateSlot); + finalizedHeader.stateRoot = finalizedState.hashTreeRoot(); - const {root: stateRoot, proof: finalityBranch} = computeMerkleBranch( - ssz.phase0.BeaconBlockHeader.hashTreeRoot(finalizedHeader), - FINALIZED_ROOT_DEPTH, - FINALIZED_ROOT_INDEX - ); + const attestedState = ssz.altair.BeaconState.defaultViewDU(); + attestedState.finalizedCheckpoint = ssz.phase0.Checkpoint.toViewDU({ + epoch: 0, + root: ssz.phase0.BeaconBlockHeader.hashTreeRoot(finalizedHeader), + }); - // attested header's state root is used to validate finality branch - const attestedHeader: phase0.BeaconBlockHeader = { - slot: updateSlot, - proposerIndex: 0, - parentRoot: SOME_HASH, - stateRoot: stateRoot, - bodyRoot: SOME_HASH, - }; + // attested state must contain next sync committees + attestedState.nextSyncCommittee = ssz.altair.SyncCommittee.toViewDU(nextSyncCommittee); + + // attestedHeader must have stateRoot to attestedState + const attestedHeader = defaultBeaconBlockHeader(updateSlot); + attestedHeader.stateRoot = attestedState.hashTreeRoot(); + + // Creates proofs for nextSyncCommitteeBranch and finalityBranch rooted in attested state + const nextSyncCommitteeBranch = new Tree(attestedState.node).getSingleProof(BigInt(NEXT_SYNC_COMMITTEE_GINDEX)); + const finalityBranch = new Tree(attestedState.node).getSingleProof(BigInt(FINALIZED_ROOT_GINDEX)); - const forkVersion = config.getForkVersion(updateSlot); + const forkVersion = ssz.Bytes4.defaultValue(); const syncAggregate = committee.signHeader(config, attestedHeader); return {