From 201843df835a78f417ca9e78fa84d3d0f25994c9 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Wed, 5 Apr 2023 11:58:17 +0200 Subject: [PATCH 01/11] Add beacon and sync committee selection APIs --- packages/api/src/beacon/routes/validator.ts | 106 ++++++++++++++++++ .../test/unit/beacon/testData/validator.ts | 9 ++ packages/beacon-node/src/api/impl/errors.ts | 8 ++ .../src/api/impl/validator/index.ts | 10 +- 4 files changed, 132 insertions(+), 1 deletion(-) diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index 12f8fb97139f..458255b5d86f 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -16,6 +16,7 @@ import { ValidatorIndex, RootHex, StringType, + SubcommitteeIndex, Wei, } from "@lodestar/types"; import {ApiClientResponse} from "../../interfaces.js"; @@ -97,6 +98,32 @@ export type SyncDuty = { validatorSyncCommitteeIndices: number[]; }; +/** + * From https://github.com/ethereum/beacon-APIs/pull/224 + */ +export type BeaconCommitteeSelection = { + /** Index of the validator */ + validatorIndex: ValidatorIndex; + /** The slot at which a validator is assigned to attest */ + slot: Slot; + /** The `slot_signature` calculated by the validator for the upcoming attestation slot */ + selectionProof: BLSSignature; +}; + +/** + * From https://github.com/ethereum/beacon-APIs/pull/224 + */ +export type SyncCommitteeSelection = { + /** Index of the validator */ + validatorIndex: ValidatorIndex; + /** The slot at which validator is assigned to produce a sync committee contribution */ + slot: Slot; + /** SubcommitteeIndex to which the validator is assigned */ + subcommitteeIndex: SubcommitteeIndex; + /** The `slot_signature` calculated by the validator for the upcoming sync committee slot */ + selectionProof: BLSSignature; +}; + export type LivenessResponseData = { index: ValidatorIndex; epoch: Epoch; @@ -303,6 +330,52 @@ export type Api = { proposers: ProposerPreparationData[] ): Promise>; + /** + * Determine if a distributed validator has been selected to aggregate attestations + * + * This endpoint is implemented by a distributed validator middleware client to exchange + * partial beacon committee selection proofs for combined/aggregated selection proofs to allow + * a validator client to correctly determine if one of its validators has been selected to + * perform an aggregation duty in this slot. + * + * Note that this endpoint is not implemented by the beacon node and will return a 501 error + * + * @param requestBody An array of partial beacon committee selection proofs + * @returns An array of threshold aggregated beacon committee selection proofs + * @throws ApiError + */ + submitBeaconCommitteeSelections( + selections: BeaconCommitteeSelection[] + ): Promise< + ApiClientResponse< + {[HttpStatusCode.OK]: {data: BeaconCommitteeSelection[]}}, + HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_IMPLEMENTED | HttpStatusCode.SERVICE_UNAVAILABLE + > + >; + + /** + * Determine if a distributed validator has been selected to make a sync committee contribution + * + * This endpoint is implemented by a distributed validator middleware client to exchange + * partial sync committee selection proofs for combined/aggregated selection proofs to allow + * a validator client to correctly determine if one of its validators has been selected to + * perform a sync committee contribution (sync aggregation) duty in this slot. + * + * Note that this endpoint is not implemented by the beacon node and will return a 501 error + * + * @param requestBody An array of partial sync committee selection proofs + * @returns An array of threshold aggregated sync committee selection proofs + * @throws ApiError + */ + submitSyncCommitteeSelections( + selections: SyncCommitteeSelection[] + ): Promise< + ApiClientResponse< + {[HttpStatusCode.OK]: {data: SyncCommitteeSelection[]}}, + HttpStatusCode.BAD_REQUEST | HttpStatusCode.NOT_IMPLEMENTED | HttpStatusCode.SERVICE_UNAVAILABLE + > + >; + /** Returns validator indices that have been observed to be active on the network */ getLiveness( indices: ValidatorIndex[], @@ -332,6 +405,8 @@ export const routesData: RoutesData = { prepareBeaconCommitteeSubnet: {url: "/eth/v1/validator/beacon_committee_subscriptions", method: "POST"}, prepareSyncCommitteeSubnets: {url: "/eth/v1/validator/sync_committee_subscriptions", method: "POST"}, 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"}, registerValidator: {url: "/eth/v1/validator/register_validator", method: "POST"}, }; @@ -352,10 +427,31 @@ export type ReqTypes = { prepareBeaconCommitteeSubnet: {body: unknown}; prepareSyncCommitteeSubnets: {body: unknown}; prepareBeaconProposer: {body: unknown}; + submitBeaconCommitteeSelections: {body: unknown}; + submitSyncCommitteeSelections: {body: unknown}; getLiveness: {query: {indices: ValidatorIndex[]; epoch: Epoch}}; registerValidator: {body: unknown}; }; +const BeaconCommitteeSelection = new ContainerType( + { + validatorIndex: ssz.ValidatorIndex, + slot: ssz.Slot, + selectionProof: ssz.BLSSignature, + }, + {jsonCase: "eth2"} +); + +const SyncCommitteeSelection = new ContainerType( + { + validatorIndex: ssz.ValidatorIndex, + slot: ssz.Slot, + subcommitteeIndex: ssz.SubcommitteeIndex, + selectionProof: ssz.BLSSignature, + }, + {jsonCase: "eth2"} +); + export function getReqSerializers(): ReqSerializers { const BeaconCommitteeSubscription = new ContainerType( { @@ -461,6 +557,14 @@ export function getReqSerializers(): ReqSerializers { ], schema: {body: Schema.ObjectArray}, }, + submitBeaconCommitteeSelections: { + writeReq: (items) => ({body: ArrayOf(BeaconCommitteeSelection).toJson(items)}), + parseReq: () => [[]], + }, + submitSyncCommitteeSelections: { + writeReq: (items) => ({body: ArrayOf(SyncCommitteeSelection).toJson(items)}), + parseReq: () => [[]], + }, getLiveness: { writeReq: (indices, epoch) => ({query: {indices, epoch}}), parseReq: ({query}) => [query.indices, query.epoch], @@ -532,6 +636,8 @@ export function getReturnTypes(): ReturnTypes { produceAttestationData: ContainerData(ssz.phase0.AttestationData), produceSyncCommitteeContribution: ContainerData(ssz.altair.SyncCommitteeContribution), getAggregatedAttestation: ContainerData(ssz.phase0.Attestation), + submitBeaconCommitteeSelections: ContainerData(ArrayOf(BeaconCommitteeSelection)), + submitSyncCommitteeSelections: ContainerData(ArrayOf(SyncCommitteeSelection)), getLiveness: jsonType("snake"), }; } diff --git a/packages/api/test/unit/beacon/testData/validator.ts b/packages/api/test/unit/beacon/testData/validator.ts index e6939614ec73..90792f467d7f 100644 --- a/packages/api/test/unit/beacon/testData/validator.ts +++ b/packages/api/test/unit/beacon/testData/validator.ts @@ -6,6 +6,7 @@ import {GenericServerTestCases} from "../../../utils/genericServerTest.js"; const ZERO_HASH = Buffer.alloc(32, 0); const ZERO_HASH_HEX = "0x" + ZERO_HASH.toString("hex"); const randaoReveal = Buffer.alloc(96, 1); +const selectionProof = Buffer.alloc(96, 1); const graffiti = "a".repeat(32); export const testData: GenericServerTestCases = { @@ -90,6 +91,14 @@ export const testData: GenericServerTestCases = { args: [[{validatorIndex: "1", feeRecipient: "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b"}]], res: undefined, }, + submitBeaconCommitteeSelections: { + args: [[]], + res: {data: [{validatorIndex: 1, slot: 2, selectionProof}]}, + }, + submitSyncCommitteeSelections: { + args: [[]], + res: {data: [{validatorIndex: 1, slot: 2, subcommitteeIndex: 3, selectionProof}]}, + }, getLiveness: { args: [[0], 0], res: {data: []}, diff --git a/packages/beacon-node/src/api/impl/errors.ts b/packages/beacon-node/src/api/impl/errors.ts index d520013a53de..2830a0438f3b 100644 --- a/packages/beacon-node/src/api/impl/errors.ts +++ b/packages/beacon-node/src/api/impl/errors.ts @@ -31,3 +31,11 @@ export class NodeIsSyncing extends ApiError { super(503, `Node is syncing - ${statusMsg}`); } } + +// Error thrown by beacon node APIs that are only supported by distributed validator middleware clients +// For example https://github.com/ethereum/beacon-APIs/blob/f087fbf2764e657578a6c29bdf0261b36ee8db1e/apis/validator/beacon_committee_selections.yaml +export class OnlySupportedByDVT extends ApiError { + constructor() { + super(501, "Only supported by distributed validator middleware clients"); + } +} diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index fa7c27864e49..f43b0d5c03fb 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -19,7 +19,7 @@ import {ZERO_HASH} from "../../../constants/index.js"; import {SyncState} from "../../../sync/index.js"; import {isOptimisticBlock} from "../../../util/forkChoice.js"; import {toGraffitiBuffer} from "../../../util/graffiti.js"; -import {ApiError, NodeIsSyncing} from "../errors.js"; +import {ApiError, NodeIsSyncing, OnlySupportedByDVT} from "../errors.js"; import {validateSyncCommitteeGossipContributionAndProof} from "../../../chain/validation/syncCommitteeContributionAndProof.js"; import {CommitteeSubscription} from "../../../network/subnets/index.js"; import {ApiModules} from "../types.js"; @@ -698,6 +698,14 @@ export function getValidatorApi({ await chain.updateBeaconProposerData(chain.clock.currentEpoch, proposers); }, + async submitBeaconCommitteeSelections() { + throw new OnlySupportedByDVT(); + }, + + async submitSyncCommitteeSelections() { + throw new OnlySupportedByDVT(); + }, + async getLiveness(indices: ValidatorIndex[], epoch: Epoch) { if (indices.length === 0) { return { From 6ade8b696b1db312dcf143c1c61301705a95b80a Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Wed, 5 Apr 2023 12:10:38 +0200 Subject: [PATCH 02/11] Implement distributed attestation aggregation selection --- .../validator/src/services/attestation.ts | 113 +++++++++++++++++- .../src/services/attestationDuties.ts | 31 +++-- packages/validator/src/validator.ts | 1 + .../test/unit/services/attestation.test.ts | 1 + .../unit/services/attestationDuties.test.ts | 21 ++-- 5 files changed, 142 insertions(+), 25 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 31d21c1cba0b..9fe00c6c3dec 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -1,7 +1,7 @@ import {phase0, Slot, ssz} from "@lodestar/types"; -import {computeEpochAtSlot} from "@lodestar/state-transition"; +import {computeEpochAtSlot, isAggregatorFromCommitteeLength} from "@lodestar/state-transition"; import {sleep} from "@lodestar/utils"; -import {Api, ApiError} from "@lodestar/api"; +import {Api, ApiError, routes} from "@lodestar/api"; import {toHexString} from "@chainsafe/ssz"; import {IClock, LoggerVc} from "../util/index.js"; import {PubkeyHex} from "../types.js"; @@ -15,6 +15,7 @@ import {ValidatorEventEmitter} from "./emitter.js"; export type AttestationServiceOpts = { afterBlockDelaySlotFraction?: number; disableAttestationGrouping?: boolean; + distributedAggregationSelection?: boolean; }; /** @@ -42,7 +43,9 @@ export class AttestationService { private readonly metrics: Metrics | null, private readonly opts?: AttestationServiceOpts ) { - this.dutiesService = new AttestationDutiesService(logger, api, clock, validatorStore, chainHeadTracker, metrics); + this.dutiesService = new AttestationDutiesService(logger, api, clock, validatorStore, chainHeadTracker, metrics, { + distributedAggregationSelection: opts?.distributedAggregationSelection, + }); // At most every slot, check existing duties from AttestationDutiesService and run tasks clock.runEverySlot(this.runAttestationTasks); @@ -59,6 +62,18 @@ export class AttestationService { return; } + if (this.opts?.distributedAggregationSelection) { + // Validator in distributed cluster only has a key share, not the full private key. + // The partial selection proofs must be exchanged for combined selection proofs by + // calling submitBeaconCommitteeSelections on the distributed validator middleware client. + // This will run in parallel to other attestation tasks but must be finished before starting + // attestation aggregation as it is required to correctly determine if validator is aggregator + // and to produce a AggregateAndProof that can be threshold aggregated by the middleware client. + this.runDistributedAggregationSelectionTasks(duties, slot, signal).catch((e) => + this.logger.error("Error on attestation aggregation selection", {slot}, e) + ); + } + // A validator should create and broadcast the attestation to the associated attestation subnet when either // (a) the validator has received a valid block from the expected block proposer for the assigned slot or // (b) one-third of the slot has transpired (SECONDS_PER_SLOT / 3 seconds after the start of slot) -- whichever comes first. @@ -231,7 +246,7 @@ export class AttestationService { const logCtx = {slot: attestation.slot, index: attestation.index}; // No validator is aggregator, skip - if (duties.every(({selectionProof}) => selectionProof === null)) { + if (duties.every(({isAggregator}) => !isAggregator)) { return; } @@ -247,11 +262,11 @@ export class AttestationService { const signedAggregateAndProofs: phase0.SignedAggregateAndProof[] = []; await Promise.all( - duties.map(async ({duty, selectionProof}) => { + duties.map(async ({duty, selectionProof, isAggregator}) => { const logCtxValidator = {...logCtx, validatorIndex: duty.validatorIndex}; try { // Produce signed aggregates only for validators that are subscribed aggregators. - if (selectionProof !== null) { + if (isAggregator) { signedAggregateAndProofs.push( await this.validatorStore.signAggregateAndProof(duty, selectionProof, aggregate.data) ); @@ -276,4 +291,90 @@ export class AttestationService { } } } + + /** + * Performs additional steps required if validator is part of distributed cluster + * + * 1. Exchange partial for combined selection proofs + * 2. Determine validators that should aggregate attestations + * 3. Resubscribe aggregators on beacon committee subnets + * + * See https://docs.google.com/document/d/1q9jOTPcYQa-3L8luRvQJ-M0eegtba4Nmon3dpO79TMk/mobilebasic + */ + private async runDistributedAggregationSelectionTasks( + duties: AttDutyAndProof[], + slot: number, + signal: AbortSignal + ): Promise { + const partialSelections: routes.validator.BeaconCommitteeSelection[] = duties.map(({duty, selectionProof}) => ({ + validatorIndex: duty.validatorIndex, + slot, + selectionProof, + })); + + this.logger.debug("Submitting partial selection proofs", {slot, count: partialSelections.length}); + + const res = await Promise.race([ + this.api.validator.submitBeaconCommitteeSelections(partialSelections), + // Exit aggregations flow if there is no response after 1/3 of slot as + // beacon node would likely not have enough time to prepare an aggregate attestation. + // Note that the aggregations flow is not explicitly exited but rather will be skipped + // due to the fact that calculation of `is_aggregator` in duties service is not done + // and defaulted to `isAggregator=false`, meaning no validator will be an aggregator. + sleep(this.clock.msToSlot(slot + 1 / 3), signal), + ]); + + if (!res) { + throw new Error("No response after 1/3 of slot"); + } + ApiError.assert(res); + + const combinedSelections = res.response.data; + this.logger.debug("Received combined selection proofs", {slot, count: combinedSelections.length}); + + const beaconCommitteeSubscriptions: routes.validator.BeaconCommitteeSubscription[] = []; + + for (const dutyAndProof of duties) { + const {validatorIndex, committeeIndex, committeeLength, committeesAtSlot} = dutyAndProof.duty; + const selection = combinedSelections.find((s) => s.validatorIndex === validatorIndex); + const logCtxValidator = {slot, index: committeeIndex, validatorIndex}; + + if (!selection) { + this.logger.warn("Did not receive combined selection proof", logCtxValidator); + continue; + } + + const isAggregator = isAggregatorFromCommitteeLength(committeeLength, selection.selectionProof); + + // Replace partial with combined selection proof by mutating object + dutyAndProof.selectionProof = selection.selectionProof; + dutyAndProof.isAggregator = isAggregator; + + if (isAggregator) { + // Only push subnet subscriptions with `isAggregator=true` as all validators + // with duties for slot are already subscribed to subnets with `isAggregator=false`. + beaconCommitteeSubscriptions.push({ + validatorIndex, + committeesAtSlot, + committeeIndex, + slot, + isAggregator, + }); + this.logger.debug("Resubscribing validator as aggregator on beacon committee subnet", logCtxValidator); + } + } + + // If there are any subscriptions with aggregators, push them out to the beacon node. + if (beaconCommitteeSubscriptions.length > 0) { + try { + ApiError.assert(await this.api.validator.prepareBeaconCommitteeSubnet(beaconCommitteeSubscriptions)); + this.logger.debug("Resubscribed validators as aggregators on beacon committee subnet", { + slot, + count: beaconCommitteeSubscriptions.length, + }); + } catch (e) { + this.logger.error("Failed to resubscribe to beacon committee subnets", {slot}, e as Error); + } + } + } } diff --git a/packages/validator/src/services/attestationDuties.ts b/packages/validator/src/services/attestationDuties.ts index 8b419f8da792..03972c64377e 100644 --- a/packages/validator/src/services/attestationDuties.ts +++ b/packages/validator/src/services/attestationDuties.ts @@ -24,13 +24,19 @@ const SUBSCRIPTIONS_PER_REQUEST = 8738; /** Neatly joins the server-generated `AttesterData` with the locally-generated `selectionProof`. */ export type AttDutyAndProof = { duty: routes.validator.AttesterDuty; - /** This value is only set to not null if the proof indicates that the validator is an aggregator. */ - selectionProof: BLSSignature | null; + /** Locally-generated selection proof, only partial if validator is part of distributed cluster */ + selectionProof: BLSSignature; + /** Whether the validator is an aggregator */ + isAggregator: boolean; }; // To assist with readability type AttDutiesAtEpoch = {dependentRoot: RootHex; dutiesByIndex: Map}; +type AttestationDutiesServiceOpts = { + distributedAggregationSelection?: boolean; +}; + export class AttestationDutiesService { /** Maps a validator public key to their duties for each epoch */ private readonly dutiesByIndexByEpoch = new Map(); @@ -46,7 +52,8 @@ export class AttestationDutiesService { private clock: IClock, private readonly validatorStore: ValidatorStore, chainHeadTracker: ChainHeaderTracker, - private readonly metrics: Metrics | null + private readonly metrics: Metrics | null, + private readonly opts?: AttestationDutiesServiceOpts ) { // Running this task every epoch is safe since a re-org of two epochs is very unlikely // TODO: If the re-org event is reliable consider re-running then @@ -176,14 +183,14 @@ export class AttestationDutiesService { for (const epoch of [currentEpoch, nextEpoch]) { const epochDuties = this.dutiesByIndexByEpoch.get(epoch)?.dutiesByIndex; if (epochDuties) { - for (const {duty, selectionProof} of epochDuties.values()) { + for (const {duty, isAggregator} of epochDuties.values()) { if (indexSet.has(duty.validatorIndex)) { beaconCommitteeSubscriptions.push({ validatorIndex: duty.validatorIndex, committeesAtSlot: duty.committeesAtSlot, committeeIndex: duty.committeeIndex, slot: duty.slot, - isAggregator: selectionProof !== null, + isAggregator, }); } } @@ -326,13 +333,15 @@ export class AttestationDutiesService { private async getDutyAndProof(duty: routes.validator.AttesterDuty): Promise { const selectionProof = await this.validatorStore.signAttestationSelectionProof(duty.pubkey, duty.slot); - const isAggregator = isAggregatorFromCommitteeLength(duty.committeeLength, selectionProof); - return { - duty, - // selectionProof === null is used to check if is aggregator - selectionProof: isAggregator ? selectionProof : null, - }; + if (this.opts?.distributedAggregationSelection) { + // Validator in distributed cluster only has a key share, not the full private key. + // Passing a partial selection proof to `is_aggregator` would produce incorrect result. + // Attestation service will combine selection proofs and determine aggregators at beginning of the slot. + return {duty, selectionProof, isAggregator: false}; + } + + return {duty, selectionProof, isAggregator: isAggregatorFromCommitteeLength(duty.committeeLength, selectionProof)}; } /** Run once per epoch to prune duties map */ diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index d52e02a5bf82..84c688cdb45f 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -129,6 +129,7 @@ export class Validator { { afterBlockDelaySlotFraction: opts.afterBlockDelaySlotFraction, disableAttestationGrouping: opts.disableAttestationGrouping || opts.distributed, + distributedAggregationSelection: opts.distributed, } ); diff --git a/packages/validator/test/unit/services/attestation.test.ts b/packages/validator/test/unit/services/attestation.test.ts index a53314dd359e..2f3ca33d3de4 100644 --- a/packages/validator/test/unit/services/attestation.test.ts +++ b/packages/validator/test/unit/services/attestation.test.ts @@ -85,6 +85,7 @@ describe("AttestationService", function () { pubkey: pubkeys[0], }, selectionProof: ZERO_HASH, + isAggregator: true, }, ]; diff --git a/packages/validator/test/unit/services/attestationDuties.test.ts b/packages/validator/test/unit/services/attestationDuties.test.ts index 09e2ef70198d..0164c2d68ec4 100644 --- a/packages/validator/test/unit/services/attestationDuties.test.ts +++ b/packages/validator/test/unit/services/attestationDuties.test.ts @@ -3,7 +3,7 @@ import {expect} from "chai"; import sinon from "sinon"; import {chainConfig} from "@lodestar/config/default"; import bls from "@chainsafe/bls"; -import {toHexString} from "@chainsafe/ssz"; +import {fromHexString, toHexString} from "@chainsafe/ssz"; import {HttpStatusCode, routes} from "@lodestar/api"; import {ssz} from "@lodestar/types"; import {computeEpochAtSlot} from "@lodestar/state-transition"; @@ -35,6 +35,11 @@ describe("AttestationDutiesService", function () { status: "active", validator: ssz.phase0.Validator.defaultValue(), }; + const signedAttSelectionProof = fromHexString( + "0x8d80fe4be57500d2fe4f99c7d5586d9bd65ea4f9e4def0591020dd66f7d1daad" + + "1cea7520beb815423e2bc8316949ac2606da80d5d00df34f352b5a946d6a4bb4" + + "7e402f5d1167dec97af9742e61820625c5c792ddd2b8796962243d8e8cbeadee" + ); before(() => { const secretKeys = [bls.SecretKey.fromBytes(toBufferBE(BigInt(98), 32))]; @@ -96,8 +101,8 @@ describe("AttestationDutiesService", function () { Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(epoch)?.dutiesByIndex || new Map()) ).to.deep.equal( { - // Since the ZERO_HASH won't pass the isAggregator test, selectionProof is null - [index]: {duty, selectionProof: null}, + // Since the ZERO_HASH won't pass the isAggregator test + [index]: {duty, selectionProof: signedAttSelectionProof, isAggregator: false}, }, "Wrong dutiesService.attesters Map at current epoch" ); @@ -105,14 +110,14 @@ describe("AttestationDutiesService", function () { Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(epoch + 1)?.dutiesByIndex || new Map()) ).to.deep.equal( { - // Since the ZERO_HASH won't pass the isAggregator test, selectionProof is null - [index]: {duty, selectionProof: null}, + // Since the ZERO_HASH won't pass the isAggregator test + [index]: {duty, selectionProof: signedAttSelectionProof, isAggregator: false}, }, "Wrong dutiesService.attesters Map at next epoch" ); expect(dutiesService.getDutiesAtSlot(slot)).to.deep.equal( - [{duty, selectionProof: null}], + [{duty, selectionProof: signedAttSelectionProof, isAggregator: false}], "Wrong getAttestersAtSlot()" ); @@ -165,13 +170,13 @@ describe("AttestationDutiesService", function () { // first confirm duties for this and next epoch should be persisted expect(Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(0)?.dutiesByIndex || new Map())).to.deep.equal( { - 4: {duty: duty, selectionProof: null}, + 4: {duty: duty, selectionProof: signedAttSelectionProof, isAggregator: false}, }, "Wrong dutiesService.attesters Map at current epoch" ); expect(Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(1)?.dutiesByIndex || new Map())).to.deep.equal( { - 4: {duty: duty, selectionProof: null}, + 4: {duty: duty, selectionProof: signedAttSelectionProof, isAggregator: false}, }, "Wrong dutiesService.attesters Map at current epoch" ); From 35c0243cf0b14b073b95a55942e3b4d4989908d3 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Wed, 5 Apr 2023 23:46:11 +0200 Subject: [PATCH 03/11] Add partial selection proof to duty object --- .../validator/src/services/attestation.ts | 51 ++++++++++--------- .../src/services/attestationDuties.ts | 25 +++++---- .../test/unit/services/attestation.test.ts | 1 - .../unit/services/attestationDuties.test.ts | 21 +++----- 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 9fe00c6c3dec..26e853c945b5 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -1,4 +1,4 @@ -import {phase0, Slot, ssz} from "@lodestar/types"; +import {BLSSignature, phase0, Slot, ssz} from "@lodestar/types"; import {computeEpochAtSlot, isAggregatorFromCommitteeLength} from "@lodestar/state-transition"; import {sleep} from "@lodestar/utils"; import {Api, ApiError, routes} from "@lodestar/api"; @@ -246,7 +246,7 @@ export class AttestationService { const logCtx = {slot: attestation.slot, index: attestation.index}; // No validator is aggregator, skip - if (duties.every(({isAggregator}) => !isAggregator)) { + if (duties.every(({selectionProof}) => selectionProof === null)) { return; } @@ -262,11 +262,11 @@ export class AttestationService { const signedAggregateAndProofs: phase0.SignedAggregateAndProof[] = []; await Promise.all( - duties.map(async ({duty, selectionProof, isAggregator}) => { + duties.map(async ({duty, selectionProof}) => { const logCtxValidator = {...logCtx, validatorIndex: duty.validatorIndex}; try { // Produce signed aggregates only for validators that are subscribed aggregators. - if (isAggregator) { + if (selectionProof !== null) { signedAggregateAndProofs.push( await this.validatorStore.signAggregateAndProof(duty, selectionProof, aggregate.data) ); @@ -293,11 +293,12 @@ export class AttestationService { } /** - * Performs additional steps required if validator is part of distributed cluster + * Performs additional attestation aggregation tasks required if validator is part of distributed cluster * * 1. Exchange partial for combined selection proofs * 2. Determine validators that should aggregate attestations - * 3. Resubscribe aggregators on beacon committee subnets + * 3. Mutate duty objects to set selection proofs for aggregators + * 4. Resubscribe validators as aggregators on beacon committee subnets * * See https://docs.google.com/document/d/1q9jOTPcYQa-3L8luRvQJ-M0eegtba4Nmon3dpO79TMk/mobilebasic */ @@ -306,21 +307,23 @@ export class AttestationService { slot: number, signal: AbortSignal ): Promise { - const partialSelections: routes.validator.BeaconCommitteeSelection[] = duties.map(({duty, selectionProof}) => ({ - validatorIndex: duty.validatorIndex, - slot, - selectionProof, - })); + const partialSelections: routes.validator.BeaconCommitteeSelection[] = duties.map( + ({duty, partialSelectionProof}) => ({ + validatorIndex: duty.validatorIndex, + slot, + selectionProof: partialSelectionProof as BLSSignature, + }) + ); - this.logger.debug("Submitting partial selection proofs", {slot, count: partialSelections.length}); + this.logger.debug("Submitting partial beacon committee selection proofs", {slot, count: partialSelections.length}); const res = await Promise.race([ this.api.validator.submitBeaconCommitteeSelections(partialSelections), - // Exit aggregations flow if there is no response after 1/3 of slot as + // Exit attestation aggregation flow if there is no response after 1/3 of slot as // beacon node would likely not have enough time to prepare an aggregate attestation. // Note that the aggregations flow is not explicitly exited but rather will be skipped // due to the fact that calculation of `is_aggregator` in duties service is not done - // and defaulted to `isAggregator=false`, meaning no validator will be an aggregator. + // and selectionProof is set to null, meaning no validator will be considered an aggregator. sleep(this.clock.msToSlot(slot + 1 / 3), signal), ]); @@ -330,27 +333,27 @@ export class AttestationService { ApiError.assert(res); const combinedSelections = res.response.data; - this.logger.debug("Received combined selection proofs", {slot, count: combinedSelections.length}); + this.logger.debug("Received combined beacon committee selection proofs", {slot, count: combinedSelections.length}); const beaconCommitteeSubscriptions: routes.validator.BeaconCommitteeSubscription[] = []; for (const dutyAndProof of duties) { const {validatorIndex, committeeIndex, committeeLength, committeesAtSlot} = dutyAndProof.duty; - const selection = combinedSelections.find((s) => s.validatorIndex === validatorIndex); const logCtxValidator = {slot, index: committeeIndex, validatorIndex}; - if (!selection) { - this.logger.warn("Did not receive combined selection proof", logCtxValidator); + const combinedSelection = combinedSelections.find((s) => s.validatorIndex === validatorIndex && s.slot === slot); + + if (!combinedSelection) { + this.logger.warn("Did not receive combined beacon committee selection proof", logCtxValidator); continue; } - const isAggregator = isAggregatorFromCommitteeLength(committeeLength, selection.selectionProof); - - // Replace partial with combined selection proof by mutating object - dutyAndProof.selectionProof = selection.selectionProof; - dutyAndProof.isAggregator = isAggregator; + const isAggregator = isAggregatorFromCommitteeLength(committeeLength, combinedSelection.selectionProof); if (isAggregator) { + // Update selection proof by mutating duty object + dutyAndProof.selectionProof = combinedSelection.selectionProof; + // Only push subnet subscriptions with `isAggregator=true` as all validators // with duties for slot are already subscribed to subnets with `isAggregator=false`. beaconCommitteeSubscriptions.push({ @@ -368,7 +371,7 @@ export class AttestationService { if (beaconCommitteeSubscriptions.length > 0) { try { ApiError.assert(await this.api.validator.prepareBeaconCommitteeSubnet(beaconCommitteeSubscriptions)); - this.logger.debug("Resubscribed validators as aggregators on beacon committee subnet", { + this.logger.debug("Resubscribed validators as aggregators on beacon committee subnets", { slot, count: beaconCommitteeSubscriptions.length, }); diff --git a/packages/validator/src/services/attestationDuties.ts b/packages/validator/src/services/attestationDuties.ts index 03972c64377e..8089634d06e4 100644 --- a/packages/validator/src/services/attestationDuties.ts +++ b/packages/validator/src/services/attestationDuties.ts @@ -24,10 +24,10 @@ const SUBSCRIPTIONS_PER_REQUEST = 8738; /** Neatly joins the server-generated `AttesterData` with the locally-generated `selectionProof`. */ export type AttDutyAndProof = { duty: routes.validator.AttesterDuty; - /** Locally-generated selection proof, only partial if validator is part of distributed cluster */ - selectionProof: BLSSignature; - /** Whether the validator is an aggregator */ - isAggregator: boolean; + /** This value is only set to not null if the proof indicates that the validator is an aggregator. */ + selectionProof: BLSSignature | null; + /** This value will only be set if validator is part of distributed cluster and only has a key share */ + partialSelectionProof?: BLSSignature; }; // To assist with readability @@ -183,14 +183,14 @@ export class AttestationDutiesService { for (const epoch of [currentEpoch, nextEpoch]) { const epochDuties = this.dutiesByIndexByEpoch.get(epoch)?.dutiesByIndex; if (epochDuties) { - for (const {duty, isAggregator} of epochDuties.values()) { + for (const {duty, selectionProof} of epochDuties.values()) { if (indexSet.has(duty.validatorIndex)) { beaconCommitteeSubscriptions.push({ validatorIndex: duty.validatorIndex, committeesAtSlot: duty.committeesAtSlot, committeeIndex: duty.committeeIndex, slot: duty.slot, - isAggregator, + isAggregator: selectionProof !== null, }); } } @@ -337,11 +337,18 @@ export class AttestationDutiesService { if (this.opts?.distributedAggregationSelection) { // Validator in distributed cluster only has a key share, not the full private key. // Passing a partial selection proof to `is_aggregator` would produce incorrect result. - // Attestation service will combine selection proofs and determine aggregators at beginning of the slot. - return {duty, selectionProof, isAggregator: false}; + // Attestation service will exchange partial for combined selection proofs retrieved from + // distributed validator middleware client and determine aggregators at beginning of every slot. + return {duty, selectionProof: null, partialSelectionProof: selectionProof}; } - return {duty, selectionProof, isAggregator: isAggregatorFromCommitteeLength(duty.committeeLength, selectionProof)}; + const isAggregator = isAggregatorFromCommitteeLength(duty.committeeLength, selectionProof); + + return { + duty, + // selectionProof === null is used to check if is aggregator + selectionProof: isAggregator ? selectionProof : null, + }; } /** Run once per epoch to prune duties map */ diff --git a/packages/validator/test/unit/services/attestation.test.ts b/packages/validator/test/unit/services/attestation.test.ts index 2f3ca33d3de4..a53314dd359e 100644 --- a/packages/validator/test/unit/services/attestation.test.ts +++ b/packages/validator/test/unit/services/attestation.test.ts @@ -85,7 +85,6 @@ describe("AttestationService", function () { pubkey: pubkeys[0], }, selectionProof: ZERO_HASH, - isAggregator: true, }, ]; diff --git a/packages/validator/test/unit/services/attestationDuties.test.ts b/packages/validator/test/unit/services/attestationDuties.test.ts index 0164c2d68ec4..09e2ef70198d 100644 --- a/packages/validator/test/unit/services/attestationDuties.test.ts +++ b/packages/validator/test/unit/services/attestationDuties.test.ts @@ -3,7 +3,7 @@ import {expect} from "chai"; import sinon from "sinon"; import {chainConfig} from "@lodestar/config/default"; import bls from "@chainsafe/bls"; -import {fromHexString, toHexString} from "@chainsafe/ssz"; +import {toHexString} from "@chainsafe/ssz"; import {HttpStatusCode, routes} from "@lodestar/api"; import {ssz} from "@lodestar/types"; import {computeEpochAtSlot} from "@lodestar/state-transition"; @@ -35,11 +35,6 @@ describe("AttestationDutiesService", function () { status: "active", validator: ssz.phase0.Validator.defaultValue(), }; - const signedAttSelectionProof = fromHexString( - "0x8d80fe4be57500d2fe4f99c7d5586d9bd65ea4f9e4def0591020dd66f7d1daad" + - "1cea7520beb815423e2bc8316949ac2606da80d5d00df34f352b5a946d6a4bb4" + - "7e402f5d1167dec97af9742e61820625c5c792ddd2b8796962243d8e8cbeadee" - ); before(() => { const secretKeys = [bls.SecretKey.fromBytes(toBufferBE(BigInt(98), 32))]; @@ -101,8 +96,8 @@ describe("AttestationDutiesService", function () { Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(epoch)?.dutiesByIndex || new Map()) ).to.deep.equal( { - // Since the ZERO_HASH won't pass the isAggregator test - [index]: {duty, selectionProof: signedAttSelectionProof, isAggregator: false}, + // Since the ZERO_HASH won't pass the isAggregator test, selectionProof is null + [index]: {duty, selectionProof: null}, }, "Wrong dutiesService.attesters Map at current epoch" ); @@ -110,14 +105,14 @@ describe("AttestationDutiesService", function () { Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(epoch + 1)?.dutiesByIndex || new Map()) ).to.deep.equal( { - // Since the ZERO_HASH won't pass the isAggregator test - [index]: {duty, selectionProof: signedAttSelectionProof, isAggregator: false}, + // Since the ZERO_HASH won't pass the isAggregator test, selectionProof is null + [index]: {duty, selectionProof: null}, }, "Wrong dutiesService.attesters Map at next epoch" ); expect(dutiesService.getDutiesAtSlot(slot)).to.deep.equal( - [{duty, selectionProof: signedAttSelectionProof, isAggregator: false}], + [{duty, selectionProof: null}], "Wrong getAttestersAtSlot()" ); @@ -170,13 +165,13 @@ describe("AttestationDutiesService", function () { // first confirm duties for this and next epoch should be persisted expect(Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(0)?.dutiesByIndex || new Map())).to.deep.equal( { - 4: {duty: duty, selectionProof: signedAttSelectionProof, isAggregator: false}, + 4: {duty: duty, selectionProof: null}, }, "Wrong dutiesService.attesters Map at current epoch" ); expect(Object.fromEntries(dutiesService["dutiesByIndexByEpoch"].get(1)?.dutiesByIndex || new Map())).to.deep.equal( { - 4: {duty: duty, selectionProof: signedAttSelectionProof, isAggregator: false}, + 4: {duty: duty, selectionProof: null}, }, "Wrong dutiesService.attesters Map at current epoch" ); From 2ad316e12ae478c14810467d67237d2f53acae7d Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Thu, 6 Apr 2023 13:41:23 +0200 Subject: [PATCH 04/11] Implement distributed sync committee aggregation selection --- .../validator/src/services/syncCommittee.ts | 101 +++++++++++++++++- .../src/services/syncCommitteeDuties.ts | 31 ++++-- packages/validator/src/validator.ts | 5 +- 3 files changed, 126 insertions(+), 11 deletions(-) diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index 362fe78ac655..0c7604183726 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -1,8 +1,8 @@ import {ChainForkConfig} from "@lodestar/config"; -import {Slot, CommitteeIndex, altair, Root} from "@lodestar/types"; +import {Slot, CommitteeIndex, altair, Root, BLSSignature} from "@lodestar/types"; import {sleep} from "@lodestar/utils"; -import {computeEpochAtSlot} from "@lodestar/state-transition"; -import {Api, ApiError} from "@lodestar/api"; +import {computeEpochAtSlot, isSyncCommitteeAggregator} from "@lodestar/state-transition"; +import {Api, ApiError, routes} from "@lodestar/api"; import {IClock, LoggerVc} from "../util/index.js"; import {PubkeyHex} from "../types.js"; import {Metrics} from "../metrics.js"; @@ -14,6 +14,7 @@ import {ValidatorEventEmitter} from "./emitter.js"; type SyncCommitteeServiceOpts = { scAfterBlockDelaySlotFraction?: number; + distributedAggregationSelection?: boolean; }; /** @@ -33,7 +34,9 @@ export class SyncCommitteeService { private readonly metrics: Metrics | null, private readonly opts?: SyncCommitteeServiceOpts ) { - this.dutiesService = new SyncCommitteeDutiesService(config, logger, api, clock, validatorStore, metrics); + this.dutiesService = new SyncCommitteeDutiesService(config, logger, api, clock, validatorStore, metrics, { + distributedAggregationSelection: opts?.distributedAggregationSelection, + }); // At most every slot, check existing duties from SyncCommitteeDutiesService and run tasks clock.runEverySlot(this.runSyncCommitteeTasks); @@ -56,6 +59,18 @@ export class SyncCommitteeService { return; } + if (this.opts?.distributedAggregationSelection) { + // Validator in distributed cluster only has a key share, not the full private key. + // The partial selection proofs must be exchanged for combined selection proofs by + // calling submitSyncCommitteeSelections on the distributed validator middleware client. + // This will run in parallel to other sync committee tasks but must be finished before starting + // sync committee contributions as it is required to correctly determine if validator is aggregator + // and to produce a ContributionAndProof that can be threshold aggregated by the middleware client. + this.runDistributedAggregationSelectionTasks(dutiesAtSlot, slot, signal).catch((e) => + this.logger.error("Error on sync committee aggregation selection", {slot}, e) + ); + } + // unlike Attestation, SyncCommitteeSignature could be published asap // especially with lodestar, it's very busy at 1/3 of slot // see https://github.com/ChainSafe/lodestar/issues/4608 @@ -214,4 +229,82 @@ export class SyncCommitteeService { } } } + + /** + * Performs additional sync committee contribution tasks required if validator is part of distributed cluster + * + * 1. Exchange partial for combined selection proofs + * 2. Determine validators that should produce sync committee contribution + * 3. Mutate duty objects to set selection proofs for aggregators + * + * See https://docs.google.com/document/d/1q9jOTPcYQa-3L8luRvQJ-M0eegtba4Nmon3dpO79TMk/mobilebasic + */ + private async runDistributedAggregationSelectionTasks( + duties: SyncDutyAndProofs[], + slot: number, + signal: AbortSignal + ): Promise { + const partialSelections: routes.validator.SyncCommitteeSelection[] = []; + + for (const {duty, selectionProofs} of duties) { + const validatorSelections: routes.validator.SyncCommitteeSelection[] = selectionProofs.map( + ({subcommitteeIndex, partialSelectionProof}) => ({ + validatorIndex: duty.validatorIndex, + slot, + subcommitteeIndex, + selectionProof: partialSelectionProof as BLSSignature, + }) + ); + partialSelections.push(...validatorSelections); + } + + this.logger.debug("Submitting partial sync committee selection proofs", {slot, count: partialSelections.length}); + + const res = await Promise.race([ + this.api.validator.submitSyncCommitteeSelections(partialSelections), + // Exit sync committee contributions flow if there is no response after 2/3 of slot. + // This is in contrast to attestations aggregations flow which is already exited at 1/3 of the slot + // because for sync committee is not required to resubscribe to subnets as beacon node will assume + // validator always aggregates. This allows us to wait until we have to produce sync committee contributions. + // Note that the sync committee contributions flow is not explicitly exited but rather will be skipped + // due to the fact that calculation of `is_sync_committee_aggregator` in duties service is not done + // and selectionProof is set to null, meaning no validator will be considered an aggregator. + sleep(this.clock.msToSlot(slot + 2 / 3), signal), + ]); + + if (!res) { + throw new Error("No response after 2/3 of slot"); + } + ApiError.assert(res); + + const combinedSelections = res.response.data; + this.logger.debug("Received combined sync committee selection proofs", {slot, count: combinedSelections.length}); + + for (const dutyAndProofs of duties) { + const {validatorIndex, subnets} = dutyAndProofs.duty; + + for (const subnet of subnets) { + const logCtxValidator = {slot, index: subnet, validatorIndex}; + + const combinedSelection = combinedSelections.find( + (s) => s.validatorIndex === validatorIndex && s.slot === slot && s.subcommitteeIndex === subnet + ); + + if (!combinedSelection) { + this.logger.warn("Did not receive combined sync committee selection proof", logCtxValidator); + continue; + } + + const isAggregator = isSyncCommitteeAggregator(combinedSelection.selectionProof); + + if (isAggregator) { + const selectionProofObject = dutyAndProofs.selectionProofs.find((p) => p.subcommitteeIndex === subnet); + if (selectionProofObject) { + // Update selection proof by mutating proof objects in duty object + selectionProofObject.selectionProof = combinedSelection.selectionProof; + } + } + } + } + } } diff --git a/packages/validator/src/services/syncCommitteeDuties.ts b/packages/validator/src/services/syncCommitteeDuties.ts index 9e9df14d9a41..0632996a432f 100644 --- a/packages/validator/src/services/syncCommitteeDuties.ts +++ b/packages/validator/src/services/syncCommitteeDuties.ts @@ -42,6 +42,8 @@ export type SyncDutySubnet = { export type SyncSelectionProof = { /** This value is only set to not null if the proof indicates that the validator is an aggregator. */ selectionProof: BLSSignature | null; + /** This value will only be set if validator is part of distributed cluster and only has a key share */ + partialSelectionProof?: BLSSignature; subcommitteeIndex: number; }; @@ -60,6 +62,10 @@ export type SyncDutyAndProofs = { // To assist with readability type DutyAtPeriod = {duty: SyncDutySubnet}; +type SyncCommitteeDutiesServiceOpts = { + distributedAggregationSelection?: boolean; +}; + /** * Validators are part of a static long (~27h) sync committee, and part of static subnets. * However, the isAggregator role changes per slot. @@ -74,7 +80,8 @@ export class SyncCommitteeDutiesService { private readonly api: Api, clock: IClock, private readonly validatorStore: ValidatorStore, - metrics: Metrics | null + metrics: Metrics | null, + private readonly opts?: SyncCommitteeDutiesServiceOpts ) { // Running this task every epoch is safe since a re-org of many epochs is very unlikely // TODO: If the re-org event is reliable consider re-running then @@ -285,11 +292,23 @@ export class SyncCommitteeDutiesService { const dutiesAndProofs: SyncSelectionProof[] = []; for (const subnet of duty.subnets) { const selectionProof = await this.validatorStore.signSyncCommitteeSelectionProof(duty.pubkey, slot, subnet); - dutiesAndProofs.push({ - // selectionProof === null is used to check if is aggregator - selectionProof: isSyncCommitteeAggregator(selectionProof) ? selectionProof : null, - subcommitteeIndex: subnet, - }); + if (this.opts?.distributedAggregationSelection) { + // Validator in distributed cluster only has a key share, not the full private key. + // Passing a partial selection proof to `is_sync_committee_aggregator` would produce incorrect result. + // Sync committee service will exchange partial for combined selection proofs retrieved from + // distributed validator middleware client and determine aggregators at beginning of every slot. + dutiesAndProofs.push({ + selectionProof: null, + partialSelectionProof: selectionProof, + subcommitteeIndex: subnet, + }); + } else { + dutiesAndProofs.push({ + // selectionProof === null is used to check if is aggregator + selectionProof: isSyncCommitteeAggregator(selectionProof) ? selectionProof : null, + subcommitteeIndex: subnet, + }); + } } return dutiesAndProofs; } diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index 84c688cdb45f..4700ad2a9dda 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -142,7 +142,10 @@ export class Validator { emitter, chainHeaderTracker, metrics, - {scAfterBlockDelaySlotFraction: opts.scAfterBlockDelaySlotFraction} + { + scAfterBlockDelaySlotFraction: opts.scAfterBlockDelaySlotFraction, + distributedAggregationSelection: opts.distributed, + } ); this.config = config; From cd181bf928eb5e9929345399f9a30db04fef7b4c Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 7 Apr 2023 18:14:18 +0200 Subject: [PATCH 05/11] Improve err message if not resolved after time of slot --- packages/validator/src/services/attestation.ts | 2 +- packages/validator/src/services/syncCommittee.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 26e853c945b5..0a6a4b51707e 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -328,7 +328,7 @@ export class AttestationService { ]); if (!res) { - throw new Error("No response after 1/3 of slot"); + throw new Error("submitBeaconCommitteeSelections did not resolve after 1/3 of slot"); } ApiError.assert(res); diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index 0c7604183726..29bf7d3cd53f 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -273,7 +273,7 @@ export class SyncCommitteeService { ]); if (!res) { - throw new Error("No response after 2/3 of slot"); + throw new Error("submitSyncCommitteeSelections did not resolve after 2/3 of slot"); } ApiError.assert(res); From d76bf7b7f601ae409e5587ed8ecb61d31072deac Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 7 Apr 2023 18:16:57 +0200 Subject: [PATCH 06/11] Log submit selections error if thrown after cutoff slot time --- packages/validator/src/services/attestation.ts | 4 +++- packages/validator/src/services/syncCommittee.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 0a6a4b51707e..cbd95021d5c1 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -318,7 +318,9 @@ export class AttestationService { this.logger.debug("Submitting partial beacon committee selection proofs", {slot, count: partialSelections.length}); const res = await Promise.race([ - this.api.validator.submitBeaconCommitteeSelections(partialSelections), + this.api.validator + .submitBeaconCommitteeSelections(partialSelections) + .catch((e) => this.logger.error("Error on submitBeaconCommitteeSelections", {slot}, e)), // Exit attestation aggregation flow if there is no response after 1/3 of slot as // beacon node would likely not have enough time to prepare an aggregate attestation. // Note that the aggregations flow is not explicitly exited but rather will be skipped diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index 29bf7d3cd53f..446b55fd361c 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -261,7 +261,9 @@ export class SyncCommitteeService { this.logger.debug("Submitting partial sync committee selection proofs", {slot, count: partialSelections.length}); const res = await Promise.race([ - this.api.validator.submitSyncCommitteeSelections(partialSelections), + this.api.validator + .submitSyncCommitteeSelections(partialSelections) + .catch((e) => this.logger.error("Error on submitSyncCommitteeSelections", {slot}, e)), // Exit sync committee contributions flow if there is no response after 2/3 of slot. // This is in contrast to attestations aggregations flow which is already exited at 1/3 of the slot // because for sync committee is not required to resubscribe to subnets as beacon node will assume From 5e6cd97d37585cd116f8fabf436cfc542f3f9445 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 14 Apr 2023 14:08:27 +0200 Subject: [PATCH 07/11] Throw prepareBeaconCommitteeSubnet errors to log in top-level catch --- packages/validator/src/services/attestation.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index cbd95021d5c1..42396504acbc 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -371,15 +371,14 @@ export class AttestationService { // If there are any subscriptions with aggregators, push them out to the beacon node. if (beaconCommitteeSubscriptions.length > 0) { - try { - ApiError.assert(await this.api.validator.prepareBeaconCommitteeSubnet(beaconCommitteeSubscriptions)); - this.logger.debug("Resubscribed validators as aggregators on beacon committee subnets", { - slot, - count: beaconCommitteeSubscriptions.length, - }); - } catch (e) { - this.logger.error("Failed to resubscribe to beacon committee subnets", {slot}, e as Error); - } + ApiError.assert( + await this.api.validator.prepareBeaconCommitteeSubnet(beaconCommitteeSubscriptions), + "Failed to resubscribe to beacon committee subnets" + ); + this.logger.debug("Resubscribed validators as aggregators on beacon committee subnets", { + slot, + count: beaconCommitteeSubscriptions.length, + }); } } } From 47b734d6e950c71fc6c5998f884674f33b1e6a46 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 14 Apr 2023 16:04:44 +0200 Subject: [PATCH 08/11] Add unit tests for distributed attestation aggregation selection --- .../test/unit/services/attestation.test.ts | 251 +++++++++++------- 1 file changed, 152 insertions(+), 99 deletions(-) diff --git a/packages/validator/test/unit/services/attestation.test.ts b/packages/validator/test/unit/services/attestation.test.ts index a53314dd359e..754b9a133ff7 100644 --- a/packages/validator/test/unit/services/attestation.test.ts +++ b/packages/validator/test/unit/services/attestation.test.ts @@ -3,7 +3,7 @@ import sinon from "sinon"; import bls from "@chainsafe/bls"; import {toHexString} from "@chainsafe/ssz"; import {ssz} from "@lodestar/types"; -import {HttpStatusCode} from "@lodestar/api"; +import {HttpStatusCode, routes} from "@lodestar/api"; import {AttestationService, AttestationServiceOpts} from "../../../src/services/attestation.js"; import {AttDutyAndProof} from "../../../src/services/attestationDuties.js"; import {ValidatorStore} from "../../../src/services/validatorStore.js"; @@ -42,104 +42,157 @@ describe("AttestationService", function () { sandbox.resetHistory(); }); - context("With attestation grouping enabled", () => { - const opts: AttestationServiceOpts = {disableAttestationGrouping: false}; - - it("Should produce, sign, and publish an attestation + aggregate", async () => { - await testAttestationTasks(opts); - }); - }); - - context("With attestation grouping disabled", () => { - const opts: AttestationServiceOpts = {disableAttestationGrouping: true}; - - it("Should produce, sign, and publish an attestation + aggregate", async () => { - await testAttestationTasks(opts); - }); - }); - - async function testAttestationTasks(opts?: AttestationServiceOpts): Promise { - const clock = new ClockMock(); - const attestationService = new AttestationService( - loggerVc, - api, - clock, - validatorStore, - emitter, - chainHeadTracker, - null, - opts - ); - - const attestation = ssz.phase0.Attestation.defaultValue(); - const aggregate = ssz.phase0.SignedAggregateAndProof.defaultValue(); - const duties: AttDutyAndProof[] = [ - { - duty: { - slot: 0, - committeeIndex: attestation.data.index, - committeeLength: 120, - committeesAtSlot: 120, - validatorCommitteeIndex: 1, - validatorIndex: 0, - pubkey: pubkeys[0], - }, - selectionProof: ZERO_HASH, - }, - ]; - - // Return empty replies to duties service - api.beacon.getStateValidators.resolves({ - response: {executionOptimistic: false, data: []}, - ok: true, - status: HttpStatusCode.OK, - }); - api.validator.getAttesterDuties.resolves({ - response: {dependentRoot: ZERO_HASH_HEX, executionOptimistic: false, data: []}, - ok: true, - status: HttpStatusCode.OK, - }); - - // Mock duties service to return some duties directly - attestationService["dutiesService"].getDutiesAtSlot = sinon.stub().returns(duties); - - // Mock beacon's attestation and aggregates endpoints - - api.validator.produceAttestationData.resolves({ - response: {data: attestation.data}, - ok: true, - status: HttpStatusCode.OK, - }); - api.validator.getAggregatedAttestation.resolves({ - response: {data: attestation}, - ok: true, - status: HttpStatusCode.OK, + const testContexts: [string, AttestationServiceOpts][] = [ + ["With default configuration", {}], + ["With attestation grouping disabled", {disableAttestationGrouping: true}], + ["With distributed aggregation selection enabled", {distributedAggregationSelection: true}], + ]; + + for (const [title, opts] of testContexts) { + context(title, () => { + it("Should produce, sign, and publish an attestation + aggregate", async () => { + const clock = new ClockMock(); + const attestationService = new AttestationService( + loggerVc, + api, + clock, + validatorStore, + emitter, + chainHeadTracker, + null, + opts + ); + + const attestation = ssz.phase0.Attestation.defaultValue(); + const aggregate = ssz.phase0.SignedAggregateAndProof.defaultValue(); + const duties: AttDutyAndProof[] = [ + { + duty: { + slot: 0, + committeeIndex: attestation.data.index, + committeeLength: 120, + committeesAtSlot: 120, + validatorCommitteeIndex: 1, + validatorIndex: 0, + pubkey: pubkeys[0], + }, + selectionProof: opts.distributedAggregationSelection ? null : ZERO_HASH, + partialSelectionProof: opts.distributedAggregationSelection ? ZERO_HASH : undefined, + }, + ]; + + // Return empty replies to duties service + api.beacon.getStateValidators.resolves({ + response: {executionOptimistic: false, data: []}, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.getAttesterDuties.resolves({ + response: {dependentRoot: ZERO_HASH_HEX, executionOptimistic: false, data: []}, + ok: true, + status: HttpStatusCode.OK, + }); + + // Mock duties service to return some duties directly + attestationService["dutiesService"].getDutiesAtSlot = sinon.stub().returns(duties); + + // Mock beacon's attestation and aggregates endpoints + + api.validator.produceAttestationData.resolves({ + response: {data: attestation.data}, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.getAggregatedAttestation.resolves({ + response: {data: attestation}, + ok: true, + status: HttpStatusCode.OK, + }); + api.beacon.submitPoolAttestations.resolves({ + response: undefined, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.publishAggregateAndProofs.resolves({ + response: undefined, + ok: true, + status: HttpStatusCode.OK, + }); + + if (opts.distributedAggregationSelection) { + // Mock distributed validator middleware client selections endpoint + // and return a selection proof that passes `is_aggregator` test + api.validator.submitBeaconCommitteeSelections.resolves({ + response: {data: [{validatorIndex: 0, slot: 0, selectionProof: Buffer.alloc(1, 0x10)}]}, + ok: true, + status: HttpStatusCode.OK, + }); + // Accept all subscriptions + api.validator.prepareBeaconCommitteeSubnet.resolves({ + response: undefined, + ok: true, + status: HttpStatusCode.OK, + }); + } + + // Mock signing service + validatorStore.signAttestation.resolves(attestation); + validatorStore.signAggregateAndProof.resolves(aggregate); + + // Trigger clock onSlot for slot 0 + await clock.tickSlotFns(0, controller.signal); + + if (opts.distributedAggregationSelection) { + // Must submit partial beacon committee selection proof based on duty + const selection: routes.validator.BeaconCommitteeSelection = { + validatorIndex: 0, + slot: 0, + selectionProof: ZERO_HASH, + }; + expect(api.validator.submitBeaconCommitteeSelections.callCount).to.equal( + 1, + "submitBeaconCommitteeSelections() must be called once" + ); + expect(api.validator.submitBeaconCommitteeSelections.getCall(0).args).to.deep.equal( + [[selection]], // 1 arg, = selection[] + "wrong submitBeaconCommitteeSelections() args" + ); + + // Must resubscribe validator as aggregator on beacon committee subnet + const subscription: routes.validator.BeaconCommitteeSubscription = { + validatorIndex: 0, + committeeIndex: 0, + committeesAtSlot: 120, + slot: 0, + isAggregator: true, + }; + expect(api.validator.prepareBeaconCommitteeSubnet.callCount).to.equal( + 1, + "prepareBeaconCommitteeSubnet() must be called once" + ); + expect(api.validator.prepareBeaconCommitteeSubnet.getCall(0).args).to.deep.equal( + [[subscription]], // 1 arg, = subscription[] + "wrong prepareBeaconCommitteeSubnet() args" + ); + } + + // Must submit the attestation received through produceAttestationData() + expect(api.beacon.submitPoolAttestations.callCount).to.equal(1, "submitAttestations() must be called once"); + expect(api.beacon.submitPoolAttestations.getCall(0).args).to.deep.equal( + [[attestation]], // 1 arg, = attestation[] + "wrong submitAttestations() args" + ); + + // Must submit the aggregate received through getAggregatedAttestation() then createAndSignAggregateAndProof() + expect(api.validator.publishAggregateAndProofs.callCount).to.equal( + 1, + "publishAggregateAndProofs() must be called once" + ); + expect(api.validator.publishAggregateAndProofs.getCall(0).args).to.deep.equal( + [[aggregate]], // 1 arg, = aggregate[] + "wrong publishAggregateAndProofs() args" + ); + }); }); - api.beacon.submitPoolAttestations.resolves(); - api.validator.publishAggregateAndProofs.resolves(); - - // Mock signing service - validatorStore.signAttestation.resolves(attestation); - validatorStore.signAggregateAndProof.resolves(aggregate); - - // Trigger clock onSlot for slot 0 - await clock.tickSlotFns(0, controller.signal); - - // Must submit the attestation received through produceAttestationData() - expect(api.beacon.submitPoolAttestations.callCount).to.equal(1, "submitAttestations() must be called once"); - expect(api.beacon.submitPoolAttestations.getCall(0).args).to.deep.equal( - [[attestation]], // 1 arg, = attestation[] - "wrong submitAttestations() args" - ); - - // Must submit the aggregate received through getAggregatedAttestation() then createAndSignAggregateAndProof() - expect(api.validator.publishAggregateAndProofs.callCount).to.equal( - 1, - "publishAggregateAndProofs() must be called once" - ); - expect(api.validator.publishAggregateAndProofs.getCall(0).args).to.deep.equal( - [[aggregate]], // 1 arg, = aggregate[] - "wrong publishAggregateAndProofs() args" - ); } }); From 78155a7a61cbae9f0bc12ba53dd018dce3c4d8e7 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 14 Apr 2023 16:05:17 +0200 Subject: [PATCH 09/11] Add unit tests for distributed sync committee aggregation selection --- .../validator/src/services/syncCommittee.ts | 2 +- .../test/unit/services/syncCommittee.test.ts | 223 +++++++++++------- 2 files changed, 141 insertions(+), 84 deletions(-) diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index 446b55fd361c..b522d3cc6c35 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -12,7 +12,7 @@ import {groupSyncDutiesBySubcommitteeIndex, SubcommitteeDuty} from "./utils.js"; import {ChainHeaderTracker} from "./chainHeaderTracker.js"; import {ValidatorEventEmitter} from "./emitter.js"; -type SyncCommitteeServiceOpts = { +export type SyncCommitteeServiceOpts = { scAfterBlockDelaySlotFraction?: number; distributedAggregationSelection?: boolean; }; diff --git a/packages/validator/test/unit/services/syncCommittee.test.ts b/packages/validator/test/unit/services/syncCommittee.test.ts index c7815570d393..9316f11eb483 100644 --- a/packages/validator/test/unit/services/syncCommittee.test.ts +++ b/packages/validator/test/unit/services/syncCommittee.test.ts @@ -5,8 +5,8 @@ import {toHexString} from "@chainsafe/ssz"; import {createChainForkConfig} from "@lodestar/config"; import {config as mainnetConfig} from "@lodestar/config/default"; import {ssz} from "@lodestar/types"; -import {HttpStatusCode} from "@lodestar/api"; -import {SyncCommitteeService} from "../../../src/services/syncCommittee.js"; +import {HttpStatusCode, routes} from "@lodestar/api"; +import {SyncCommitteeService, SyncCommitteeServiceOpts} from "../../../src/services/syncCommittee.js"; import {SyncDutyAndProofs} from "../../../src/services/syncCommitteeDuties.js"; import {ValidatorStore} from "../../../src/services/validatorStore.js"; import {getApiClientStub} from "../../utils/apiStub.js"; @@ -47,87 +47,144 @@ describe("SyncCommitteeService", function () { let controller: AbortController; // To stop clock beforeEach(() => (controller = new AbortController())); - afterEach(() => controller.abort()); - - it("Should produce, sign, and publish a sync committee + contribution", async () => { - const clock = new ClockMock(); - const syncCommitteeService = new SyncCommitteeService( - config, - loggerVc, - api, - clock, - validatorStore, - emitter, - chainHeaderTracker, - null - ); - - const beaconBlockRoot = Buffer.alloc(32, 0x4d); - const syncCommitteeSignature = ssz.altair.SyncCommitteeMessage.defaultValue(); - const contribution = ssz.altair.SyncCommitteeContribution.defaultValue(); - const contributionAndProof = ssz.altair.SignedContributionAndProof.defaultValue(); - const duties: SyncDutyAndProofs[] = [ - { - duty: { - pubkey: toHexString(pubkeys[0]), - validatorIndex: 0, - subnets: [0], - }, - selectionProofs: [{selectionProof: ZERO_HASH, subcommitteeIndex: 0}], - }, - ]; - - // Return empty replies to duties service - api.beacon.getStateValidators.resolves({ - response: {data: [], executionOptimistic: false}, - ok: true, - status: HttpStatusCode.OK, - }); - api.validator.getSyncCommitteeDuties.resolves({ - response: {data: [], executionOptimistic: false}, - ok: true, - status: HttpStatusCode.OK, - }); - - // Mock duties service to return some duties directly - syncCommitteeService["dutiesService"].getDutiesAtSlot = sinon.stub().returns(duties); - - // Mock beacon's sync committee and contribution routes + afterEach(() => { + controller.abort(); + sandbox.resetHistory(); + }); - chainHeaderTracker.getCurrentChainHead.returns(beaconBlockRoot); - api.beacon.submitPoolSyncCommitteeSignatures.resolves(); - api.validator.produceSyncCommitteeContribution.resolves({ - response: {data: contribution}, - ok: true, - status: HttpStatusCode.OK, + const testContexts: [string, SyncCommitteeServiceOpts][] = [ + ["With default configuration", {}], + ["With distributed aggregation selection enabled", {distributedAggregationSelection: true}], + ]; + + for (const [title, opts] of testContexts) { + context(title, () => { + it("Should produce, sign, and publish a sync committee + contribution", async () => { + const clock = new ClockMock(); + const syncCommitteeService = new SyncCommitteeService( + config, + loggerVc, + api, + clock, + validatorStore, + emitter, + chainHeaderTracker, + null, + opts + ); + + const beaconBlockRoot = Buffer.alloc(32, 0x4d); + const syncCommitteeSignature = ssz.altair.SyncCommitteeMessage.defaultValue(); + const contribution = ssz.altair.SyncCommitteeContribution.defaultValue(); + const contributionAndProof = ssz.altair.SignedContributionAndProof.defaultValue(); + const duties: SyncDutyAndProofs[] = [ + { + duty: { + pubkey: toHexString(pubkeys[0]), + validatorIndex: 0, + subnets: [0], + }, + selectionProofs: [ + { + selectionProof: opts.distributedAggregationSelection ? null : ZERO_HASH, + partialSelectionProof: opts.distributedAggregationSelection ? ZERO_HASH : undefined, + subcommitteeIndex: 0, + }, + ], + }, + ]; + + // Return empty replies to duties service + api.beacon.getStateValidators.resolves({ + response: {data: [], executionOptimistic: false}, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.getSyncCommitteeDuties.resolves({ + response: {data: [], executionOptimistic: false}, + ok: true, + status: HttpStatusCode.OK, + }); + + // Mock duties service to return some duties directly + syncCommitteeService["dutiesService"].getDutiesAtSlot = sinon.stub().returns(duties); + + // Mock beacon's sync committee and contribution routes + + chainHeaderTracker.getCurrentChainHead.returns(beaconBlockRoot); + api.beacon.submitPoolSyncCommitteeSignatures.resolves({ + response: undefined, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.produceSyncCommitteeContribution.resolves({ + response: {data: contribution}, + ok: true, + status: HttpStatusCode.OK, + }); + api.validator.publishContributionAndProofs.resolves({ + response: undefined, + ok: true, + status: HttpStatusCode.OK, + }); + + if (opts.distributedAggregationSelection) { + // Mock distributed validator middleware client selections endpoint + // and return a selection proof that passes `is_sync_committee_aggregator` test + api.validator.submitSyncCommitteeSelections.resolves({ + response: { + data: [{validatorIndex: 0, slot: 0, subcommitteeIndex: 0, selectionProof: Buffer.alloc(1, 0x19)}], + }, + ok: true, + status: HttpStatusCode.OK, + }); + } + + // Mock signing service + validatorStore.signSyncCommitteeSignature.resolves(syncCommitteeSignature); + validatorStore.signContributionAndProof.resolves(contributionAndProof); + + // Trigger clock onSlot for slot 0 + await clock.tickSlotFns(0, controller.signal); + + if (opts.distributedAggregationSelection) { + // Must submit partial sync committee selection proof based on duty + const selection: routes.validator.SyncCommitteeSelection = { + validatorIndex: 0, + slot: 0, + subcommitteeIndex: 0, + selectionProof: ZERO_HASH, + }; + expect(api.validator.submitSyncCommitteeSelections.callCount).to.equal( + 1, + "submitSyncCommitteeSelections() must be called once" + ); + expect(api.validator.submitSyncCommitteeSelections.getCall(0).args).to.deep.equal( + [[selection]], // 1 arg, = selection[] + "wrong submitSyncCommitteeSelections() args" + ); + } + + // Must submit the signature received through signSyncCommitteeSignature() + expect(api.beacon.submitPoolSyncCommitteeSignatures.callCount).to.equal( + 1, + "submitPoolSyncCommitteeSignatures() must be called once" + ); + expect(api.beacon.submitPoolSyncCommitteeSignatures.getCall(0).args).to.deep.equal( + [[syncCommitteeSignature]], // 1 arg, = syncCommitteeSignature[] + "wrong submitPoolSyncCommitteeSignatures() args" + ); + + // Must submit the aggregate received through produceSyncCommitteeContribution() then signContributionAndProof() + expect(api.validator.publishContributionAndProofs.callCount).to.equal( + 1, + "publishContributionAndProofs() must be called once" + ); + expect(api.validator.publishContributionAndProofs.getCall(0).args).to.deep.equal( + [[contributionAndProof]], // 1 arg, = contributionAndProof[] + "wrong publishContributionAndProofs() args" + ); + }); }); - api.validator.publishContributionAndProofs.resolves(); - - // Mock signing service - validatorStore.signSyncCommitteeSignature.resolves(syncCommitteeSignature); - validatorStore.signContributionAndProof.resolves(contributionAndProof); - - // Trigger clock onSlot for slot 0 - await clock.tickSlotFns(0, controller.signal); - - // Must submit the signature received through signSyncCommitteeSignature() - expect(api.beacon.submitPoolSyncCommitteeSignatures.callCount).to.equal( - 1, - "submitPoolSyncCommitteeSignatures() must be called once" - ); - expect(api.beacon.submitPoolSyncCommitteeSignatures.getCall(0).args).to.deep.equal( - [[syncCommitteeSignature]], // 1 arg, = syncCommitteeSignature[] - "wrong submitPoolSyncCommitteeSignatures() args" - ); - - // Must submit the aggregate received through produceSyncCommitteeContribution() then signContributionAndProof() - expect(api.validator.publishContributionAndProofs.callCount).to.equal( - 1, - "publishContributionAndProofs() must be called once" - ); - expect(api.validator.publishContributionAndProofs.getCall(0).args).to.deep.equal( - [[contributionAndProof]], // 1 arg, = contributionAndProof[] - "wrong publishContributionAndProofs() args" - ); - }); + } }); From c22d59e00b57e5c74c93510a127cd7410daa7c97 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 14 Apr 2023 17:37:36 +0200 Subject: [PATCH 10/11] Improve err message when request to selection endpoints fails --- packages/validator/src/services/attestation.ts | 2 +- packages/validator/src/services/syncCommittee.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 42396504acbc..95824e6e1bf8 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -332,7 +332,7 @@ export class AttestationService { if (!res) { throw new Error("submitBeaconCommitteeSelections did not resolve after 1/3 of slot"); } - ApiError.assert(res); + ApiError.assert(res, "Error receiving combined selection proofs"); const combinedSelections = res.response.data; this.logger.debug("Received combined beacon committee selection proofs", {slot, count: combinedSelections.length}); diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index b522d3cc6c35..fd5ec577691c 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -277,7 +277,7 @@ export class SyncCommitteeService { if (!res) { throw new Error("submitSyncCommitteeSelections did not resolve after 2/3 of slot"); } - ApiError.assert(res); + ApiError.assert(res, "Error receiving combined selection proofs"); const combinedSelections = res.response.data; this.logger.debug("Received combined sync committee selection proofs", {slot, count: combinedSelections.length}); From f9d418437e6f6d3ffcd7d88b264ef2f3cbdc08a1 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 14 Apr 2023 18:08:51 +0200 Subject: [PATCH 11/11] Reference services by class name in comments --- packages/validator/src/services/attestation.ts | 2 +- packages/validator/src/services/attestationDuties.ts | 2 +- packages/validator/src/services/syncCommittee.ts | 2 +- packages/validator/src/services/syncCommitteeDuties.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 95824e6e1bf8..7e9affb65a41 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -324,7 +324,7 @@ export class AttestationService { // Exit attestation aggregation flow if there is no response after 1/3 of slot as // beacon node would likely not have enough time to prepare an aggregate attestation. // Note that the aggregations flow is not explicitly exited but rather will be skipped - // due to the fact that calculation of `is_aggregator` in duties service is not done + // due to the fact that calculation of `is_aggregator` in AttestationDutiesService is not done // and selectionProof is set to null, meaning no validator will be considered an aggregator. sleep(this.clock.msToSlot(slot + 1 / 3), signal), ]); diff --git a/packages/validator/src/services/attestationDuties.ts b/packages/validator/src/services/attestationDuties.ts index 8089634d06e4..916c02f91994 100644 --- a/packages/validator/src/services/attestationDuties.ts +++ b/packages/validator/src/services/attestationDuties.ts @@ -337,7 +337,7 @@ export class AttestationDutiesService { if (this.opts?.distributedAggregationSelection) { // Validator in distributed cluster only has a key share, not the full private key. // Passing a partial selection proof to `is_aggregator` would produce incorrect result. - // Attestation service will exchange partial for combined selection proofs retrieved from + // AttestationService will exchange partial for combined selection proofs retrieved from // distributed validator middleware client and determine aggregators at beginning of every slot. return {duty, selectionProof: null, partialSelectionProof: selectionProof}; } diff --git a/packages/validator/src/services/syncCommittee.ts b/packages/validator/src/services/syncCommittee.ts index fd5ec577691c..adbda3231697 100644 --- a/packages/validator/src/services/syncCommittee.ts +++ b/packages/validator/src/services/syncCommittee.ts @@ -269,7 +269,7 @@ export class SyncCommitteeService { // because for sync committee is not required to resubscribe to subnets as beacon node will assume // validator always aggregates. This allows us to wait until we have to produce sync committee contributions. // Note that the sync committee contributions flow is not explicitly exited but rather will be skipped - // due to the fact that calculation of `is_sync_committee_aggregator` in duties service is not done + // due to the fact that calculation of `is_sync_committee_aggregator` in SyncCommitteeDutiesService is not done // and selectionProof is set to null, meaning no validator will be considered an aggregator. sleep(this.clock.msToSlot(slot + 2 / 3), signal), ]); diff --git a/packages/validator/src/services/syncCommitteeDuties.ts b/packages/validator/src/services/syncCommitteeDuties.ts index 0632996a432f..764cafa0e3e1 100644 --- a/packages/validator/src/services/syncCommitteeDuties.ts +++ b/packages/validator/src/services/syncCommitteeDuties.ts @@ -295,7 +295,7 @@ export class SyncCommitteeDutiesService { if (this.opts?.distributedAggregationSelection) { // Validator in distributed cluster only has a key share, not the full private key. // Passing a partial selection proof to `is_sync_committee_aggregator` would produce incorrect result. - // Sync committee service will exchange partial for combined selection proofs retrieved from + // SyncCommitteeService will exchange partial for combined selection proofs retrieved from // distributed validator middleware client and determine aggregators at beginning of every slot. dutiesAndProofs.push({ selectionProof: null,