fix!: update validator liveness API to be spec compliant

packages/api/src/beacon/routes/validator.ts

@@ -132,7 +132,6 @@ export type SyncCommitteeSelection = {
 
 export type LivenessResponseData = {
   index: ValidatorIndex;
-  epoch: Epoch;
   isLive: boolean;
 };
 
@@ -390,9 +389,9 @@ export type Api = {
 
   /** Returns validator indices that have been observed to be active on the network */
   getLiveness(
-    indices: ValidatorIndex[],
-    epoch: Epoch
-  ): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: LivenessResponseData[]}}>>;
+    epoch: Epoch,
+    validatorIndices: ValidatorIndex[]
+  ): Promise<ApiClientResponse<{[HttpStatusCode.OK]: {data: LivenessResponseData[]}}, HttpStatusCode.BAD_REQUEST>>;
 
   registerValidator(
     registrations: bellatrix.SignedValidatorRegistrationV1[]
@@ -419,7 +418,7 @@ export const routesData: RoutesData<Api> = {
   prepareBeaconProposer: {url: "/eth/v1/validator/prepare_beacon_proposer", method: "POST"},
   submitBeaconCommitteeSelections: {url: "/eth/v1/validator/beacon_committee_selections", method: "POST"},
   submitSyncCommitteeSelections: {url: "/eth/v1/validator/sync_committee_selections", method: "POST"},
-  getLiveness: {url: "/eth/v1/validator/liveness", method: "GET"},
+  getLiveness: {url: "/eth/v1/validator/liveness/{epoch}", method: "POST"},
   registerValidator: {url: "/eth/v1/validator/register_validator", method: "POST"},
 };
 
@@ -441,7 +440,7 @@ export type ReqTypes = {
   prepareBeaconProposer: {body: unknown};
   submitBeaconCommitteeSelections: {body: unknown};
   submitSyncCommitteeSelections: {body: unknown};
-  getLiveness: {query: {indices: ValidatorIndex[]; epoch: Epoch}};
+  getLiveness: {params: {epoch: Epoch}; body: U64Str[]};
   registerValidator: {body: unknown};
 };
 
@@ -578,9 +577,12 @@ export function getReqSerializers(): ReqSerializers<Api, ReqTypes> {
       parseReq: () => [[]],
     },
     getLiveness: {
-      writeReq: (indices, epoch) => ({query: {indices, epoch}}),
-      parseReq: ({query}) => [query.indices, query.epoch],
-      schema: {query: {indices: Schema.UintArray, epoch: Schema.Uint}},
+      writeReq: (epoch, indexes) => ({params: {epoch}, body: indexes.map((i) => toU64Str(i))}),
+      parseReq: ({params, body}) => [params.epoch, body.map((i) => fromU64Str(i))],
+      schema: {
+        params: {epoch: Schema.UintRequired},
+        body: Schema.StringArray,
+      },
     },
     registerValidator: reqOnlyBody(ArrayOf(ssz.bellatrix.SignedValidatorRegistrationV1), Schema.ObjectArray),
   };

packages/api/test/unit/beacon/testData/validator.ts

