From 76ce08ff02e35088150223633b0593ab220b3ced Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 4 May 2024 14:42:57 +0700 Subject: [PATCH 1/6] feat: getAttestationsForBlock() for electra --- .../src/chain/blocks/importBlock.ts | 7 +- .../opPools/aggregatedAttestationPool.ts | 351 +++++++++++++----- .../test/unit/api/impl/events/events.test.ts | 6 +- .../opPools/aggregatedAttestationPool.test.ts | 3 +- 4 files changed, 268 insertions(+), 99 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index ad6d4eb5a186..6c28e020873b 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -1,6 +1,6 @@ import {toHexString} from "@chainsafe/ssz"; import {capella, ssz, allForks, altair} from "@lodestar/types"; -import {ForkName, ForkSeq, INTERVALS_PER_SLOT, MAX_SEED_LOOKAHEAD, SLOTS_PER_EPOCH} from "@lodestar/params"; +import {ForkSeq, INTERVALS_PER_SLOT, MAX_SEED_LOOKAHEAD, SLOTS_PER_EPOCH} from "@lodestar/params"; import { CachedBeaconStateAltair, computeEpochAtSlot, @@ -433,7 +433,10 @@ export async function importBlock( } if (this.emitter.listenerCount(routes.events.EventType.attesterSlashing)) { for (const attesterSlashing of block.message.body.attesterSlashings) { - this.emitter.emit(routes.events.EventType.attesterSlashing, {version: this.config.getForkName(blockSlot), data: attesterSlashing}); + this.emitter.emit(routes.events.EventType.attesterSlashing, { + version: this.config.getForkName(blockSlot), + data: attesterSlashing, + }); } } if (this.emitter.listenerCount(routes.events.EventType.proposerSlashing)) { diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index d56fe294f660..76902b370488 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -1,14 +1,26 @@ import bls from "@chainsafe/bls"; -import {toHexString} from "@chainsafe/ssz"; +import {Signature} from "@chainsafe/bls/types"; +import {BitArray, toHexString} from "@chainsafe/ssz"; import { ForkName, ForkSeq, MAX_ATTESTATIONS, MAX_ATTESTATIONS_ELECTRA, + MAX_COMMITTEES_PER_SLOT, MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH, } from "@lodestar/params"; -import {phase0, Epoch, Slot, ssz, ValidatorIndex, RootHex, allForks, electra} from "@lodestar/types"; +import { + phase0, + Epoch, + Slot, + ssz, + ValidatorIndex, + RootHex, + allForks, + electra, + isElectraAttestation, +} from "@lodestar/types"; import { CachedBeaconStateAllForks, CachedBeaconStatePhase0, @@ -27,13 +39,21 @@ type DataRootHex = string; type CommitteeIndex = number; +// for pre-electra type AttestationWithScore = {attestation: allForks.Attestation; score: number}; +// for electra +type AttestationsConsolidation = { + byCommittee: Map; + attData: phase0.AttestationData; + totalNotSeenCount: number; + score: number; +}; /** - * This function returns not seen participation for a given epoch and committee. + * This function returns not seen participation for a given epoch and slot and committe index. * Return null if all validators are seen or no info to check. */ -type GetNotSeenValidatorsFn = (epoch: Epoch, committee: Uint32Array) => Set | null; +type GetNotSeenValidatorsFn = (epoch: Epoch, slot: Slot, committeeIndex: number) => Set | null; type ValidateAttestationDataFn = (attData: phase0.AttestationData) => boolean; @@ -46,11 +66,15 @@ type ValidateAttestationDataFn = (attData: phase0.AttestationData) => boolean; const MAX_RETAINED_ATTESTATIONS_PER_GROUP = 4; /** - * On mainnet, each slot has 64 committees, and each block has 128 (8 in electra) attestations max so in average + * Pre-electra, each slot has 64 committees, and each block has 128 attestations max so in average * we get 2 attestation per groups. * Starting from Jan 2024, we have a performance issue getting attestations for a block. Based on the - * fact that lot of groups will have only 1 attestation since it's full of participation increase this number + * fact that lot of groups will have only 1 full participation attestation, increase this number * a bit higher than average. This also help decrease number of slots to search for attestations. + * + * For electra, each block has up to 8 aggregated attestations, assuming there are 3 for the "best" + * attestation data, there are still 5 for other attestation data so this constant is still good. + * We should separate to 2 constant based on conditions of different networks */ const MAX_ATTESTATIONS_PER_GROUP = 3; @@ -61,20 +85,21 @@ const MAX_ATTESTATIONS_PER_GROUP = 3; * Note that we want to remove attestations with attesters that were included in the chain. */ export class AggregatedAttestationPool { - private readonly attestationGroupByDataHashByIndexBySlot = new MapDef< + // per electra, different committees could have the same AttData hash + private readonly attestationGroupByIndexByDataHashBySlot = new MapDef< Slot, - Map> - >(() => new Map>()); + Map> + >(() => new Map>()); private lowestPermissibleSlot = 0; /** For metrics to track size of the pool */ getAttestationCount(): {attestationCount: number; attestationDataCount: number} { let attestationCount = 0; let attestationDataCount = 0; - for (const attestationGroupByDataByIndex of this.attestationGroupByDataHashByIndexBySlot.values()) { - for (const attestationGroupByData of attestationGroupByDataByIndex.values()) { - attestationDataCount += attestationGroupByData.size; - for (const attestationGroup of attestationGroupByData.values()) { + for (const attestationGroupByIndexByData of this.attestationGroupByIndexByDataHashBySlot.values()) { + for (const attestationGroupByIndex of attestationGroupByIndexByData.values()) { + attestationDataCount += attestationGroupByIndex.size; + for (const attestationGroup of attestationGroupByIndex.values()) { attestationCount += attestationGroup.getAttestationCount(); } } @@ -96,16 +121,20 @@ export class AggregatedAttestationPool { return InsertOutcome.Old; } - const attestationGroupByDataHashByIndex = this.attestationGroupByDataHashByIndexBySlot.getOrDefault(slot); - let attestationGroupByDataHash = attestationGroupByDataHashByIndex.get(attestation.data.index); - if (!attestationGroupByDataHash) { - attestationGroupByDataHash = new Map(); - attestationGroupByDataHashByIndex.set(attestation.data.index, attestationGroupByDataHash); + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.getOrDefault(slot); + let attestationGroupByIndex = attestationGroupByIndexByDataHash.get(dataRootHex); + if (!attestationGroupByIndex) { + attestationGroupByIndex = new Map(); + attestationGroupByIndexByDataHash.set(dataRootHex, attestationGroupByIndex); } - let attestationGroup = attestationGroupByDataHash.get(dataRootHex); + const committeeIndex = isElectraAttestation(attestation) + ? // this attestation is added to pool after validation + (attestation.committeeBits.getSingleTrueBit() as number) + : attestation.data.index; + let attestationGroup = attestationGroupByIndex.get(committeeIndex); if (!attestationGroup) { attestationGroup = new MatchingDataAttestationGroup(committee, attestation.data); - attestationGroupByDataHash.set(dataRootHex, attestationGroup); + attestationGroupByIndex.set(committeeIndex, attestationGroup); } return attestationGroup.add({ @@ -117,14 +146,25 @@ export class AggregatedAttestationPool { /** Remove attestations which are too old to be included in a block. */ prune(clockSlot: Slot): void { // Only retain SLOTS_PER_EPOCH slots - pruneBySlot(this.attestationGroupByDataHashByIndexBySlot, clockSlot, SLOTS_PER_EPOCH); + pruneBySlot(this.attestationGroupByIndexByDataHashBySlot, clockSlot, SLOTS_PER_EPOCH); this.lowestPermissibleSlot = Math.max(clockSlot - SLOTS_PER_EPOCH, 0); } + getAttestationsForBlock( + fork: ForkName, + forkChoice: IForkChoice, + state: CachedBeaconStateAllForks + ): allForks.Attestation[] { + const forkSeq = ForkSeq[fork]; + return forkSeq < ForkSeq.electra + ? this.getAttestationsForBlockPreElectra(fork, forkChoice, state) + : this.getAttestationsForBlockElectra(fork, forkChoice, state); + } + /** - * Get attestations to be included in a block. Returns $MAX_ATTESTATIONS items + * Get attestations to be included in a block pre-electra. Returns up to $MAX_ATTESTATIONS items */ - getAttestationsForBlock( + getAttestationsForBlockPreElectra( fork: ForkName, forkChoice: IForkChoice, state: CachedBeaconStateAllForks @@ -138,14 +178,14 @@ export class AggregatedAttestationPool { const attestationsByScore: AttestationWithScore[] = []; - const slots = Array.from(this.attestationGroupByDataHashByIndexBySlot.keys()).sort((a, b) => b - a); + const slots = Array.from(this.attestationGroupByIndexByDataHashBySlot.keys()).sort((a, b) => b - a); let minScore = Number.MAX_SAFE_INTEGER; let slotCount = 0; slot: for (const slot of slots) { slotCount++; - const attestationGroupByDataHashByIndex = this.attestationGroupByDataHashByIndexBySlot.get(slot); + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.get(slot); // should not happen - if (!attestationGroupByDataHashByIndex) { + if (!attestationGroupByIndexByDataHash) { throw Error(`No aggregated attestation pool for slot=${slot}`); } @@ -166,35 +206,25 @@ export class AggregatedAttestationPool { } const slotDelta = stateSlot - slot; - const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); - const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; - for (const [committeeIndex, attestationGroupByData] of attestationGroupByDataHashByIndex.entries()) { - // all attestations will be validated against the state in next step so we can get committee from the state - // this is an improvement to save the notSeenValidatorsFn call for the same slot/index instead of the same attestation data - if (committeeIndex > slotCommittees.length) { - // invalid index, should not happen - continue; - } - - const committee = slotCommittees[committeeIndex]; - const notSeenAttestingIndices = notSeenValidatorsFn(epoch, committee); - if (notSeenAttestingIndices === null || notSeenAttestingIndices.size === 0) { - continue; - } + for (const attestationGroupByIndex of attestationGroupByIndexByDataHash.values()) { + for (const [committeeIndex, attestationGroup] of attestationGroupByIndex.entries()) { + const notSeenAttestingIndices = notSeenValidatorsFn(epoch, slot, committeeIndex); + if (notSeenAttestingIndices === null || notSeenAttestingIndices.size === 0) { + continue; + } - if ( - slotCount > 2 && - attestationsByScore.length >= MAX_ATTESTATIONS && - notSeenAttestingIndices.size / slotDelta < minScore - ) { - // after 2 slots, there are a good chance that we have 2 * MAX_ATTESTATIONS attestations and break the for loop early - // if not, we may have to scan all slots in the pool - // if we have enough attestations and the max possible score is lower than scores of `attestationsByScore`, we should skip - // otherwise it takes time to check attestation, add it and remove it later after the sort by score - continue; - } + if ( + slotCount > 2 && + attestationsByScore.length >= MAX_ATTESTATIONS && + notSeenAttestingIndices.size / slotDelta < minScore + ) { + // after 2 slots, there are a good chance that we have 2 * MAX_ATTESTATIONS attestations and break the for loop early + // if not, we may have to scan all slots in the pool + // if we have enough attestations and the max possible score is lower than scores of `attestationsByScore`, we should skip + // otherwise it takes time to check attestation, add it and remove it later after the sort by score + continue; + } - for (const attestationGroup of attestationGroupByData.values()) { if (!validateAttestationDataFn(attestationGroup.data)) { continue; } @@ -207,6 +237,7 @@ export class AggregatedAttestationPool { // IF they have to be validated, do it only with one attestation per group since same data // The committeeCountPerSlot can be precomputed once per slot for (const {attestation, notSeenAttesterCount} of attestationGroup.getAttestationsForBlock( + fork, notSeenAttestingIndices )) { const score = notSeenAttesterCount / slotDelta; @@ -227,34 +258,128 @@ export class AggregatedAttestationPool { } } - if (ForkSeq[fork] >= ForkSeq.electra) { - // In Electra, we further pack attestations with same attestationData from different committee - const sortedAttestationsByScore = this.aggregateAttestationsByScore(attestationsByScore).sort( - (a, b) => b.score - a.score - ); - const attestationsForBlock: electra.Attestation[] = []; - for (const [i, attestationWithScore] of sortedAttestationsByScore.entries()) { - if (i >= MAX_ATTESTATIONS_ELECTRA) { - break; - } - // attestations could be modified in this op pool, so we need to clone for block - attestationsForBlock.push( - ssz.electra.Attestation.clone(attestationWithScore.attestation as electra.Attestation) - ); + const sortedAttestationsByScore = attestationsByScore.sort((a, b) => b.score - a.score); + const attestationsForBlock: phase0.Attestation[] = []; + for (const [i, attestationWithScore] of sortedAttestationsByScore.entries()) { + if (i >= MAX_ATTESTATIONS) { + break; } - return attestationsForBlock; - } else { - const sortedAttestationsByScore = attestationsByScore.sort((a, b) => b.score - a.score); - const attestationsForBlock: phase0.Attestation[] = []; - for (const [i, attestationWithScore] of sortedAttestationsByScore.entries()) { - if (i >= MAX_ATTESTATIONS) { - break; + // attestations could be modified in this op pool, so we need to clone for block + attestationsForBlock.push(ssz.phase0.Attestation.clone(attestationWithScore.attestation)); + } + return attestationsForBlock; + } + + /** + * Get attestations to be included in an electra block. Returns up to $MAX_ATTESTATIONS_ELECTRA items + */ + getAttestationsForBlockElectra( + fork: ForkName, + forkChoice: IForkChoice, + state: CachedBeaconStateAllForks + ): allForks.Attestation[] { + const stateSlot = state.slot; + const stateEpoch = state.epochCtx.epoch; + const statePrevEpoch = stateEpoch - 1; + + const notSeenValidatorsFn = getNotSeenValidatorsFn(state); + const validateAttestationDataFn = getValidateAttestationDataFn(forkChoice, state); + + const slots = Array.from(this.attestationGroupByIndexByDataHashBySlot.keys()).sort((a, b) => b - a); + const consolidations: AttestationsConsolidation[] = []; + let minScore = Number.MAX_SAFE_INTEGER; + let slotCount = 0; + slot: for (const slot of slots) { + slotCount++; + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.get(slot); + // should not happen + if (!attestationGroupByIndexByDataHash) { + throw Error(`No aggregated attestation pool for slot=${slot}`); + } + + const epoch = computeEpochAtSlot(slot); + // validateAttestation condition: Attestation target epoch not in previous or current epoch + if (!(epoch === stateEpoch || epoch === statePrevEpoch)) { + continue; // Invalid attestations + } + // validateAttestation condition: Attestation slot not within inclusion window + if (!(slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot)) { + continue; // Invalid attestations + } + + const slotDelta = stateSlot - slot; + // CommitteeIndex 0 1 2 ... Consolidation + // Attestations att00 --- att10 --- att20 --- 0 (att 00 10 20) + // att01 --- - --- att21 --- 1 (att 01 __ 21) + // - --- - --- att22 --- 2 (att __ __ 22) + for (const attestationGroupByIndex of attestationGroupByIndexByDataHash.values()) { + // sameAttDataCons could be up to MAX_ATTESTATIONS_PER_GROUP + const sameAttDataCons: AttestationsConsolidation[] = []; + for (const [committeeIndex, attestationGroup] of attestationGroupByIndex.entries()) { + const notSeenAttestingIndices = notSeenValidatorsFn(epoch, slot, committeeIndex); + if (notSeenAttestingIndices === null || notSeenAttestingIndices.size === 0) { + continue; + } + + if ( + slotCount > 2 && + consolidations.length >= MAX_ATTESTATIONS_ELECTRA && + notSeenAttestingIndices.size / slotDelta < minScore + ) { + // after 2 slots, there are a good chance that we have 2 * MAX_ATTESTATIONS_ELECTRA attestations and break the for loop early + // if not, we may have to scan all slots in the pool + // if we have enough attestations and the max possible score is lower than scores of `attestationsByScore`, we should skip + // otherwise it takes time to check attestation, add it and remove it later after the sort by score + continue; + } + + if (!validateAttestationDataFn(attestationGroup.data)) { + continue; + } + + // TODO: Is it necessary to validateAttestation for: + // - Attestation committee index not within current committee count + // - Attestation aggregation bits length does not match committee length + // + // These properties should not change after being validate in gossip + // IF they have to be validated, do it only with one attestation per group since same data + // The committeeCountPerSlot can be precomputed once per slot + // for (const [i, {attestation, notSeenAttesterCount}] of attestationGroup + for (const [i, attestationNonParticipation] of attestationGroup + .getAttestationsForBlock(fork, notSeenAttestingIndices) + .entries()) { + if (sameAttDataCons[i] === undefined) { + sameAttDataCons[i] = { + byCommittee: new Map(), + attData: attestationNonParticipation.attestation.data, + totalNotSeenCount: 0, + // only update score after we have full data + score: 0, + }; + } + sameAttDataCons[i].byCommittee.set(committeeIndex, attestationNonParticipation); + sameAttDataCons[i].totalNotSeenCount += attestationNonParticipation.notSeenAttesterCount; + } + for (const consolidation of sameAttDataCons) { + const score = consolidation.totalNotSeenCount / slotDelta; + if (score < minScore) { + minScore = score; + } + consolidations.push({...consolidation, score}); + // Stop accumulating attestations there are enough that may have good scoring + if (consolidations.length >= MAX_ATTESTATIONS_ELECTRA * 2) { + break slot; + } + } } - // attestations could be modified in this op pool, so we need to clone for block - attestationsForBlock.push(ssz.phase0.Attestation.clone(attestationWithScore.attestation)); } - return attestationsForBlock; } + + const sortedConsolidationsByScore = consolidations + .sort((a, b) => b.score - a.score) + .slice(0, MAX_ATTESTATIONS_ELECTRA); + // on chain aggregation is expensive, only do it after all + return sortedConsolidationsByScore.map(consolidationToAttestation); } /** @@ -262,13 +387,13 @@ export class AggregatedAttestationPool { * @param bySlot slot to filter, `bySlot === attestation.data.slot` */ getAll(bySlot?: Slot): allForks.Attestation[] { - let attestationGroupsArr: Map[]; + let attestationGroupsArr: Map[]; if (bySlot === undefined) { - attestationGroupsArr = Array.from(this.attestationGroupByDataHashByIndexBySlot.values()).flatMap((byIndex) => + attestationGroupsArr = Array.from(this.attestationGroupByIndexByDataHashBySlot.values()).flatMap((byIndex) => Array.from(byIndex.values()) ); } else { - const attestationGroupsByIndex = this.attestationGroupByDataHashByIndexBySlot.get(bySlot); + const attestationGroupsByIndex = this.attestationGroupByIndexByDataHashBySlot.get(bySlot); if (!attestationGroupsByIndex) throw Error(`No attestations for slot ${bySlot}`); attestationGroupsArr = Array.from(attestationGroupsByIndex.values()); } @@ -281,16 +406,6 @@ export class AggregatedAttestationPool { } return attestations; } - - /** - * Electra and after: Block proposer consolidates attestations with the same - * attestation data from different committee into a single attestation - * https://github.com/ethereum/consensus-specs/blob/aba6345776aa876dad368cab27fbbb23fae20455/specs/_features/eip7549/validator.md?plain=1#L39 - * TODO Electra: implement this or consider other strategy - */ - private aggregateAttestationsByScore(attestationsByScore: AttestationWithScore[]): AttestationWithScore[] { - return attestationsByScore; - } } interface AttestationWithIndex { @@ -381,8 +496,9 @@ export class MatchingDataAttestationGroup { * @param notSeenAttestingIndices not seen attestting indices, i.e. indices in the same committee * @returns an array of AttestationNonParticipant */ - getAttestationsForBlock(notSeenAttestingIndices: Set): AttestationNonParticipant[] { + getAttestationsForBlock(fork: ForkName, notSeenAttestingIndices: Set): AttestationNonParticipant[] { const attestations: AttestationNonParticipant[] = []; + const forkSeq = ForkSeq[fork]; for (const {attestation} of this.attestations) { let notSeenAttesterCount = 0; const {aggregationBits} = attestation; @@ -392,7 +508,8 @@ export class MatchingDataAttestationGroup { } } - if (notSeenAttesterCount > 0) { + // if fork >= electra, should return electra-only attestations + if (notSeenAttesterCount > 0 && (forkSeq < ForkSeq.electra || isElectraAttestation(attestation))) { attestations.push({attestation, notSeenAttesterCount}); } } @@ -421,6 +538,34 @@ export function aggregateInto(attestation1: AttestationWithIndex, attestation2: attestation1.attestation.signature = bls.Signature.aggregate([signature1, signature2]).toBytes(); } +/** + * Electra and after: Block proposer consolidates attestations with the same + * attestation data from different committee into a single attestation + * https://github.com/ethereum/consensus-specs/blob/aba6345776aa876dad368cab27fbbb23fae20455/specs/_features/eip7549/validator.md?plain=1#L39 + */ +export function consolidationToAttestation({byCommittee, attData}: AttestationsConsolidation): electra.Attestation { + const committeeBits = BitArray.fromBitLen(MAX_COMMITTEES_PER_SLOT); + // TODO: can we improve this? + let aggregationBits: boolean[] = []; + const signatures: Signature[] = []; + const sortedCommittees = Array.from(byCommittee.keys()).sort((a, b) => a - b); + for (const committeeIndex of sortedCommittees) { + const attestationNonParticipation = byCommittee.get(committeeIndex); + if (attestationNonParticipation !== undefined) { + const {attestation} = attestationNonParticipation; + committeeBits.set(committeeIndex, true); + aggregationBits = [...aggregationBits, ...attestation.aggregationBits.toBoolArray()]; + signatures.push(signatureFromBytesNoCheck(attestation.signature)); + } + } + return { + aggregationBits: BitArray.fromBoolArray(aggregationBits), + data: attData, + committeeBits, + signature: bls.Signature.aggregate(signatures).toBytes(), + }; +} + /** * Pre-compute participation from a CachedBeaconStateAllForks, for use to check if an attestation's committee * has already attested or not. @@ -443,12 +588,15 @@ export function getNotSeenValidatorsFn(state: CachedBeaconStateAllForks): GetNot state ); - return (epoch: Epoch, committee: Uint32Array) => { + return (epoch: Epoch, slot: Slot, committeeIndex: number) => { const participants = epoch === stateEpoch ? currentEpochParticipants : epoch === stateEpoch - 1 ? previousEpochParticipants : null; if (participants === null) { return null; } + const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); + const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; + const committee = slotCommittees[committeeIndex]; const notSeenAttestingIndices = new Set(); for (const [i, validatorIndex] of committee.entries()) { @@ -470,22 +618,35 @@ export function getNotSeenValidatorsFn(state: CachedBeaconStateAllForks): GetNot const previousParticipation = altairState.previousEpochParticipation.getAll(); const currentParticipation = altairState.currentEpochParticipation.getAll(); const stateEpoch = computeEpochAtSlot(state.slot); + // this function could be called multiple times with same slot + committeeIndex + const cachedNotSeenValidators = new Map>(); - return (epoch: Epoch, committee: Uint32Array) => { + return (epoch: Epoch, slot: Slot, committeeIndex: number) => { const participationStatus = epoch === stateEpoch ? currentParticipation : epoch === stateEpoch - 1 ? previousParticipation : null; if (participationStatus === null) { return null; } + const cacheKey = slot + "_" + committeeIndex; + let notSeenAttestingIndices = cachedNotSeenValidators.get(cacheKey); + if (notSeenAttestingIndices != null) { + // if all validators are seen then return null, we don't need to check for any attestations of same committee again + return notSeenAttestingIndices.size === 0 ? null : notSeenAttestingIndices; + } - const notSeenAttestingIndices = new Set(); + const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); + const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; + const committee = slotCommittees[committeeIndex]; + + notSeenAttestingIndices = new Set(); for (const [i, validatorIndex] of committee.entries()) { // no need to check flagIsTimelySource as if validator is not seen, it's participation status is 0 if (participationStatus[validatorIndex] === 0) { notSeenAttestingIndices.add(i); } } + cachedNotSeenValidators.set(cacheKey, notSeenAttestingIndices); // if all validators are seen then return null, we don't need to check for any attestations of same committee again return notSeenAttestingIndices.size === 0 ? null : notSeenAttestingIndices; }; diff --git a/packages/beacon-node/test/unit/api/impl/events/events.test.ts b/packages/beacon-node/test/unit/api/impl/events/events.test.ts index 52ece27c4d5d..e19f369cf6ad 100644 --- a/packages/beacon-node/test/unit/api/impl/events/events.test.ts +++ b/packages/beacon-node/test/unit/api/impl/events/events.test.ts @@ -2,6 +2,7 @@ import {describe, it, expect, beforeEach, afterEach, vi, MockedObject} from "vit import {routes} from "@lodestar/api"; import {config} from "@lodestar/config/default"; import {ssz} from "@lodestar/types"; +import {ForkName} from "@lodestar/params"; import {BeaconChain, ChainEventEmitter, HeadEventData} from "../../../../../src/chain/index.js"; import {getEventsApi} from "../../../../../src/api/impl/events/index.js"; import {ZERO_HASH_HEX} from "../../../../../src/constants/constants.js"; @@ -62,7 +63,10 @@ describe("Events api impl", function () { it("should ignore not sent topics", async function () { const events = getEvents([routes.events.EventType.head]); - chainEventEmmitter.emit(routes.events.EventType.attestation, ssz.phase0.Attestation.defaultValue()); + chainEventEmmitter.emit(routes.events.EventType.attestation, { + version: ForkName.phase0, + data: ssz.phase0.Attestation.defaultValue(), + }); chainEventEmmitter.emit(routes.events.EventType.head, headEventData); expect(events).toHaveLength(1); diff --git a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index 3c248ad4d194..048b10011932 100644 --- a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -85,7 +85,7 @@ describe("AggregatedAttestationPool", function () { // previousEpochParticipation and currentEpochParticipation is created inside generateCachedState // 0 and 1 are fully participated const notSeenValidatorFn = getNotSeenValidatorsFn(altairState); - const participation = notSeenValidatorFn(currentEpoch, committee); + const participation = notSeenValidatorFn(currentEpoch, currentSlot, committeeIndex); // seen attesting indices are 0, 1 => not seen are 2, 3 expect(participation).toEqual( // { @@ -280,6 +280,7 @@ describe("MatchingDataAttestationGroup.getAttestationsForBlock", () => { } } const attestationsForBlock = attestationGroup.getAttestationsForBlock( + ForkName.phase0, // notSeenValidatorIndices, notSeenAttestingIndices ); From 1e83f104875e907982a3de871ca9fa2d09e3fbe5 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 4 May 2024 14:59:33 +0700 Subject: [PATCH 2/6] chore: fix lint --- .../src/chain/opPools/aggregatedAttestationPool.ts | 2 +- .../state-transition/src/signatureSets/attesterSlashings.ts | 2 +- packages/types/src/electra/types.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 76902b370488..981ccef1a1fa 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -85,7 +85,7 @@ const MAX_ATTESTATIONS_PER_GROUP = 3; * Note that we want to remove attestations with attesters that were included in the chain. */ export class AggregatedAttestationPool { - // per electra, different committees could have the same AttData hash + // per electra, different committees could have the same AttData hex private readonly attestationGroupByIndexByDataHashBySlot = new MapDef< Slot, Map> diff --git a/packages/state-transition/src/signatureSets/attesterSlashings.ts b/packages/state-transition/src/signatureSets/attesterSlashings.ts index d83d55dc9e1c..9b04fceac45f 100644 --- a/packages/state-transition/src/signatureSets/attesterSlashings.ts +++ b/packages/state-transition/src/signatureSets/attesterSlashings.ts @@ -16,7 +16,7 @@ export function getAttesterSlashingsSignatureSets( /** Get signature sets from a single AttesterSlashing object */ export function getAttesterSlashingSignatureSets( state: CachedBeaconStateAllForks, - attesterSlashing: allForks.AttesterSlashing, + attesterSlashing: allForks.AttesterSlashing ): ISignatureSet[] { return [attesterSlashing.attestation1, attesterSlashing.attestation2].map((attestation) => getIndexedAttestationBigintSignatureSet(state, attestation) diff --git a/packages/types/src/electra/types.ts b/packages/types/src/electra/types.ts index 5264118b1820..ccfa34d19c99 100644 --- a/packages/types/src/electra/types.ts +++ b/packages/types/src/electra/types.ts @@ -45,5 +45,5 @@ export type LightClientStore = ValueOf; export type AggregateAndProof = ValueOf; export type SignedAggregateAndProof = ValueOf; -export type AttesterSlashing = ValueOf -export type IndexedAttestationBigint = ValueOf \ No newline at end of file +export type AttesterSlashing = ValueOf; +export type IndexedAttestationBigint = ValueOf; From 0df33bf2556b09529495feb9ac0dbc4637b092ef Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Mon, 6 May 2024 09:37:48 +0700 Subject: [PATCH 3/6] fix: MAX_ATTESTATIONS_PER_GROUP_ELECTRA and address PR comments --- .../opPools/aggregatedAttestationPool.ts | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 981ccef1a1fa..e82c554b6000 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -41,7 +41,10 @@ type CommitteeIndex = number; // for pre-electra type AttestationWithScore = {attestation: allForks.Attestation; score: number}; -// for electra +/** + * for electra, this is to consolidate aggregated attestations of the same attestation data into a single attestation to be included in block + * note that this is not validator consolidation + */ type AttestationsConsolidation = { byCommittee: Map; attData: phase0.AttestationData; @@ -71,12 +74,15 @@ const MAX_RETAINED_ATTESTATIONS_PER_GROUP = 4; * Starting from Jan 2024, we have a performance issue getting attestations for a block. Based on the * fact that lot of groups will have only 1 full participation attestation, increase this number * a bit higher than average. This also help decrease number of slots to search for attestations. - * + */ +const MAX_ATTESTATIONS_PER_GROUP = 3; + +/** * For electra, each block has up to 8 aggregated attestations, assuming there are 3 for the "best" * attestation data, there are still 5 for other attestation data so this constant is still good. * We should separate to 2 constant based on conditions of different networks */ -const MAX_ATTESTATIONS_PER_GROUP = 3; +const MAX_ATTESTATIONS_PER_GROUP_ELECTRA = 3; /** * Maintain a pool of aggregated attestations. Attestations can be retrieved for inclusion in a block @@ -85,8 +91,12 @@ const MAX_ATTESTATIONS_PER_GROUP = 3; * Note that we want to remove attestations with attesters that were included in the chain. */ export class AggregatedAttestationPool { - // per electra, different committees could have the same AttData hex - private readonly attestationGroupByIndexByDataHashBySlot = new MapDef< + /** + * post electra, different committees could have the same AttData and we have to consolidate attestations of the same + * data to be included in block, so we should group by data before index + * // TODO: make sure it does not affect performance for pre electra forks + */ + private readonly attestationGroupByIndexByDataHexBySlot = new MapDef< Slot, Map> >(() => new Map>()); @@ -96,8 +106,8 @@ export class AggregatedAttestationPool { getAttestationCount(): {attestationCount: number; attestationDataCount: number} { let attestationCount = 0; let attestationDataCount = 0; - for (const attestationGroupByIndexByData of this.attestationGroupByIndexByDataHashBySlot.values()) { - for (const attestationGroupByIndex of attestationGroupByIndexByData.values()) { + for (const attestationGroupByIndexByDataHex of this.attestationGroupByIndexByDataHexBySlot.values()) { + for (const attestationGroupByIndex of attestationGroupByIndexByDataHex.values()) { attestationDataCount += attestationGroupByIndex.size; for (const attestationGroup of attestationGroupByIndex.values()) { attestationCount += attestationGroup.getAttestationCount(); @@ -121,7 +131,7 @@ export class AggregatedAttestationPool { return InsertOutcome.Old; } - const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.getOrDefault(slot); + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHexBySlot.getOrDefault(slot); let attestationGroupByIndex = attestationGroupByIndexByDataHash.get(dataRootHex); if (!attestationGroupByIndex) { attestationGroupByIndex = new Map(); @@ -129,8 +139,12 @@ export class AggregatedAttestationPool { } const committeeIndex = isElectraAttestation(attestation) ? // this attestation is added to pool after validation - (attestation.committeeBits.getSingleTrueBit() as number) + 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}`); + } let attestationGroup = attestationGroupByIndex.get(committeeIndex); if (!attestationGroup) { attestationGroup = new MatchingDataAttestationGroup(committee, attestation.data); @@ -146,7 +160,7 @@ export class AggregatedAttestationPool { /** Remove attestations which are too old to be included in a block. */ prune(clockSlot: Slot): void { // Only retain SLOTS_PER_EPOCH slots - pruneBySlot(this.attestationGroupByIndexByDataHashBySlot, clockSlot, SLOTS_PER_EPOCH); + pruneBySlot(this.attestationGroupByIndexByDataHexBySlot, clockSlot, SLOTS_PER_EPOCH); this.lowestPermissibleSlot = Math.max(clockSlot - SLOTS_PER_EPOCH, 0); } @@ -156,9 +170,9 @@ export class AggregatedAttestationPool { state: CachedBeaconStateAllForks ): allForks.Attestation[] { const forkSeq = ForkSeq[fork]; - return forkSeq < ForkSeq.electra - ? this.getAttestationsForBlockPreElectra(fork, forkChoice, state) - : this.getAttestationsForBlockElectra(fork, forkChoice, state); + return forkSeq >= ForkSeq.electra + ? this.getAttestationsForBlockElectra(fork, forkChoice, state) + : this.getAttestationsForBlockPreElectra(fork, forkChoice, state); } /** @@ -178,12 +192,12 @@ export class AggregatedAttestationPool { const attestationsByScore: AttestationWithScore[] = []; - const slots = Array.from(this.attestationGroupByIndexByDataHashBySlot.keys()).sort((a, b) => b - a); + const slots = Array.from(this.attestationGroupByIndexByDataHexBySlot.keys()).sort((a, b) => b - a); let minScore = Number.MAX_SAFE_INTEGER; let slotCount = 0; slot: for (const slot of slots) { slotCount++; - const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.get(slot); + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHexBySlot.get(slot); // should not happen if (!attestationGroupByIndexByDataHash) { throw Error(`No aggregated attestation pool for slot=${slot}`); @@ -285,13 +299,13 @@ export class AggregatedAttestationPool { const notSeenValidatorsFn = getNotSeenValidatorsFn(state); const validateAttestationDataFn = getValidateAttestationDataFn(forkChoice, state); - const slots = Array.from(this.attestationGroupByIndexByDataHashBySlot.keys()).sort((a, b) => b - a); + const slots = Array.from(this.attestationGroupByIndexByDataHexBySlot.keys()).sort((a, b) => b - a); const consolidations: AttestationsConsolidation[] = []; let minScore = Number.MAX_SAFE_INTEGER; let slotCount = 0; slot: for (const slot of slots) { slotCount++; - const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHashBySlot.get(slot); + const attestationGroupByIndexByDataHash = this.attestationGroupByIndexByDataHexBySlot.get(slot); // should not happen if (!attestationGroupByIndexByDataHash) { throw Error(`No aggregated attestation pool for slot=${slot}`); @@ -313,7 +327,7 @@ export class AggregatedAttestationPool { // att01 --- - --- att21 --- 1 (att 01 __ 21) // - --- - --- att22 --- 2 (att __ __ 22) for (const attestationGroupByIndex of attestationGroupByIndexByDataHash.values()) { - // sameAttDataCons could be up to MAX_ATTESTATIONS_PER_GROUP + // sameAttDataCons could be up to MAX_ATTESTATIONS_PER_GROUP_ELECTRA const sameAttDataCons: AttestationsConsolidation[] = []; for (const [committeeIndex, attestationGroup] of attestationGroupByIndex.entries()) { const notSeenAttestingIndices = notSeenValidatorsFn(epoch, slot, committeeIndex); @@ -389,11 +403,11 @@ export class AggregatedAttestationPool { getAll(bySlot?: Slot): allForks.Attestation[] { let attestationGroupsArr: Map[]; if (bySlot === undefined) { - attestationGroupsArr = Array.from(this.attestationGroupByIndexByDataHashBySlot.values()).flatMap((byIndex) => + attestationGroupsArr = Array.from(this.attestationGroupByIndexByDataHexBySlot.values()).flatMap((byIndex) => Array.from(byIndex.values()) ); } else { - const attestationGroupsByIndex = this.attestationGroupByIndexByDataHashBySlot.get(bySlot); + const attestationGroupsByIndex = this.attestationGroupByIndexByDataHexBySlot.get(bySlot); if (!attestationGroupsByIndex) throw Error(`No attestations for slot ${bySlot}`); attestationGroupsArr = Array.from(attestationGroupsByIndex.values()); } @@ -514,12 +528,11 @@ export class MatchingDataAttestationGroup { } } - if (attestations.length <= MAX_ATTESTATIONS_PER_GROUP) { + const maxAttestation = forkSeq >= ForkSeq.electra ? MAX_ATTESTATIONS_PER_GROUP_ELECTRA : MAX_ATTESTATIONS_PER_GROUP; + if (attestations.length <= maxAttestation) { return attestations; } else { - return attestations - .sort((a, b) => b.notSeenAttesterCount - a.notSeenAttesterCount) - .slice(0, MAX_ATTESTATIONS_PER_GROUP); + return attestations.sort((a, b) => b.notSeenAttesterCount - a.notSeenAttesterCount).slice(0, maxAttestation); } } @@ -594,9 +607,7 @@ export function getNotSeenValidatorsFn(state: CachedBeaconStateAllForks): GetNot if (participants === null) { return null; } - const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); - const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; - const committee = slotCommittees[committeeIndex]; + const committee = state.epochCtx.getBeaconCommittee(slot, committeeIndex); const notSeenAttestingIndices = new Set(); for (const [i, validatorIndex] of committee.entries()) { @@ -635,10 +646,7 @@ export function getNotSeenValidatorsFn(state: CachedBeaconStateAllForks): GetNot return notSeenAttestingIndices.size === 0 ? null : notSeenAttestingIndices; } - const shuffling = state.epochCtx.getShufflingAtEpoch(epoch); - const slotCommittees = shuffling.committees[slot % SLOTS_PER_EPOCH]; - const committee = slotCommittees[committeeIndex]; - + const committee = state.epochCtx.getBeaconCommittee(slot, committeeIndex); notSeenAttestingIndices = new Set(); for (const [i, validatorIndex] of committee.entries()) { // no need to check flagIsTimelySource as if validator is not seen, it's participation status is 0 From 60645a53e948784e4541e6aa3c2a0fb23b1562c7 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 7 May 2024 16:04:54 +0700 Subject: [PATCH 4/6] chore: unit test aggregateConsolidation --- .../opPools/aggregatedAttestationPool.ts | 8 +- .../opPools/aggregatedAttestationPool.test.ts | 85 ++++++++++++++++++- 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index e82c554b6000..5d794d29637d 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -43,9 +43,9 @@ type CommitteeIndex = number; type AttestationWithScore = {attestation: allForks.Attestation; score: number}; /** * for electra, this is to consolidate aggregated attestations of the same attestation data into a single attestation to be included in block - * note that this is not validator consolidation + * note that this is local definition in this file and it's NOT validator consolidation */ -type AttestationsConsolidation = { +export type AttestationsConsolidation = { byCommittee: Map; attData: phase0.AttestationData; totalNotSeenCount: number; @@ -393,7 +393,7 @@ export class AggregatedAttestationPool { .sort((a, b) => b.score - a.score) .slice(0, MAX_ATTESTATIONS_ELECTRA); // on chain aggregation is expensive, only do it after all - return sortedConsolidationsByScore.map(consolidationToAttestation); + return sortedConsolidationsByScore.map(aggregateConsolidation); } /** @@ -556,7 +556,7 @@ export function aggregateInto(attestation1: AttestationWithIndex, attestation2: * attestation data from different committee into a single attestation * https://github.com/ethereum/consensus-specs/blob/aba6345776aa876dad368cab27fbbb23fae20455/specs/_features/eip7549/validator.md?plain=1#L39 */ -export function consolidationToAttestation({byCommittee, attData}: AttestationsConsolidation): electra.Attestation { +export function aggregateConsolidation({byCommittee, attData}: AttestationsConsolidation): electra.Attestation { const committeeBits = BitArray.fromBitLen(MAX_COMMITTEES_PER_SLOT); // TODO: can we improve this? let aggregationBits: boolean[] = []; diff --git a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index 048b10011932..ec570483f689 100644 --- a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -3,13 +3,21 @@ import bls from "@chainsafe/bls"; import {BitArray, fromHexString, toHexString} from "@chainsafe/ssz"; import {describe, it, expect, beforeEach, beforeAll, afterEach, vi} from "vitest"; import {CachedBeaconStateAllForks, newFilledArray} from "@lodestar/state-transition"; -import {FAR_FUTURE_EPOCH, ForkName, MAX_EFFECTIVE_BALANCE, SLOTS_PER_EPOCH} from "@lodestar/params"; +import { + FAR_FUTURE_EPOCH, + ForkName, + MAX_COMMITTEES_PER_SLOT, + MAX_EFFECTIVE_BALANCE, + SLOTS_PER_EPOCH, +} from "@lodestar/params"; import {ssz, phase0} from "@lodestar/types"; import {CachedBeaconStateAltair} from "@lodestar/state-transition/src/types.js"; import {MockedForkChoice, getMockedForkChoice} from "../../../mocks/mockedBeaconChain.js"; import { + aggregateConsolidation, AggregatedAttestationPool, aggregateInto, + AttestationsConsolidation, getNotSeenValidatorsFn, MatchingDataAttestationGroup, } from "../../../../src/chain/opPools/aggregatedAttestationPool.js"; @@ -81,7 +89,7 @@ describe("AggregatedAttestationPool", function () { vi.clearAllMocks(); }); - it("getParticipationFn", () => { + it("getNotSeenValidatorsFn", () => { // previousEpochParticipation and currentEpochParticipation is created inside generateCachedState // 0 and 1 are fully participated const notSeenValidatorFn = getNotSeenValidatorsFn(altairState); @@ -320,3 +328,76 @@ describe("MatchingDataAttestationGroup aggregateInto", function () { expect(aggregatedSignature.verifyAggregate([sk1.toPublicKey(), sk2.toPublicKey()], attestationDataRoot)).toBe(true); }); }); + +describe("aggregateConsolidation", function () { + const sk0 = bls.SecretKey.fromBytes(Buffer.alloc(32, 1)); + const sk1 = bls.SecretKey.fromBytes(Buffer.alloc(32, 2)); + const sk2 = bls.SecretKey.fromBytes(Buffer.alloc(32, 3)); + const skArr = [sk0, sk1, sk2]; + const testCases: { + name: string; + committeeIndices: number[]; + aggregationBitsArr: Array[]; + expectedAggregationBits: Array; + expectedCommitteeBits: Array; + }[] = [ + // note that bit index starts from the right + { + name: "test case 0", + committeeIndices: [0, 1, 2], + aggregationBitsArr: [[0b111], [0b011], [0b111]], + expectedAggregationBits: [0b11011111, 0b1], + expectedCommitteeBits: [true, true, true, false], + }, + { + name: "test case 1", + committeeIndices: [2, 3, 1], + aggregationBitsArr: [[0b100], [0b010], [0b001]], + expectedAggregationBits: [0b10100001, 0b0], + expectedCommitteeBits: [false, true, true, true], + }, + ]; + for (const { + name, + committeeIndices, + aggregationBitsArr, + expectedAggregationBits, + expectedCommitteeBits, + } of testCases) { + it(name, () => { + const attData = ssz.phase0.AttestationData.defaultValue(); + const consolidation: AttestationsConsolidation = { + byCommittee: new Map(), + attData: attData, + totalNotSeenCount: 0, + score: 0, + }; + // to simplify, instead of signing the signingRoot, just sign the attData root + const sigArr = skArr.map((sk) => sk.sign(ssz.phase0.AttestationData.hashTreeRoot(attData))); + const attestationSeed = ssz.electra.Attestation.defaultValue(); + for (let i = 0; i < committeeIndices.length; i++) { + const committeeIndex = committeeIndices[i]; + const commiteeBits = BitArray.fromBoolArray( + Array.from({length: MAX_COMMITTEES_PER_SLOT}, (_, i) => i === committeeIndex) + ); + const aggAttestation = { + ...attestationSeed, + // aggregationBits: BitArray.fromBoolArray(aggregationBitsArr[i]), + aggregationBits: new BitArray(new Uint8Array(aggregationBitsArr[i]), 3), + committeeBits: commiteeBits, + signature: sigArr[i].toBytes(), + }; + consolidation.byCommittee.set(committeeIndex, { + attestation: aggAttestation, + notSeenAttesterCount: aggregationBitsArr[i].filter((item) => item).length, + }); + } + + const finalAttestation = aggregateConsolidation(consolidation); + expect(finalAttestation.aggregationBits.uint8Array).toEqual(new Uint8Array(expectedAggregationBits)); + expect(finalAttestation.committeeBits.toBoolArray()).toEqual(expectedCommitteeBits); + expect(finalAttestation.data).toEqual(attData); + expect(finalAttestation.signature).toEqual(bls.Signature.aggregate(sigArr).toBytes()); + }); + } +}); From 6408326467969b4b53dc8597e24aeb23f2121f22 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Tue, 7 May 2024 20:54:24 +0300 Subject: [PATCH 5/6] Fix rebase mistake --- packages/types/src/electra/sszTypes.ts | 17 ----------------- packages/types/src/electra/types.ts | 5 ----- 2 files changed, 22 deletions(-) diff --git a/packages/types/src/electra/sszTypes.ts b/packages/types/src/electra/sszTypes.ts index 849d0ac0f443..53195f30de11 100644 --- a/packages/types/src/electra/sszTypes.ts +++ b/packages/types/src/electra/sszTypes.ts @@ -367,20 +367,3 @@ export const SSEPayloadAttributes = new ContainerType( }, {typeName: "SSEPayloadAttributes", jsonCase: "eth2"} ); - -export const AggregateAndProof = new ContainerType( - { - aggregatorIndex: ValidatorIndex, - aggregate: Attestation, - selectionProof: BLSSignature, - }, - {typeName: "AggregateAndProof", jsonCase: "eth2", cachePermanentRootStruct: true} -); - -export const SignedAggregateAndProof = new ContainerType( - { - message: AggregateAndProof, - signature: BLSSignature, - }, - {typeName: "SignedAggregateAndProof", jsonCase: "eth2"} -); diff --git a/packages/types/src/electra/types.ts b/packages/types/src/electra/types.ts index ccfa34d19c99..d1d0109e6ae6 100644 --- a/packages/types/src/electra/types.ts +++ b/packages/types/src/electra/types.ts @@ -42,8 +42,3 @@ export type LightClientUpdate = ValueOf; export type LightClientFinalityUpdate = ValueOf; export type LightClientOptimisticUpdate = ValueOf; export type LightClientStore = ValueOf; - -export type AggregateAndProof = ValueOf; -export type SignedAggregateAndProof = ValueOf; -export type AttesterSlashing = ValueOf; -export type IndexedAttestationBigint = ValueOf; From ea89168f6765371ba6436d3be549fab453304356 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Tue, 7 May 2024 21:27:58 +0300 Subject: [PATCH 6/6] Address my own comment :) --- .../src/chain/opPools/aggregatedAttestationPool.ts | 5 ++--- .../unit/chain/opPools/aggregatedAttestationPool.test.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts index 5d794d29637d..d6cceb9572ae 100644 --- a/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/beacon-node/src/chain/opPools/aggregatedAttestationPool.ts @@ -182,7 +182,7 @@ export class AggregatedAttestationPool { fork: ForkName, forkChoice: IForkChoice, state: CachedBeaconStateAllForks - ): allForks.Attestation[] { + ): phase0.Attestation[] { const stateSlot = state.slot; const stateEpoch = state.epochCtx.epoch; const statePrevEpoch = stateEpoch - 1; @@ -291,7 +291,7 @@ export class AggregatedAttestationPool { fork: ForkName, forkChoice: IForkChoice, state: CachedBeaconStateAllForks - ): allForks.Attestation[] { + ): electra.Attestation[] { const stateSlot = state.slot; const stateEpoch = state.epochCtx.epoch; const statePrevEpoch = stateEpoch - 1; @@ -358,7 +358,6 @@ export class AggregatedAttestationPool { // These properties should not change after being validate in gossip // IF they have to be validated, do it only with one attestation per group since same data // The committeeCountPerSlot can be precomputed once per slot - // for (const [i, {attestation, notSeenAttesterCount}] of attestationGroup for (const [i, attestationNonParticipation] of attestationGroup .getAttestationsForBlock(fork, notSeenAttestingIndices) .entries()) { diff --git a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index ec570483f689..c375c9956758 100644 --- a/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/beacon-node/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -382,7 +382,6 @@ describe("aggregateConsolidation", function () { ); const aggAttestation = { ...attestationSeed, - // aggregationBits: BitArray.fromBoolArray(aggregationBitsArr[i]), aggregationBits: new BitArray(new Uint8Array(aggregationBitsArr[i]), 3), committeeBits: commiteeBits, signature: sigArr[i].toBytes(),