From f2abbff8ae0a1816e8d135a91672d429f97d887c Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Wed, 8 May 2024 10:09:11 +0700 Subject: [PATCH 1/4] feat: attestationPool to group by slot by data root by committee index for electra --- packages/api/src/beacon/routes/validator.ts | 11 +- .../api/test/unit/beacon/testData/events.ts | 98 +- .../test/unit/beacon/testData/validator.ts | 7 +- .../src/api/impl/validator/index.ts | 9 +- .../src/chain/opPools/attestationPool.ts | 75 +- packages/light-client/stats.html | 4842 +++++++++++++++++ .../test/perf/epoch/epochAltair.test.ts | 4 +- .../test/perf/epoch/epochCapella.test.ts | 4 +- .../test/perf/epoch/epochPhase0.test.ts | 4 +- .../processEffectiveBalanceUpdates.test.ts | 3 +- .../perf/epoch/processRegistryUpdates.test.ts | 3 +- .../validator/src/services/attestation.ts | 10 +- .../validator/src/services/validatorStore.ts | 3 +- .../test/unit/services/attestation.test.ts | 3 +- 14 files changed, 4961 insertions(+), 115 deletions(-) create mode 100644 packages/light-client/stats.html diff --git a/packages/api/src/beacon/routes/validator.ts b/packages/api/src/beacon/routes/validator.ts index de5b1e0e30e1..c9a8e52ba35e 100644 --- a/packages/api/src/beacon/routes/validator.ts +++ b/packages/api/src/beacon/routes/validator.ts @@ -338,7 +338,8 @@ export type Api = { */ getAggregatedAttestation( attestationDataRoot: Root, - slot: Slot + slot: Slot, + index: CommitteeIndex ): Promise< ApiClientResponse< {[HttpStatusCode.OK]: {data: allForks.Attestation; version: ForkName}}, @@ -498,7 +499,7 @@ export type ReqTypes = { produceBlindedBlock: {params: {slot: number}; query: {randao_reveal: string; graffiti: string}}; produceAttestationData: {query: {slot: number; committee_index: number}}; produceSyncCommitteeContribution: {query: {slot: number; subcommittee_index: number; beacon_block_root: string}}; - getAggregatedAttestation: {query: {attestation_data_root: string; slot: number}}; + getAggregatedAttestation: {query: {attestation_data_root: string; slot: number; index: number}}; publishAggregateAndProofs: {body: unknown}; publishContributionAndProofs: {body: unknown}; prepareBeaconCommitteeSubnet: {body: unknown}; @@ -647,10 +648,10 @@ export function getReqSerializers(): ReqSerializers { }, getAggregatedAttestation: { - writeReq: (root, slot) => ({query: {attestation_data_root: toHexString(root), slot}}), - parseReq: ({query}) => [fromHexString(query.attestation_data_root), query.slot], + writeReq: (root, slot, index) => ({query: {attestation_data_root: toHexString(root), slot, index}}), + parseReq: ({query}) => [fromHexString(query.attestation_data_root), query.slot, query.index], schema: { - query: {attestation_data_root: Schema.StringRequired, slot: Schema.UintRequired}, + query: {attestation_data_root: Schema.StringRequired, slot: Schema.UintRequired, index: Schema.UintRequired}, }, }, diff --git a/packages/api/test/unit/beacon/testData/events.ts b/packages/api/test/unit/beacon/testData/events.ts index 1ac101f32f4d..a6e546ddd272 100644 --- a/packages/api/test/unit/beacon/testData/events.ts +++ b/packages/api/test/unit/beacon/testData/events.ts @@ -31,18 +31,21 @@ export const eventTestData: EventData = { block: "0x9a2fefd2fdb57f74993c7780ea5b9030d2897b615b89f808011ca5aebed54eaf", executionOptimistic: false, }, - [EventType.attestation]: ssz.phase0.Attestation.fromJson({ - aggregation_bits: "0x01", - signature: - "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", - data: { - slot: "1", - index: "1", - beacon_block_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - source: {epoch: "1", root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, - target: {epoch: "1", root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, - }, - }), + [EventType.attestation]: { + version: ForkName.altair, + data: ssz.phase0.Attestation.fromJson({ + aggregation_bits: "0x01", + signature: + "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505", + data: { + slot: "1", + index: "1", + beacon_block_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + source: {epoch: "1", root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, + target: {epoch: "1", root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2"}, + }, + }), + }, [EventType.voluntaryExit]: ssz.phase0.SignedVoluntaryExit.fromJson({ message: {epoch: "1", validator_index: "1"}, signature: @@ -72,44 +75,47 @@ export const eventTestData: EventData = { "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", }, }), - [EventType.attesterSlashing]: ssz.phase0.AttesterSlashing.fromJson({ - attestation_1: { - attesting_indices: ["0", "1"], - data: { - slot: "0", - index: "0", - beacon_block_root: "0x0000000000000000000000000000000000000000000000000000000000000000", - source: { - epoch: "0", - root: "0x0000000000000000000000000000000000000000000000000000000000000000", - }, - target: { - epoch: "0", - root: "0x0000000000000000000000000000000000000000000000000000000000000000", + [EventType.attesterSlashing]: { + version: ForkName.altair, + data: ssz.phase0.AttesterSlashing.fromJson({ + attestation_1: { + attesting_indices: ["0", "1"], + data: { + slot: "0", + index: "0", + beacon_block_root: "0x0000000000000000000000000000000000000000000000000000000000000000", + source: { + epoch: "0", + root: "0x0000000000000000000000000000000000000000000000000000000000000000", + }, + target: { + epoch: "0", + root: "0x0000000000000000000000000000000000000000000000000000000000000000", + }, }, + signature: + "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", }, - signature: - "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - }, - attestation_2: { - attesting_indices: ["0", "1"], - data: { - slot: "0", - index: "0", - beacon_block_root: "0x0000000000000000000000000000000000000000000000000000000000000000", - source: { - epoch: "0", - root: "0x0000000000000000000000000000000000000000000000000000000000000000", - }, - target: { - epoch: "0", - root: "0x0000000000000000000000000000000000000000000000000000000000000000", + attestation_2: { + attesting_indices: ["0", "1"], + data: { + slot: "0", + index: "0", + beacon_block_root: "0x0000000000000000000000000000000000000000000000000000000000000000", + source: { + epoch: "0", + root: "0x0000000000000000000000000000000000000000000000000000000000000000", + }, + target: { + epoch: "0", + root: "0x0000000000000000000000000000000000000000000000000000000000000000", + }, }, + signature: + "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", }, - signature: - "0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - }, - }), + }), + }, [EventType.blsToExecutionChange]: ssz.capella.SignedBLSToExecutionChange.fromJson({ message: { validator_index: "1", diff --git a/packages/api/test/unit/beacon/testData/validator.ts b/packages/api/test/unit/beacon/testData/validator.ts index 6b410b724e4f..abd2ef45f300 100644 --- a/packages/api/test/unit/beacon/testData/validator.ts +++ b/packages/api/test/unit/beacon/testData/validator.ts @@ -134,8 +134,11 @@ export const testData: GenericServerTestCases = { res: {data: ssz.altair.SyncCommitteeContribution.defaultValue()}, }, getAggregatedAttestation: { - args: [ZERO_HASH, 32000], - res: {data: ssz.phase0.Attestation.defaultValue()}, + args: [ZERO_HASH, 32000, 2], + res: { + data: ssz.phase0.Attestation.defaultValue(), + version: ForkName.altair, + }, }, publishAggregateAndProofs: { args: [[ssz.phase0.SignedAggregateAndProof.defaultValue()]], diff --git a/packages/beacon-node/src/api/impl/validator/index.ts b/packages/beacon-node/src/api/impl/validator/index.ts index 0ae526835c37..3b1486c6415b 100644 --- a/packages/beacon-node/src/api/impl/validator/index.ts +++ b/packages/beacon-node/src/api/impl/validator/index.ts @@ -1063,16 +1063,19 @@ export function getValidatorApi({ }; }, - async getAggregatedAttestation(attestationDataRoot, slot) { + async getAggregatedAttestation(attestationDataRoot, slot, committeeIndex) { notWhileSyncing(); await waitForSlot(slot); // Must never request for a future slot > currentSlot const dataRootHex = toHexString(attestationDataRoot); - const aggregate = chain.attestationPool.getAggregate(slot, dataRootHex); + const aggregate = chain.attestationPool.getAggregate(slot, committeeIndex, dataRootHex); if (!aggregate) { - throw new ApiError(404, `No aggregated attestation for slot=${slot}, dataRoot=${dataRootHex}`); + throw new ApiError( + 404, + `No aggregated attestation for slot=${slot} committeeIndex=${committeeIndex}, dataRoot=${dataRootHex}` + ); } metrics?.production.producedAggregateParticipants.observe(aggregate.aggregationBits.getTrueBitIndexes().length); diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index 38e910753440..a58f9534c5e9 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -1,7 +1,7 @@ import {PointFormat, Signature} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import {BitArray} from "@chainsafe/ssz"; -import {Slot, RootHex, allForks} from "@lodestar/types"; +import {Slot, RootHex, allForks, isElectraAttestation} from "@lodestar/types"; import {MapDef} from "@lodestar/utils"; import {IClock} from "../../util/clock.js"; import {InsertOutcome, OpPoolError, OpPoolErrorCode} from "./types.js"; @@ -36,6 +36,8 @@ type AggregateFast = AggregateFastPhase0 | AggregateFastElectra; /** Hex string of DataRoot `TODO` */ type DataRootHex = string; +type CommitteeIndex = number; + /** * A pool of `Attestation` that is specially designed to store "unaggregated" attestations from * the native aggregation scheme. @@ -60,8 +62,8 @@ type DataRootHex = string; * receives and it can be triggered manually. */ export class AttestationPool { - private readonly attestationByRootBySlot = new MapDef>( - () => new Map() + private readonly attestationByRootBySlot = new MapDef>>( + () => new Map>() ); private lowestPermissibleSlot = 0; @@ -117,14 +119,28 @@ export class AttestationPool { throw new OpPoolError({code: OpPoolErrorCode.REACHED_MAX_PER_SLOT}); } + const committeeIndex = isElectraAttestation(attestation) + ? // this attestation is added to pool after validation + attestation.committeeBits.getSingleTrueBit() + : attestation.data.index; + if (committeeIndex === null) { + // this should not happen because attestation should be validated before reaching this + throw Error(`Invalid attestation slot=${slot} committeeIndex=${committeeIndex}`); + } + // Pre-aggregate the contribution with existing items - const aggregate = aggregateByRoot.get(attDataRootHex); + let aggregateByIndex = aggregateByRoot.get(attDataRootHex); + if (aggregateByIndex === undefined) { + aggregateByIndex = new Map(); + aggregateByRoot.set(attDataRootHex, aggregateByIndex); + } + const aggregate = aggregateByIndex.get(committeeIndex); if (aggregate) { // Aggregate mutating return aggregateAttestationInto(aggregate, attestation); } else { // Create new aggregate - aggregateByRoot.set(attDataRootHex, attestationToAggregate(attestation)); + aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation)); return InsertOutcome.NewData; } } @@ -132,8 +148,8 @@ export class AttestationPool { /** * For validator API to get an aggregate */ - getAggregate(slot: Slot, dataRootHex: RootHex): allForks.Attestation | null { - const aggregate = this.attestationByRootBySlot.get(slot)?.get(dataRootHex); + getAggregate(slot: Slot, committeeIndex: CommitteeIndex, dataRootHex: RootHex): allForks.Attestation | null { + const aggregate = this.attestationByRootBySlot.get(slot)?.get(dataRootHex)?.get(committeeIndex); if (!aggregate) { // TODO: Add metric for missing aggregates return null; @@ -166,8 +182,10 @@ export class AttestationPool { for (const aggregateByRoot of aggregateByRoots) { if (aggregateByRoot) { - for (const aggFast of aggregateByRoot.values()) { - attestations.push(fastToAttestation(aggFast)); + for (const aggFastByIndex of aggregateByRoot.values()) { + for (const aggFast of aggFastByIndex.values()) { + attestations.push(fastToAttestation(aggFast)); + } } } } @@ -180,7 +198,7 @@ export class AttestationPool { // - Insert attestations coming from gossip and API /** - * Aggregate a new contribution into `aggregate` mutating it + * Aggregate a new attestation into `aggregate` mutating it */ function aggregateAttestationInto(aggregate: AggregateFast, attestation: allForks.Attestation): InsertOutcome { const bitIndex = attestation.aggregationBits.getSingleTrueBit(); @@ -190,26 +208,6 @@ function aggregateAttestationInto(aggregate: AggregateFast, attestation: allFork throw Error("Invalid attestation not exactly one bit set"); } - if ("committeeBits" in attestation && !("committeeBits" in aggregate)) { - throw Error("Attempt to aggregate electra attestation into phase0 attestation"); - } - - if (!("committeeBits" in attestation) && "committeeBits" in aggregate) { - throw Error("Attempt to aggregate phase0 attestation into electra attestation"); - } - - if ("committeeBits" in attestation) { - // We assume attestation.committeeBits should already be validated in api and gossip handler and should be non-null - const attestationCommitteeIndex = attestation.committeeBits.getSingleTrueBit(); - const aggregateCommitteeIndex = (aggregate as AggregateFastElectra).committeeBits.getSingleTrueBit(); - - if (attestationCommitteeIndex !== aggregateCommitteeIndex) { - throw Error( - `Committee index mismatched: attestation ${attestationCommitteeIndex} aggregate ${aggregateCommitteeIndex}` - ); - } - } - if (aggregate.aggregationBits.get(bitIndex) === true) { return InsertOutcome.AlreadyKnown; } @@ -226,7 +224,7 @@ function aggregateAttestationInto(aggregate: AggregateFast, attestation: allFork * Format `contribution` into an efficient `aggregate` to add more contributions in with aggregateContributionInto() */ function attestationToAggregate(attestation: allForks.Attestation): AggregateFast { - if ("committeeBits" in attestation) { + if (isElectraAttestation(attestation)) { return { data: attestation.data, // clone because it will be mutated @@ -247,18 +245,5 @@ function attestationToAggregate(attestation: allForks.Attestation): AggregateFas * Unwrap AggregateFast to phase0.Attestation */ function fastToAttestation(aggFast: AggregateFast): allForks.Attestation { - if ("committeeBits" in aggFast) { - return { - data: aggFast.data, - aggregationBits: aggFast.aggregationBits, - committeeBits: aggFast.committeeBits, - signature: aggFast.signature.toBytes(PointFormat.compressed), - }; - } else { - return { - data: aggFast.data, - aggregationBits: aggFast.aggregationBits, - signature: aggFast.signature.toBytes(PointFormat.compressed), - }; - } + return {...aggFast, signature: aggFast.signature.toBytes(PointFormat.compressed)}; } diff --git a/packages/light-client/stats.html b/packages/light-client/stats.html new file mode 100644 index 000000000000..4e62fc91acbd --- /dev/null +++ b/packages/light-client/stats.html @@ -0,0 +1,4842 @@ + + + + + + + + Rollup Visualizer + + + +
+ + + + + diff --git a/packages/state-transition/test/perf/epoch/epochAltair.test.ts b/packages/state-transition/test/perf/epoch/epochAltair.test.ts index 273353d8632b..6c43151cc137 100644 --- a/packages/state-transition/test/perf/epoch/epochAltair.test.ts +++ b/packages/state-transition/test/perf/epoch/epochAltair.test.ts @@ -120,7 +120,7 @@ function benchmarkAltairEpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - altair processRegistryUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processRegistryUpdates(state, cache.value), + fn: (state) => processRegistryUpdates(ForkSeq.altair, state, cache.value), }); // TODO: Needs a better state to test with, current does not include enough actions: 39.985 us/op @@ -141,7 +141,7 @@ function benchmarkAltairEpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - altair processEffectiveBalanceUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processEffectiveBalanceUpdates(state, cache.value), + fn: (state) => processEffectiveBalanceUpdates(ForkSeq.altair, state, cache.value), }); itBench({ diff --git a/packages/state-transition/test/perf/epoch/epochCapella.test.ts b/packages/state-transition/test/perf/epoch/epochCapella.test.ts index eeaf8bfc5400..5b8300df3f18 100644 --- a/packages/state-transition/test/perf/epoch/epochCapella.test.ts +++ b/packages/state-transition/test/perf/epoch/epochCapella.test.ts @@ -99,7 +99,7 @@ function benchmarkAltairEpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - capella processRegistryUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processRegistryUpdates(state, cache.value), + fn: (state) => processRegistryUpdates(ForkSeq.capella, state, cache.value), }); // TODO: Needs a better state to test with, current does not include enough actions: 39.985 us/op @@ -120,7 +120,7 @@ function benchmarkAltairEpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - capella processEffectiveBalanceUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processEffectiveBalanceUpdates(state, cache.value), + fn: (state) => processEffectiveBalanceUpdates(ForkSeq.capella, state, cache.value), }); itBench({ diff --git a/packages/state-transition/test/perf/epoch/epochPhase0.test.ts b/packages/state-transition/test/perf/epoch/epochPhase0.test.ts index 4e43634b1669..411577878102 100644 --- a/packages/state-transition/test/perf/epoch/epochPhase0.test.ts +++ b/packages/state-transition/test/perf/epoch/epochPhase0.test.ts @@ -102,7 +102,7 @@ function benchmarkPhase0EpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - phase0 processRegistryUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processRegistryUpdates(state, cache.value), + fn: (state) => processRegistryUpdates(ForkSeq.phase0, state, cache.value), }); // TODO: Needs a better state to test with, current does not include enough actions: 39.985 us/op @@ -123,7 +123,7 @@ function benchmarkPhase0EpochSteps(stateOg: LazyValue itBench({ id: `${stateId} - phase0 processEffectiveBalanceUpdates`, beforeEach: () => stateOg.value.clone(), - fn: (state) => processEffectiveBalanceUpdates(state, cache.value), + fn: (state) => processEffectiveBalanceUpdates(ForkSeq.phase0, state, cache.value), }); itBench({ diff --git a/packages/state-transition/test/perf/epoch/processEffectiveBalanceUpdates.test.ts b/packages/state-transition/test/perf/epoch/processEffectiveBalanceUpdates.test.ts index 0fb1d448142f..d94daac9e59b 100644 --- a/packages/state-transition/test/perf/epoch/processEffectiveBalanceUpdates.test.ts +++ b/packages/state-transition/test/perf/epoch/processEffectiveBalanceUpdates.test.ts @@ -1,6 +1,7 @@ import {itBench} from "@dapplion/benchmark"; import {ssz} from "@lodestar/types"; import {config} from "@lodestar/config/default"; +import {ForkSeq} from "@lodestar/params"; import {beforeProcessEpoch, CachedBeaconStateAllForks, EpochTransitionCache} from "../../../src/index.js"; import {processEffectiveBalanceUpdates} from "../../../src/epoch/processEffectiveBalanceUpdates.js"; import {numValidators} from "../util.js"; @@ -35,7 +36,7 @@ describe("phase0 processEffectiveBalanceUpdates", () => { minRuns: 5, // Worst case is very slow before: () => getEffectiveBalanceTestData(vc, changeRatio), beforeEach: ({state, cache}) => ({state: state.clone(), cache}), - fn: ({state, cache}) => processEffectiveBalanceUpdates(state, cache), + fn: ({state, cache}) => processEffectiveBalanceUpdates(ForkSeq.phase0, state, cache), }); } }); diff --git a/packages/state-transition/test/perf/epoch/processRegistryUpdates.test.ts b/packages/state-transition/test/perf/epoch/processRegistryUpdates.test.ts index ccfd2405a665..2d57de44f8ee 100644 --- a/packages/state-transition/test/perf/epoch/processRegistryUpdates.test.ts +++ b/packages/state-transition/test/perf/epoch/processRegistryUpdates.test.ts @@ -1,4 +1,5 @@ import {itBench} from "@dapplion/benchmark"; +import {ForkSeq} from "@lodestar/params"; import {beforeProcessEpoch, CachedBeaconStateAllForks, EpochTransitionCache} from "../../../src/index.js"; import {processRegistryUpdates} from "../../../src/epoch/processRegistryUpdates.js"; import {generatePerfTestCachedStatePhase0, numValidators} from "../util.js"; @@ -62,7 +63,7 @@ describe("phase0 processRegistryUpdates", () => { noThreshold: notTrack, before: () => getRegistryUpdatesTestData(vc, lengths), beforeEach: async ({state, cache}) => ({state: state.clone(), cache}), - fn: ({state, cache}) => processRegistryUpdates(state, cache), + fn: ({state, cache}) => processRegistryUpdates(ForkSeq.phase0, state, cache), }); } }); diff --git a/packages/validator/src/services/attestation.ts b/packages/validator/src/services/attestation.ts index c1eeab5f3e53..978653dcd609 100644 --- a/packages/validator/src/services/attestation.ts +++ b/packages/validator/src/services/attestation.ts @@ -128,7 +128,7 @@ export class AttestationService { // Then download, sign and publish a `SignedAggregateAndProof` for each // validator that is elected to aggregate for this `slot` and `committeeIndex`. - await this.produceAndPublishAggregates(attestation, dutiesSameCommittee); + await this.produceAndPublishAggregates(attestation, index, dutiesSameCommittee); } private async runAttestationTasksGrouped( @@ -154,7 +154,7 @@ export class AttestationService { await Promise.all( Array.from(dutiesByCommitteeIndex.entries()).map(([index, dutiesSameCommittee]) => { const attestationData: phase0.AttestationData = {...attestationNoCommittee, index}; - return this.produceAndPublishAggregates(attestationData, dutiesSameCommittee); + return this.produceAndPublishAggregates(attestationData, index, dutiesSameCommittee); }) ); } @@ -247,9 +247,10 @@ export class AttestationService { */ private async produceAndPublishAggregates( attestation: phase0.AttestationData, + committeeIndex: number, duties: AttDutyAndProof[] ): Promise { - const logCtx = {slot: attestation.slot, index: attestation.index}; + const logCtx = {slot: attestation.slot, index: committeeIndex}; // No validator is aggregator, skip if (duties.every(({selectionProof}) => selectionProof === null)) { @@ -259,7 +260,8 @@ export class AttestationService { this.logger.verbose("Aggregating attestations", logCtx); const res = await this.api.validator.getAggregatedAttestation( ssz.phase0.AttestationData.hashTreeRoot(attestation), - attestation.slot + attestation.slot, + committeeIndex ); ApiError.assert(res, "Error producing aggregateAndProofs"); const aggregate = res.response; diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 1d5d73377946..d7a945998f33 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -20,6 +20,7 @@ import { DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF, DOMAIN_APPLICATION_BUILDER, ForkSeq, + MAX_COMMITTEES_PER_SLOT, } from "@lodestar/params"; import { allForks, @@ -528,7 +529,7 @@ export class ValidatorStore { return { aggregationBits: BitArray.fromSingleBit(duty.committeeLength, duty.validatorCommitteeIndex), data: attestationData, - committeeBits: BitArray.fromSingleBit(duty.committeesAtSlot, duty.committeeIndex), + committeeBits: BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, duty.committeeIndex), signature: await this.getSignature(duty.pubkey, signingRoot, signingSlot, signableMessage), } as electra.Attestation; } else { diff --git a/packages/validator/test/unit/services/attestation.test.ts b/packages/validator/test/unit/services/attestation.test.ts index bb983a8073a6..3f35401ab321 100644 --- a/packages/validator/test/unit/services/attestation.test.ts +++ b/packages/validator/test/unit/services/attestation.test.ts @@ -5,6 +5,7 @@ import {ssz} from "@lodestar/types"; import {HttpStatusCode, routes} from "@lodestar/api"; import {createChainForkConfig} from "@lodestar/config"; import {config} from "@lodestar/config/default"; +import {ForkName} from "@lodestar/params"; import {AttestationService, AttestationServiceOpts} from "../../../src/services/attestation.js"; import {AttDutyAndProof} from "../../../src/services/attestationDuties.js"; import {ValidatorStore} from "../../../src/services/validatorStore.js"; @@ -109,7 +110,7 @@ describe("AttestationService", function () { status: HttpStatusCode.OK, }); api.validator.getAggregatedAttestation.mockResolvedValue({ - response: {data: attestation}, + response: {version: ForkName.phase0, data: attestation}, ok: true, status: HttpStatusCode.OK, }); From 2517b1d7f308cb819a19bcccdce400d45ec1b0c5 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Wed, 8 May 2024 10:36:05 +0700 Subject: [PATCH 2/4] fix: gossip validation and assert.notNull() util --- .../src/chain/opPools/aggregatedAttestationPool.ts | 8 +++----- .../beacon-node/src/chain/opPools/attestationPool.ts | 12 ++++-------- .../src/chain/validation/aggregateAndProof.ts | 2 +- .../beacon-node/src/chain/validation/attestation.ts | 4 ++-- packages/utils/src/assert.ts | 12 ++++++++++++ packages/utils/test/unit/assert.test.ts | 10 ++++++++++ 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index d6cceb9572ae..fde1ac88ace0 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -30,7 +30,7 @@ import { getBlockRootAtSlot, } from "@lodestar/state-transition"; import {IForkChoice, EpochDifference} from "@lodestar/fork-choice"; -import {toHex, MapDef} from "@lodestar/utils"; +import {toHex, MapDef, assert} from "@lodestar/utils"; import {intersectUint8Arrays, IntersectResult} from "../../util/bitArray.js"; import {pruneBySlot, signatureFromBytesNoCheck} from "./utils.js"; import {InsertOutcome} from "./types.js"; @@ -141,10 +141,8 @@ export class AggregatedAttestationPool { ? // this attestation is added to pool after validation attestation.committeeBits.getSingleTrueBit() : attestation.data.index; - if (committeeIndex === null) { - // this should not happen because attestation should be validated before reaching this - throw Error(`Invalid attestation slot=${slot} committeeIndex=${committeeIndex}`); - } + // this should not happen because attestation should be validated before reaching this + assert.notNull(committeeIndex, "Committee index should not be null in aggregated attestation pool"); let attestationGroup = attestationGroupByIndex.get(committeeIndex); if (!attestationGroup) { attestationGroup = new MatchingDataAttestationGroup(committee, attestation.data); diff --git a/packages/beacon-node/src/chain/opPools/attestationPool.ts b/packages/beacon-node/src/chain/opPools/attestationPool.ts index a58f9534c5e9..7d6cca30056c 100644 --- a/packages/beacon-node/src/chain/opPools/attestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/attestationPool.ts @@ -2,7 +2,7 @@ import {PointFormat, Signature} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import {BitArray} from "@chainsafe/ssz"; import {Slot, RootHex, allForks, isElectraAttestation} from "@lodestar/types"; -import {MapDef} from "@lodestar/utils"; +import {MapDef, assert} from "@lodestar/utils"; import {IClock} from "../../util/clock.js"; import {InsertOutcome, OpPoolError, OpPoolErrorCode} from "./types.js"; import {pruneBySlot, signatureFromBytesNoCheck} from "./utils.js"; @@ -123,10 +123,8 @@ export class AttestationPool { ? // this attestation is added to pool after validation attestation.committeeBits.getSingleTrueBit() : attestation.data.index; - if (committeeIndex === null) { - // this should not happen because attestation should be validated before reaching this - throw Error(`Invalid attestation slot=${slot} committeeIndex=${committeeIndex}`); - } + // this should not happen because attestation should be validated before reaching this + assert.notNull(committeeIndex, "Committee index should not be null in attestation pool"); // Pre-aggregate the contribution with existing items let aggregateByIndex = aggregateByRoot.get(attDataRootHex); @@ -204,9 +202,7 @@ function aggregateAttestationInto(aggregate: AggregateFast, attestation: allFork const bitIndex = attestation.aggregationBits.getSingleTrueBit(); // Should never happen, attestations are verified against this exact condition before - if (bitIndex === null) { - throw Error("Invalid attestation not exactly one bit set"); - } + assert.notNull(bitIndex, "Invalid attestation in pool, not exactly one bit set"); if (aggregate.aggregationBits.get(bitIndex) === true) { return InsertOutcome.AlreadyKnown; diff --git a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts index 7950466570ce..a1d7135716ce 100644 --- a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts +++ b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts @@ -82,7 +82,7 @@ async function validateAggregateAndProof( throw new AttestationError(GossipAction.REJECT, {code: AttestationErrorCode.NOT_EXACTLY_ONE_COMMITTEE_BIT_SET}); } // [REJECT] aggregate.data.index == 0 - if (attData.index === 0) { + if (attData.index !== 0) { throw new AttestationError(GossipAction.REJECT, {code: AttestationErrorCode.NON_ZERO_ATTESTATION_DATA_INDEX}); } } else { diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index a186b1325396..87e8c2a6b2bd 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -44,7 +44,7 @@ export type AttestationValidationResult = { export type AttestationOrBytes = ApiAttestation | GossipAttestation; /** attestation from api */ -export type ApiAttestation = {attestation: phase0.Attestation; serializedData: null}; // TODO Electra: add new attestation type +export type ApiAttestation = {attestation: phase0.Attestation; serializedData: null}; /** attestation from gossip */ export type GossipAttestation = { @@ -298,7 +298,7 @@ async function validateGossipAttestationNoSignatureCheck( } // [REJECT] aggregate.data.index == 0 - if (attData.index === 0) { + if (attData.index !== 0) { throw new AttestationError(GossipAction.REJECT, {code: AttestationErrorCode.NON_ZERO_ATTESTATION_DATA_INDEX}); } } else { diff --git a/packages/utils/src/assert.ts b/packages/utils/src/assert.ts index aa86161cca44..91612b0e6407 100644 --- a/packages/utils/src/assert.ts +++ b/packages/utils/src/assert.ts @@ -21,6 +21,18 @@ export const assert = { } }, + /** + * Assert not null + * ``` + * actual !== null + * ``` + */ + notNull(actual: T | null, message?: string): asserts actual is T { + if (!(actual !== null)) { + throw new AssertionError(`${message || "Expected value to be not null"}`); + } + }, + /** * Assert less than or equal * ```js diff --git a/packages/utils/test/unit/assert.test.ts b/packages/utils/test/unit/assert.test.ts index 0555bcbd01a0..e45e792a67e9 100644 --- a/packages/utils/test/unit/assert.test.ts +++ b/packages/utils/test/unit/assert.test.ts @@ -20,6 +20,16 @@ describe("assert", () => { }); }); + describe("notNull with custom message", () => { + it("Should not throw error with not null value", () => { + expect(() => assert.notNull(0)).not.toThrow(); + expect(() => assert.notNull("")).not.toThrow(); + }); + it("Should throw with null value", () => { + expect(() => assert.notNull(null, "something must not be null")).toThrow("something must not be null"); + }); + }); + const cases: { op: keyof Omit; args: [number, number]; From 5e4f1ddd45edda30c2c02baab7a2f13e5fa7681f Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Wed, 8 May 2024 10:44:35 +0700 Subject: [PATCH 3/4] fix: remove light-client stats.html --- packages/light-client/stats.html | 4842 ------------------------------ 1 file changed, 4842 deletions(-) delete mode 100644 packages/light-client/stats.html diff --git a/packages/light-client/stats.html b/packages/light-client/stats.html deleted file mode 100644 index 4e62fc91acbd..000000000000 --- a/packages/light-client/stats.html +++ /dev/null @@ -1,4842 +0,0 @@ - - - - - - - - Rollup Visualizer - - - -
- - - - - From 93e84eb8d1c0aef9ea7e2240edff3c260a3fedd4 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Wed, 8 May 2024 17:42:41 +0700 Subject: [PATCH 4/4] fix: lint and check-types --- packages/types/src/electra/types.ts | 2 +- packages/utils/test/unit/assert.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/types/src/electra/types.ts b/packages/types/src/electra/types.ts index ab13f5811f44..4de209b48960 100644 --- a/packages/types/src/electra/types.ts +++ b/packages/types/src/electra/types.ts @@ -48,4 +48,4 @@ export type SignedConsolidation = ValueOf; export type PendingBalanceDeposit = ValueOf; export type PartialWithdrawal = ValueOf; -export type PendingConsolidation = ValueOf; \ No newline at end of file +export type PendingConsolidation = ValueOf; diff --git a/packages/utils/test/unit/assert.test.ts b/packages/utils/test/unit/assert.test.ts index e45e792a67e9..3b413efa11be 100644 --- a/packages/utils/test/unit/assert.test.ts +++ b/packages/utils/test/unit/assert.test.ts @@ -31,7 +31,7 @@ describe("assert", () => { }); const cases: { - op: keyof Omit; + op: keyof Omit; args: [number, number]; ok: boolean; }[] = [