Updating light client proof logic to root next sync committee in atte…

…sted 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
ChainSafe · Sep 7, 2022 · cdac3ae · cdac3ae
1 parent 36c6eb0
commit cdac3ae
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 119 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {