From 635cc12b586697505457ed8f4ed29f49e054bdb8 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Wed, 14 Aug 2024 11:09:54 +0100 Subject: [PATCH 1/2] fix: use attestation v1 endpoints pre-electra --- .../validator/src/services/attestation.ts | 29 ++++++-- .../test/unit/services/attestation.test.ts | 74 +++++++++++++------ 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index 604f4adeb47e..fc43b603c6b2 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -237,8 +237,11 @@ export class AttestationService { ...(this.opts?.disableAttestationGrouping && {index: attestationNoCommittee.index}), }; try { - // TODO Electra: Ensure calling V2 works in pre-electra - (await this.api.beacon.submitPoolAttestationsV2({signedAttestations})).assertOk(); + if (isPostElectra) { + (await this.api.beacon.submitPoolAttestationsV2({signedAttestations})).assertOk(); + } else { + (await this.api.beacon.submitPoolAttestations({signedAttestations})).assertOk(); + } this.logger.info("Published attestations", {...logCtx, count: signedAttestations.length}); this.metrics?.publishedAttestations.inc(signedAttestations.length); } catch (e) { @@ -264,6 +267,7 @@ export class AttestationService { duties: AttDutyAndProof[] ): Promise { const logCtx = {slot: attestation.slot, index: committeeIndex}; + const isPostElectra = this.config.getForkSeq(attestation.slot) >= ForkSeq.electra; // No validator is aggregator, skip if (duties.every(({selectionProof}) => selectionProof === null)) { @@ -271,11 +275,16 @@ export class AttestationService { } this.logger.verbose("Aggregating attestations", logCtx); - const res = await this.api.validator.getAggregatedAttestationV2({ - attestationDataRoot: ssz.phase0.AttestationData.hashTreeRoot(attestation), - slot: attestation.slot, - committeeIndex, - }); + const res = isPostElectra + ? await this.api.validator.getAggregatedAttestationV2({ + attestationDataRoot: ssz.phase0.AttestationData.hashTreeRoot(attestation), + slot: attestation.slot, + committeeIndex, + }) + : await this.api.validator.getAggregatedAttestation({ + attestationDataRoot: ssz.phase0.AttestationData.hashTreeRoot(attestation), + slot: attestation.slot, + }); const aggregate = res.value(); this.metrics?.numParticipantsInAggregate.observe(aggregate.aggregationBits.getTrueBitIndexes().length); @@ -302,7 +311,11 @@ export class AttestationService { if (signedAggregateAndProofs.length > 0) { try { - (await this.api.validator.publishAggregateAndProofsV2({signedAggregateAndProofs})).assertOk(); + if (isPostElectra) { + (await this.api.validator.publishAggregateAndProofsV2({signedAggregateAndProofs})).assertOk(); + } else { + (await this.api.validator.publishAggregateAndProofs({signedAggregateAndProofs})).assertOk(); + } this.logger.info("Published aggregateAndProofs", {...logCtx, count: signedAggregateAndProofs.length}); this.metrics?.publishedAggregates.inc(signedAggregateAndProofs.length); } catch (e) { diff --git a/packages/validator/test/unit/services/attestation.test.ts b/packages/validator/test/unit/services/attestation.test.ts index 591ac51fa05e..356503cbdd09 100644 --- a/packages/validator/test/unit/services/attestation.test.ts +++ b/packages/validator/test/unit/services/attestation.test.ts @@ -3,8 +3,8 @@ import {toHexString} from "@chainsafe/ssz"; import {SecretKey} from "@chainsafe/blst"; import {ssz} from "@lodestar/types"; import {routes} from "@lodestar/api"; -import {createChainForkConfig} from "@lodestar/config"; -import {config} from "@lodestar/config/default"; +import {ChainConfig, createChainForkConfig} from "@lodestar/config"; +import {config as defaultConfig} from "@lodestar/config/default"; import {ForkName} from "@lodestar/params"; import {AttestationService, AttestationServiceOpts} from "../../../src/services/attestation.js"; import {AttDutyAndProof} from "../../../src/services/attestationDuties.js"; @@ -52,16 +52,23 @@ describe("AttestationService", function () { vi.resetAllMocks(); }); - const testContexts: [string, AttestationServiceOpts][] = [ - ["With default configuration", {}], - ["With attestation grouping disabled", {disableAttestationGrouping: true}], - ["With distributed aggregation selection enabled", {distributedAggregationSelection: true}], + // eslint-disable-next-line @typescript-eslint/naming-convention + const electraConfig: Partial = {ELECTRA_FORK_EPOCH: 0}; + + const testContexts: [string, AttestationServiceOpts, Partial][] = [ + ["With default configuration", {}, {}], + ["With default configuration post-electra", {}, electraConfig], + ["With attestation grouping disabled", {disableAttestationGrouping: true}, {}], + ["With attestation grouping disabled post-electra", {disableAttestationGrouping: true}, electraConfig], + ["With distributed aggregation selection enabled", {distributedAggregationSelection: true}, {}], ]; - for (const [title, opts] of testContexts) { + for (const [title, opts, chainConfig] of testContexts) { describe(title, () => { it("Should produce, sign, and publish an attestation + aggregate", async () => { const clock = new ClockMock(); + const config = createChainForkConfig({...defaultConfig, ...chainConfig}); + const isPostElectra = chainConfig.ELECTRA_FORK_EPOCH === 0; const attestationService = new AttestationService( loggerVc, api, @@ -71,12 +78,16 @@ describe("AttestationService", function () { chainHeadTracker, syncingStatusTracker, null, - createChainForkConfig(config), + config, opts ); - const attestation = ssz.phase0.Attestation.defaultValue(); - const aggregate = ssz.phase0.SignedAggregateAndProof.defaultValue(); + const attestation = isPostElectra + ? ssz.electra.Attestation.defaultValue() + : ssz.phase0.Attestation.defaultValue(); + const aggregate = isPostElectra + ? ssz.electra.SignedAggregateAndProof.defaultValue() + : ssz.phase0.SignedAggregateAndProof.defaultValue(); const duties: AttDutyAndProof[] = [ { duty: { @@ -106,12 +117,17 @@ describe("AttestationService", function () { // Mock beacon's attestation and aggregates endpoints api.validator.produceAttestationData.mockResolvedValue(mockApiResponse({data: attestation.data})); - api.validator.getAggregatedAttestationV2.mockResolvedValue( - mockApiResponse({data: attestation, meta: {version: ForkName.phase0}}) - ); - - api.beacon.submitPoolAttestationsV2.mockResolvedValue(mockApiResponse({})); - api.validator.publishAggregateAndProofsV2.mockResolvedValue(mockApiResponse({})); + if (isPostElectra) { + api.validator.getAggregatedAttestationV2.mockResolvedValue( + mockApiResponse({data: attestation, meta: {version: ForkName.electra}}) + ); + api.beacon.submitPoolAttestationsV2.mockResolvedValue(mockApiResponse({})); + api.validator.publishAggregateAndProofsV2.mockResolvedValue(mockApiResponse({})); + } else { + api.validator.getAggregatedAttestation.mockResolvedValue(mockApiResponse({data: attestation})); + api.beacon.submitPoolAttestations.mockResolvedValue(mockApiResponse({})); + api.validator.publishAggregateAndProofs.mockResolvedValue(mockApiResponse({})); + } if (opts.distributedAggregationSelection) { // Mock distributed validator middleware client selections endpoint @@ -152,13 +168,25 @@ describe("AttestationService", function () { expect(api.validator.prepareBeaconCommitteeSubnet).toHaveBeenCalledWith({subscriptions: [subscription]}); } - // Must submit the attestation received through produceAttestationData() - expect(api.beacon.submitPoolAttestationsV2).toHaveBeenCalledOnce(); - expect(api.beacon.submitPoolAttestationsV2).toHaveBeenCalledWith({signedAttestations: [attestation]}); - - // Must submit the aggregate received through getAggregatedAttestationV2() then createAndSignAggregateAndProof() - expect(api.validator.publishAggregateAndProofsV2).toHaveBeenCalledOnce(); - expect(api.validator.publishAggregateAndProofsV2).toHaveBeenCalledWith({signedAggregateAndProofs: [aggregate]}); + if (isPostElectra) { + // Must submit the attestation received through produceAttestationData() + expect(api.beacon.submitPoolAttestationsV2).toHaveBeenCalledOnce(); + expect(api.beacon.submitPoolAttestationsV2).toHaveBeenCalledWith({signedAttestations: [attestation]}); + + // Must submit the aggregate received through getAggregatedAttestationV2() then createAndSignAggregateAndProof() + expect(api.validator.publishAggregateAndProofsV2).toHaveBeenCalledOnce(); + expect(api.validator.publishAggregateAndProofsV2).toHaveBeenCalledWith({ + signedAggregateAndProofs: [aggregate], + }); + } else { + // Must submit the attestation received through produceAttestationData() + expect(api.beacon.submitPoolAttestations).toHaveBeenCalledOnce(); + expect(api.beacon.submitPoolAttestations).toHaveBeenCalledWith({signedAttestations: [attestation]}); + + // Must submit the aggregate received through getAggregatedAttestation() then createAndSignAggregateAndProof() + expect(api.validator.publishAggregateAndProofs).toHaveBeenCalledOnce(); + expect(api.validator.publishAggregateAndProofs).toHaveBeenCalledWith({signedAggregateAndProofs: [aggregate]}); + } }); }); } From 0b8e450e4c410b3e8268da2664b7a8580eacf047 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 16 Aug 2024 11:43:38 +0100 Subject: [PATCH 2/2] Adapt block attestations error message with pool apis --- packages/beacon-node/src/api/impl/beacon/blocks/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts index d89b6c4aaecd..2df571aa3ee7 100644 --- a/packages/beacon-node/src/api/impl/beacon/blocks/index.ts +++ b/packages/beacon-node/src/api/impl/beacon/blocks/index.ts @@ -410,7 +410,10 @@ export function getBeaconBlockApi({ const fork = config.getForkName(block.message.slot); if (isForkPostElectra(fork)) { - throw new ApiError(400, `Use getBlockAttestationsV2 to retrieve electra+ block attestations fork=${fork}`); + throw new ApiError( + 400, + `Use getBlockAttestationsV2 to retrieve block attestations for post-electra fork=${fork}` + ); } return {