Implement light-client spec tests

packages/beacon-node/src/chain/lightClient/index.ts

@@ -1,6 +1,12 @@
 import {altair, phase0, Root, RootHex, Slot, ssz, SyncPeriod} from "@lodestar/types";
 import {IChainForkConfig} from "@lodestar/config";
-import {CachedBeaconStateAltair, computeSyncPeriodAtEpoch, computeSyncPeriodAtSlot} from "@lodestar/state-transition";
+import {
+  CachedBeaconStateAltair,
+  computeStartSlotAtEpoch,
+  computeSyncPeriodAtEpoch,
+  computeSyncPeriodAtSlot,
+} from "@lodestar/state-transition";
+import {isBetterUpdate, toLightClientUpdateSummary, LightClientUpdateSummary} from "@lodestar/light-client/spec";
 import {ILogger, MapDef, pruneSetToMax} from "@lodestar/utils";
 import {BitArray, CompositeViewDU, toHexString} from "@chainsafe/ssz";
 import {MIN_SYNC_COMMITTEE_PARTICIPANTS, SYNC_COMMITTEE_SIZE} from "@lodestar/params";
@@ -24,7 +30,7 @@ export type LightClientServerOpts = {
 type DependantRootHex = RootHex;
 type BlockRooHex = RootHex;
 
-type SyncAttestedData = {
+export type SyncAttestedData = {
   attestedHeader: phase0.BeaconBlockHeader;
   /** Precomputed root to prevent re-hashing */
   blockRoot: Uint8Array;
@@ -534,9 +540,29 @@ export class LightClientServer {
     attestedData: SyncAttestedData
   ): Promise<void> {
     const prevBestUpdate = await this.db.bestLightClientUpdate.get(syncPeriod);
-    if (prevBestUpdate && !isBetterUpdate(prevBestUpdate, syncAggregate, attestedData)) {
-      this.metrics?.lightclientServer.updateNotBetter.inc();
-      return;
+
+    if (prevBestUpdate) {
+      const prevBestUpdateSummary = toLightClientUpdateSummary(prevBestUpdate);
+
+      const nextBestUpdate: LightClientUpdateSummary = {
+        activeParticipants: sumBits(syncAggregate.syncCommitteeBits),
+        attestedHeaderSlot: attestedData.attestedHeader.slot,
+        signatureSlot,
+        // The actual finalizedHeader is fetched below. To prevent a DB read we approximate the actual slot.
+        // If update is not finalized finalizedHeaderSlot does not matter (see is_better_update), so setting
+        // to zero to set it some number.
+        finalizedHeaderSlot: attestedData.isFinalized
+          ? computeStartSlotAtEpoch(attestedData.finalizedCheckpoint.epoch)
+          : 0,
+        // All updates include a valid `nextSyncCommitteeBranch`, see below code
+        isSyncCommitteeUpdate: true,
+        isFinalityUpdate: attestedData.isFinalized,
+      };
+
+      if (!isBetterUpdate(prevBestUpdateSummary, nextBestUpdate)) {
+        this.metrics?.lightclientServer.updateNotBetter.inc();
+        return;
+      }
     }
 
     const syncCommitteeWitness = await this.db.syncCommitteeWitness.get(attestedData.blockRoot);
@@ -636,42 +662,6 @@ export class LightClientServer {
   }
 }
 
-/**
- * Returns the update with more bits. On ties, prevUpdate is the better
- *
- * Spec v1.0.1
- * ```python
- * max(store.valid_updates, key=lambda update: sum(update.sync_committee_bits)))
- * ```
- */
-export function isBetterUpdate(
-  prevUpdate: altair.LightClientUpdate,
-  nextSyncAggregate: altair.SyncAggregate,
-  nextSyncAttestedData: SyncAttestedData
-): boolean {
-  const nextBitCount = sumBits(nextSyncAggregate.syncCommitteeBits);
-
-  // Finalized if participation is over 66%
-  if (
-    ssz.altair.LightClientUpdate.fields["finalityBranch"].equals(
-      ssz.altair.LightClientUpdate.fields["finalityBranch"].defaultValue(),
-      prevUpdate.finalityBranch
-    ) &&
-    nextSyncAttestedData.isFinalized &&
-    nextBitCount * 3 > SYNC_COMMITTEE_SIZE * 2
-  ) {
-    return true;
-  }
-
-  // Higher bit count
-  const prevBitCount = sumBits(prevUpdate.syncAggregate.syncCommitteeBits);
-  if (prevBitCount > nextBitCount) return false;
-  if (prevBitCount < nextBitCount) return true;
-
-  // else keep the oldest, lowest chance or re-org and requires less updating
-  return prevUpdate.attestedHeader.slot > nextSyncAttestedData.attestedHeader.slot;
-}
-
 export function sumBits(bits: BitArray): number {
   return bits.getTrueBitIndexes().length;
 }

packages/beacon-node/test/spec/presets/index.test.ts

@@ -8,6 +8,7 @@ import {finality} from "./finality.js";
 import {fork} from "./fork.js";
 import {forkChoiceTest} from "./fork_choice.js";
 import {genesis} from "./genesis.js";
+import {lightClient} from "./light_client/index.js";
 import {merkle} from "./merkle.js";
 import {operations} from "./operations.js";
 import {rewards} from "./rewards.js";
@@ -20,7 +21,7 @@ import {transition} from "./transition.js";
 // because the latest withdrawals we implemented are a breaking change
 const skipOpts: SkipOpts = {
   skippedForks: [],
-  skippedRunners: ["light_client", "sync"],
+  skippedRunners: ["sync"],
   skippedHandlers: ["full_withdrawals", "partial_withdrawals", "bls_to_execution_change", "withdrawals"],
 };
 
@@ -34,6 +35,7 @@ specTestIterator(
     fork: {type: RunnerType.default, fn: fork},
     fork_choice: {type: RunnerType.default, fn: forkChoiceTest},
     genesis: {type: RunnerType.default, fn: genesis},
+    light_client: {type: RunnerType.default, fn: lightClient},
     merkle: {type: RunnerType.default, fn: merkle},
     operations: {type: RunnerType.default, fn: operations},
     random: {type: RunnerType.default, fn: sanityBlocks},

packages/beacon-node/test/spec/presets/light_client/index.ts

@@ -0,0 +1,21 @@
+import {TestRunnerFn} from "../../utils/types.js";
+import {singleMerkleProof} from "./single_merkle_proof.js";
+import {sync} from "./sync.js";
+import {updateRanking} from "./update_ranking.js";
+
+/* eslint-disable @typescript-eslint/naming-convention */
+
+export const lightClient: TestRunnerFn<any, any> = (fork, testName, testSuite) => {
+  const testFn = lightclientTestFns[testName];
+  if (testFn === undefined) {
+    throw Error(`Unknown lightclient test ${testName}`);
+  }
+
+  return testFn(fork, testName, testSuite);
+};
+
+const lightclientTestFns: Record<string, TestRunnerFn<any, any>> = {
+  single_merkle_proof: singleMerkleProof,
+  sync: sync,
+  update_ranking: updateRanking,
+};

packages/beacon-node/test/spec/presets/light_client/single_merkle_proof.ts

@@ -0,0 +1,57 @@
+import {expect} from "chai";
+import {RootHex, ssz} from "@lodestar/types";
+import {InputType} from "@lodestar/spec-test-util";
+import {ForkName} from "@lodestar/params";
+import {Tree} from "@chainsafe/persistent-merkle-tree";
+import {TreeViewDU, Type} from "@chainsafe/ssz";
+import {toHex} from "@lodestar/utils";
+import {TestRunnerFn} from "../../utils/types.js";
+
+/* eslint-disable @typescript-eslint/naming-convention */
+
+// https://github.com/ethereum/consensus-specs/blob/da3f5af919be4abb5a6db5a80b235deb8b4b5cba/tests/formats/light_client/single_merkle_proof.md
+type SingleMerkleProofTestCase = {
+  meta?: any;
+  object: TreeViewDU<any>;
+  // leaf: Bytes32            # string, hex encoded, with 0x prefix
+  // leaf_index: int          # integer, decimal
+  // branch: list of Bytes32  # list, each element is a string, hex encoded, with 0x prefix
+  proof: {
+    leaf: RootHex;
+    leaf_index: bigint;
+    branch: RootHex[];
+  };
+};
+
+export const singleMerkleProof: TestRunnerFn<SingleMerkleProofTestCase, RootHex[]> = (fork, testHandler, testSuite) => {
+  return {
+    testFunction: (testcase) => {
+      // Assert correct proof generation
+      const branch = new Tree(testcase.object.node).getSingleProof(testcase.proof.leaf_index);
+      return branch.map(toHex);
+    },
+    options: {
+      inputTypes: {
+        object: InputType.SSZ_SNAPPY,
+        proof: InputType.YAML,
+      },
+      sszTypes: {
+        object: getObjectType(fork, testSuite),
+      },
+      getExpected: (testCase) => testCase.proof.branch,
+      expectFunc: (testCase, expected, actual) => {
+        expect(actual).deep.equals(expected);
+      },
+      // Do not manually skip tests here, do it in packages/beacon-node/test/spec/presets/index.test.ts
+    },
+  };
+};
+
+function getObjectType(fork: ForkName, objectName: string): Type<unknown> {
+  switch (objectName) {
+    case "BeaconState":
+      return ssz[fork].BeaconState;
+    default:
+      throw Error(`Unknown objectName ${objectName}`);
+  }
+}