@@ -100,7 +100,7 @@ export const testData: GenericServerTestCases<Api> = {
     res: {data: [{validatorIndex: 1, slot: 2, subcommitteeIndex: 3, selectionProof}]},
   },
   getLiveness: {
-    args: [[0], 0],
+    args: [0, [0]],
     res: {data: []},
   },
   registerValidator: {

packages/beacon-node/src/api/impl/validator/index.ts

@@ -731,23 +731,23 @@ export function getValidatorApi({
       throw new OnlySupportedByDVT();
     },
 
-    async getLiveness(indices: ValidatorIndex[], epoch: Epoch) {
-      if (indices.length === 0) {
+    async getLiveness(epoch, validatorIndices) {
+      if (validatorIndices.length === 0) {
         return {
           data: [],
         };
       }
       const currentEpoch = chain.clock.currentEpoch;
       if (epoch < currentEpoch - 1 || epoch > currentEpoch + 1) {
-        throw new Error(
+        throw new ApiError(
+          400,
           `Request epoch ${epoch} is more than one epoch before or after the current epoch ${currentEpoch}`
         );
       }
 
       return {
-        data: indices.map((index: ValidatorIndex) => ({
+        data: validatorIndices.map((index) => ({
           index,
-          epoch,
           isLive: chain.validatorSeenAtEpoch(index, epoch),
         })),
       };

packages/beacon-node/test/e2e/api/lodestar/lodestar.test.ts

@@ -65,15 +65,15 @@ describe("api / impl / validator", function () {
 
       const client = getClient({baseUrl: `http://127.0.0.1:${restPort}`}, {config});
 
-      await expect(client.validator.getLiveness([1, 2, 3, 4, 5], 0)).to.eventually.deep.equal(
+      await expect(client.validator.getLiveness(0, [1, 2, 3, 4, 5])).to.eventually.deep.equal(
         {
           response: {
             data: [
-              {index: 1, epoch: 0, isLive: true},
-              {index: 2, epoch: 0, isLive: true},
-              {index: 3, epoch: 0, isLive: true},
-              {index: 4, epoch: 0, isLive: true},
-              {index: 5, epoch: 0, isLive: false},
+              {index: 1, isLive: true},
+              {index: 2, isLive: true},
+              {index: 3, isLive: true},
+              {index: 4, isLive: true},
+              {index: 5, isLive: false},
             ],
           },
           ok: true,
@@ -117,19 +117,19 @@ describe("api / impl / validator", function () {
       const previousEpoch = currentEpoch - 1;
 
       // current epoch is fine
-      await expect(client.validator.getLiveness([1], currentEpoch)).to.not.be.rejected;
+      await expect(client.validator.getLiveness(currentEpoch, [1])).to.not.be.rejected;
       // next epoch is fine
-      await expect(client.validator.getLiveness([1], nextEpoch)).to.not.be.rejected;
+      await expect(client.validator.getLiveness(nextEpoch, [1])).to.not.be.rejected;
       // previous epoch is fine
-      await expect(client.validator.getLiveness([1], previousEpoch)).to.not.be.rejected;
+      await expect(client.validator.getLiveness(previousEpoch, [1])).to.not.be.rejected;
       // more than next epoch is not fine
-      const res1 = await client.validator.getLiveness([1], currentEpoch + 2);
+      const res1 = await client.validator.getLiveness(currentEpoch + 2, [1]);
       expect(res1.ok).to.be.false;
       expect(res1.error?.message).to.include(
         `Request epoch ${currentEpoch + 2} is more than one epoch before or after the current epoch ${currentEpoch}`
       );
       // more than previous epoch is not fine
-      const res2 = await client.validator.getLiveness([1], currentEpoch - 2);
+      const res2 = await client.validator.getLiveness(currentEpoch - 2, [1]);
       expect(res2.ok).to.be.false;
       expect(res2.error?.message).to.include(
         `Request epoch ${currentEpoch - 2} is more than one epoch before or after the current epoch ${currentEpoch}`

packages/validator/src/services/doppelgangerService.ts

@@ -1,5 +1,5 @@
 import {Epoch, ValidatorIndex} from "@lodestar/types";
-import {Api, ApiError} from "@lodestar/api";
+import {Api, ApiError, routes} from "@lodestar/api";
 import {Logger, sleep} from "@lodestar/utils";
 import {computeStartSlotAtEpoch} from "@lodestar/state-transition";
 import {ProcessShutdownCallback, PubkeyHex} from "../types.js";
@@ -12,10 +12,10 @@ import {IndicesService} from "./indices.js";
 const DEFAULT_REMAINING_DETECTION_EPOCHS = 1;
 const REMAINING_EPOCHS_IF_DOPPLEGANGER = Infinity;
 
-export type LivenessResponseData = {
-  index: ValidatorIndex;
+/** Liveness responses for a given epoch */
+type EpochLivenessData = {
   epoch: Epoch;
-  isLive: boolean;
+  responses: routes.validator.LivenessResponseData[];
 };
 
 export type DoppelgangerState = {
@@ -127,34 +127,34 @@ export class DoppelgangerService {
     // in the remaining 25% of the last slot of the previous epoch
     const indicesToCheck = Array.from(indicesToCheckMap.keys());
     const [previous, current] = await Promise.all([
-      this.getLiveness(indicesToCheck, currentEpoch - 1),
-      this.getLiveness(indicesToCheck, currentEpoch),
+      this.getLiveness(currentEpoch - 1, indicesToCheck),
+      this.getLiveness(currentEpoch, indicesToCheck),
     ]);
 
     this.detectDoppelganger(currentEpoch, previous, current, indicesToCheckMap);
   };
 
-  private async getLiveness(indicesToCheck: ValidatorIndex[], epoch: Epoch): Promise<LivenessResponseData[]> {
+  private async getLiveness(epoch: Epoch, indicesToCheck: ValidatorIndex[]): Promise<EpochLivenessData> {
     if (epoch < 0) {
-      return [];
+      return {epoch, responses: []};
     }
 
-    const res = await this.api.validator.getLiveness(indicesToCheck, epoch);
+    const res = await this.api.validator.getLiveness(epoch, indicesToCheck);
     if (!res.ok) {
       this.logger.error(
         `Error getting liveness data for epoch ${epoch}`,
         {},
         new ApiError(res.error.message ?? "", res.error.code, "validator.getLiveness")
       );
-      return [];
+      return {epoch, responses: []};
     }
-    return res.response.data;
+    return {epoch, responses: res.response.data};
   }
 
   private detectDoppelganger(
     currentEpoch: Epoch,
-    previousEpochLiveness: LivenessResponseData[],
-    currentEpochLiveness: LivenessResponseData[],
+    previousEpochLiveness: EpochLivenessData,
+    currentEpochLiveness: EpochLivenessData,
     indicesToCheckMap: Map<ValidatorIndex, PubkeyHex>
   ): void {
     const previousEpoch = currentEpoch - 1;
@@ -165,7 +165,7 @@ export class DoppelgangerService {
     // A following loop will update the states of each validator, depending on whether or not
     // any violators were detected here.
 
-    for (const responses of [previousEpochLiveness, currentEpochLiveness]) {
+    for (const {epoch, responses} of [previousEpochLiveness, currentEpochLiveness]) {
       for (const response of responses) {
         if (!response.isLive) {
           continue;
@@ -177,7 +177,7 @@ export class DoppelgangerService {
           continue;
         }
 
-        if (state.nextEpochToCheck <= response.epoch) {
+        if (state.nextEpochToCheck <= epoch) {
           // Doppleganger detected
           violators.push(response.index);
         }
@@ -209,21 +209,16 @@ export class DoppelgangerService {
       //
       // Do not bother iterating through the current epoch responses since they've already been
       // checked for violators and they don't result in updating the state.
-      for (const response of previousEpochLiveness) {
-        if (response.epoch !== previousEpoch) {
-          // Server sending bad data
-          throw Error(`Inconsistent livenessResponseData epoch ${response.epoch} != ${previousEpoch}`);
-        }
-
+      for (const response of previousEpochLiveness.responses) {
         const state = this.doppelgangerStateByPubkey.get(indicesToCheckMap.get(response.index) ?? "");
         if (!state) {
           this.logger.error(`Inconsistent livenessResponseData unknown index ${response.index}`);
           continue;
         }
 
-        if (!response.isLive && state.nextEpochToCheck <= response.epoch) {
+        if (!response.isLive && state.nextEpochToCheck <= previousEpoch) {
           state.remainingEpochs--;
-          state.nextEpochToCheck = response.epoch + 1;
+          state.nextEpochToCheck = currentEpoch;
           this.metrics?.doppelganger.epochsChecked.inc(1);
 
           const {remainingEpochs} = state;

packages/validator/test/unit/services/doppleganger.test.ts

@@ -145,15 +145,15 @@ type LivenessMap = Map<Epoch, Map<ValidatorIndex, boolean>>;
 function getMockBeaconApi(livenessMap: LivenessMap): Api {
   return {
     validator: {
-      async getLiveness(indices, epoch) {
+      async getLiveness(epoch, validatorIndices) {
         return {
           response: {
-            data: indices.map((index) => {
+            data: validatorIndices.map((index) => {
               const livenessEpoch = livenessMap.get(epoch);
               if (!livenessEpoch) throw Error(`Unknown epoch ${epoch}`);
               const isLive = livenessEpoch.get(index);
               if (isLive === undefined) throw Error(`No liveness for epoch ${epoch} index ${index}`);
-              return {index, epoch, isLive};
+              return {index, isLive};
             }),
           },
           ok: true,