Skip to content

Commit

Permalink
fix: use attestation v1 endpoints pre-electra (#7024)
Browse files Browse the repository at this point in the history 
* fix: use attestation v1 endpoints pre-electra

* Adapt block attestations error message with pool apis
  • Loading branch information
@nflaig
nflaig authored Aug 17, 2024
1 parent 55062cd commit 1f75c59
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 32 deletions.
5 changes: 4 additions & 1 deletion packages/beacon-node/src/api/impl/beacon/blocks/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
29 changes: 21 additions & 8 deletions packages/validator/src/services/attestation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -264,18 +267,24 @@ export class AttestationService {
duties: AttDutyAndProof[]
): Promise<void> {
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)) {
return;
}

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);

Expand All @@ -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) {
Expand Down
74 changes: 51 additions & 23 deletions packages/validator/test/unit/services/attestation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<ChainConfig> = {ELECTRA_FORK_EPOCH: 0};

const testContexts: [string, AttestationServiceOpts, Partial<ChainConfig>][] = [
["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,
Expand All @@ -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: {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]});
}
});
});
}
Expand Down

0 comments on commit 1f75c59

Please sign in to comment.