diff --git a/packages/lodestar/src/api/impl/validator/index.ts b/packages/lodestar/src/api/impl/validator/index.ts index 06b7b1d72723..b9c32476de80 100644 --- a/packages/lodestar/src/api/impl/validator/index.ts +++ b/packages/lodestar/src/api/impl/validator/index.ts @@ -441,7 +441,7 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}: chain.aggregatedAttestationPool.add( signedAggregateAndProof.message.aggregate, - indexedAttestation.attestingIndices, + indexedAttestation.attestingIndices.length, committeeIndices ); const sentPeers = await network.gossip.publishBeaconAggregateAndProof(signedAggregateAndProof); diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts index 8ecd286c036d..73f7e42b9211 100644 --- a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts +++ b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts @@ -16,6 +16,7 @@ import { } from "@chainsafe/lodestar-beacon-state-transition"; import {toHexString} from "@chainsafe/ssz"; import {MapDef} from "../../util/map"; +import {intersectUint8Arrays, IntersectResult} from "../../util/bitArray"; import {pruneBySlot} from "./utils"; import {InsertOutcome} from "./types"; @@ -54,7 +55,7 @@ export class AggregatedAttestationPool { ); private lowestPermissibleSlot = 0; - add(attestation: phase0.Attestation, attestingIndices: ValidatorIndex[], committee: ValidatorIndex[]): InsertOutcome { + add(attestation: phase0.Attestation, attestingIndicesCount: number, committee: ValidatorIndex[]): InsertOutcome { const slot = attestation.data.slot; const lowestPermissibleSlot = this.lowestPermissibleSlot; @@ -73,7 +74,10 @@ export class AggregatedAttestationPool { attestationGroupByDataHash.set(dataRootHex, attestationGroup); } - return attestationGroup.add({attestation, attestingIndices: new Set(attestingIndices)}); + return attestationGroup.add({ + attestation, + trueBitsCount: attestingIndicesCount, + }); } /** Remove attestations which are too old to be included in a block. */ @@ -90,10 +94,8 @@ export class AggregatedAttestationPool { const stateSlot = state.slot; const stateEpoch = state.epochCtx.epoch; const statePrevEpoch = stateEpoch - 1; - const forkName = state.config.getForkName(stateSlot); - const getParticipationFn = - forkName === ForkName.phase0 ? this.getParticipationPhase0(state) : this.getParticipationAltair(state); + const getParticipation = getParticipationFn(state); const attestationsByScore: AttestationWithScore[] = []; @@ -128,7 +130,7 @@ export class AggregatedAttestationPool { ) { continue; } - const participation = getParticipationFn(epoch, attestationGroup.committee); + const participation = getParticipation(epoch, attestationGroup.committee); if (participation === null) { continue; } @@ -144,7 +146,7 @@ export class AggregatedAttestationPool { attestationsByScore.push( ...attestationGroup.getAttestationsForBlock(participation).map((attestation) => ({ attestation: attestation.attestation, - score: (attestation.notSeenAttesterCount ?? attestation.attestingIndices.size) / (stateSlot - slot), + score: attestation.notSeenAttesterCount / (stateSlot - slot), })) ); @@ -183,69 +185,20 @@ export class AggregatedAttestationPool { } return attestations; } - - /** - * Get attestations to be included in a phase0 block. - * As we are close to altair, this is not really important, it's mainly for e2e. - * The performance is not great due to the different BeaconState data structure to altair. - */ - private getParticipationPhase0(state: CachedBeaconStateAllForks): GetParticipationFn { - // check for phase0 block already - const phase0State = state as CachedBeaconStatePhase0; - const stateEpoch = computeEpochAtSlot(state.slot); - - const previousEpochParticipants = extractParticipation( - phase0State.previousEpochAttestations.getAllReadonly(), - state - ); - const currentEpochParticipants = extractParticipation(phase0State.currentEpochAttestations.getAllReadonly(), state); - - return (epoch: Epoch) => { - return epoch === stateEpoch - ? currentEpochParticipants - : epoch === stateEpoch - 1 - ? previousEpochParticipants - : null; - }; - } - - /** - * Get attestations to be included in an altair block. - * Attestations are sorted by inclusion distance then number of attesters. - * Attestations should pass the validation when processing attestations in beacon-state-transition. - */ - private getParticipationAltair(state: CachedBeaconStateAllForks): GetParticipationFn { - // check for altair block already - const altairState = state as CachedBeaconStateAltair; - const stateEpoch = computeEpochAtSlot(state.slot); - const previousParticipation = altairState.previousEpochParticipation.getAll(); - const currentParticipation = altairState.currentEpochParticipation.getAll(); - - return (epoch: Epoch, committee: number[]) => { - const participationStatus = - epoch === stateEpoch ? currentParticipation : epoch === stateEpoch - 1 ? previousParticipation : null; - - if (participationStatus === null) return null; - - const seenValidatorIndices = new Set(); - for (const validatorIndex of committee) { - if (flagIsTimelySource(participationStatus[validatorIndex])) { - seenValidatorIndices.add(validatorIndex); - } - } - return seenValidatorIndices; - }; - } } // eslint-disable-next-line @typescript-eslint/naming-convention interface AttestationWithIndex { attestation: phase0.Attestation; - attestingIndices: Set; + trueBitsCount: number; +} + +type AttestationNonParticipant = { + attestation: phase0.Attestation; // this is <= attestingIndices.count since some attesters may be seen by the chain // this is only updated and used in removeBySeenValidators function - notSeenAttesterCount?: number; -} + notSeenAttesterCount: number; +}; /** * Maintain a pool of AggregatedAttestation which all share the same AttestationData. @@ -265,64 +218,80 @@ export class MatchingDataAttestationGroup { * If it's a superset of an existing attestation, remove the existing attestation and add new. */ add(attestation: AttestationWithIndex): InsertOutcome { - const {attestingIndices} = attestation; - // preaggregate - let insertResult = InsertOutcome.NewData; + const newBits = attestation.attestation.aggregationBits; + const indicesToRemove = []; - for (const [i, existingAttestation] of this.attestations.entries()) { - const existingAttestingIndices = existingAttestation.attestingIndices; - const numIntersection = - // TODO: Intersect the uint8arrays from BitArray directly, it's probably much faster - existingAttestingIndices.size >= attestingIndices.size - ? intersection(existingAttestingIndices, attestingIndices) - : intersection(attestingIndices, existingAttestingIndices); - // no intersection - if (numIntersection === 0) { - aggregateInto(existingAttestation, attestation); - insertResult = InsertOutcome.Aggregated; - } else if (numIntersection === attestingIndices.size) { - // this new attestation is actually a subset of an existing one, don't want to add it - insertResult = InsertOutcome.AlreadyKnown; - } else if (numIntersection === existingAttestingIndices.size) { - // this new attestation is superset of an existing one, remove existing one - indicesToRemove.push(i); + + for (const [i, prevAttestation] of this.attestations.entries()) { + const prevBits = prevAttestation.attestation.aggregationBits; + + switch (intersectUint8Arrays(newBits.uint8Array, prevBits.uint8Array)) { + case IntersectResult.Subset: + case IntersectResult.Equal: + // this new attestation is actually a subset of an existing one, don't want to add it + return InsertOutcome.AlreadyKnown; + + case IntersectResult.Exclusive: + // no intersection + aggregateInto(prevAttestation, attestation); + return InsertOutcome.Aggregated; + + case IntersectResult.Superset: + // newBits superset of prevBits + // this new attestation is superset of an existing one, remove existing one + indicesToRemove.push(i); } } - if (insertResult === InsertOutcome.NewData) { - for (const index of indicesToRemove.reverse()) { - this.attestations.splice(index, 1); - } - this.attestations.push(attestation); - // Remove the attestations with less participation - if (this.attestations.length > MAX_RETAINED_ATTESTATIONS_PER_GROUP) { - this.attestations.sort((a, b) => b.attestingIndices.size - a.attestingIndices.size); - this.attestations.splice( - MAX_RETAINED_ATTESTATIONS_PER_GROUP, - this.attestations.length - MAX_RETAINED_ATTESTATIONS_PER_GROUP - ); - } + + // Added new data + for (const index of indicesToRemove.reverse()) { + // TODO: .splice performance warning + this.attestations.splice(index, 1); } - return insertResult; + + this.attestations.push(attestation); + + // Remove the attestations with less participation + if (this.attestations.length > MAX_RETAINED_ATTESTATIONS_PER_GROUP) { + this.attestations.sort((a, b) => b.trueBitsCount - a.trueBitsCount); + this.attestations.splice( + MAX_RETAINED_ATTESTATIONS_PER_GROUP, + this.attestations.length - MAX_RETAINED_ATTESTATIONS_PER_GROUP + ); + } + + return InsertOutcome.NewData; } - getAttestationsForBlock(seenAttestingIndices: Set): AttestationWithIndex[] { - const attestations: AttestationWithIndex[] = []; + getAttestationsForBlock(seenAttestingIndices: Set): AttestationNonParticipant[] { + const attestations: AttestationNonParticipant[] = []; + + const committeeLen = this.committee.length; + const committeeSeenAttesting = new Array(committeeLen); - for (const attestation of this.attestations) { + // Intersect committee with participation only once for all attestations + for (let i = 0; i < committeeLen; i++) { + committeeSeenAttesting[i] = seenAttestingIndices.has(this.committee[i]); + } + + for (const {attestation} of this.attestations) { + const {aggregationBits} = attestation; let notSeenAttesterCount = 0; - for (const attIndex of attestation.attestingIndices) { - if (!seenAttestingIndices.has(attIndex)) notSeenAttesterCount++; + + for (let i = 0; i < committeeLen; i++) { + // TODO: Optimize aggregationBits.get() in bulk for the entire BitArray + if (!committeeSeenAttesting[i] && aggregationBits.get(i)) { + notSeenAttesterCount++; + } } + if (notSeenAttesterCount > 0) { - attestations.push({...attestation, notSeenAttesterCount}); + attestations.push({attestation, notSeenAttesterCount}); } } return attestations - .sort( - (a, b) => - (b.notSeenAttesterCount ?? b.attestingIndices.size) - (a.notSeenAttesterCount ?? a.attestingIndices.size) - ) + .sort((a, b) => b.notSeenAttesterCount - a.notSeenAttesterCount) .slice(0, MAX_ATTESTATIONS_PER_GROUP); } @@ -333,10 +302,6 @@ export class MatchingDataAttestationGroup { } export function aggregateInto(attestation1: AttestationWithIndex, attestation2: AttestationWithIndex): void { - for (const attIndex of attestation2.attestingIndices) { - attestation1.attestingIndices.add(attIndex); - } - // Merge bits of attestation2 into attestation1 attestation1.attestation.aggregationBits.mergeOrWith(attestation2.attestation.aggregationBits); @@ -345,6 +310,62 @@ export function aggregateInto(attestation1: AttestationWithIndex, attestation2: attestation1.attestation.signature = bls.Signature.aggregate([signature1, signature2]).toBytes(); } +/** + * Pre-compute participation from a CachedBeaconStateAllForks, for use to check if an attestation's committee + * has already attested or not. + */ +export function getParticipationFn(state: CachedBeaconStateAllForks): GetParticipationFn { + if (state.config.getForkName(state.slot) === ForkName.phase0) { + // Get attestations to be included in a phase0 block. + // As we are close to altair, this is not really important, it's mainly for e2e. + // The performance is not great due to the different BeaconState data structure to altair. + // check for phase0 block already + const phase0State = state as CachedBeaconStatePhase0; + const stateEpoch = computeEpochAtSlot(state.slot); + + const previousEpochParticipants = extractParticipation( + phase0State.previousEpochAttestations.getAllReadonly(), + state + ); + const currentEpochParticipants = extractParticipation(phase0State.currentEpochAttestations.getAllReadonly(), state); + + return (epoch: Epoch) => { + return epoch === stateEpoch + ? currentEpochParticipants + : epoch === stateEpoch - 1 + ? previousEpochParticipants + : null; + }; + } + + // altair and future forks + else { + // Get attestations to be included in an altair block. + // Attestations are sorted by inclusion distance then number of attesters. + // Attestations should pass the validation when processing attestations in beacon-state-transition. + // check for altair block already + const altairState = state as CachedBeaconStateAltair; + const previousParticipation = altairState.previousEpochParticipation.getAll(); + const currentParticipation = altairState.currentEpochParticipation.getAll(); + const stateEpoch = computeEpochAtSlot(state.slot); + + return (epoch: Epoch, committee: number[]) => { + const participationStatus = + epoch === stateEpoch ? currentParticipation : epoch === stateEpoch - 1 ? previousParticipation : null; + + if (participationStatus === null) return null; + + const seenValidatorIndices = new Set(); + for (const validatorIndex of committee) { + if (flagIsTimelySource(participationStatus[validatorIndex])) { + seenValidatorIndices.add(validatorIndex); + } + } + return seenValidatorIndices; + }; + } +} + export function extractParticipation( attestations: phase0.PendingAttestation[], state: CachedBeaconStateAllForks @@ -365,14 +386,6 @@ export function extractParticipation( return allParticipants; } -export function intersection(bigSet: Set, smallSet: Set): number { - let numIntersection = 0; - for (const validatorIndex of smallSet) { - if (bigSet.has(validatorIndex)) numIntersection++; - } - return numIntersection; -} - /** * The state transition accepts incorrect target and head attestations. * We only need to validate the source checkpoint. diff --git a/packages/lodestar/src/network/gossip/handlers/index.ts b/packages/lodestar/src/network/gossip/handlers/index.ts index 51c15bf5dc60..f7982f4452bd 100644 --- a/packages/lodestar/src/network/gossip/handlers/index.ts +++ b/packages/lodestar/src/network/gossip/handlers/index.ts @@ -1,7 +1,7 @@ import {toHexString} from "@chainsafe/ssz"; import PeerId from "peer-id"; import {IBeaconConfig} from "@chainsafe/lodestar-config"; -import {phase0, ssz, ValidatorIndex} from "@chainsafe/lodestar-types"; +import {phase0, ssz} from "@chainsafe/lodestar-types"; import {ILogger, prettyBytes} from "@chainsafe/lodestar-utils"; import {IMetrics} from "../../../metrics"; import {OpSource} from "../../../metrics/validatorMonitor"; @@ -168,7 +168,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.aggregatedAttestationPool.add( aggregatedAttestation, - indexedAttestation.attestingIndices as ValidatorIndex[], + indexedAttestation.attestingIndices.length, committeeIndices ); diff --git a/packages/lodestar/src/util/bitArray.ts b/packages/lodestar/src/util/bitArray.ts new file mode 100644 index 000000000000..ab56c2b6b060 --- /dev/null +++ b/packages/lodestar/src/util/bitArray.ts @@ -0,0 +1,74 @@ +export enum IntersectResult { + Equal, + /** All elements in set B are in set A */ + Superset, + /** All elements in set A are in set B */ + Subset, + /** Set A and set B do not share any elements */ + Exclusive, + /** Set A and set B intersect but are not superset or subset */ + Intersect, +} + +/** + * For each byte check if a includes b, + * | a | b | result | + * | -------- | -------- | ------------- | + * | 00001111 | 00001111 | A equals B | + * | 00001111 | 00000011 | A superset B | + * | 00000011 | 00001111 | A subset B | + * | 11110000 | 00001111 | A exclude B | + * | 11111100 | 00111111 | A intersect B | + * + * For all bytes in BitArray: + * - equals = MAYBE ONLY equals + * - excludes = MUST ONLY equals + * - superset = MUST superset MAYBE equal + * - subset = MUST subset MAYBE equal + * - intersect = any other condition + */ +export function intersectUint8Arrays(aUA: Uint8Array, bUA: Uint8Array): IntersectResult { + const len = aUA.length; + + let someEquals = false; + let someExcludes = false; + let someSuperset = false; + let someSubset = false; + + for (let i = 0; i < len; i++) { + const a = aUA[i]; + const b = bUA[i]; + + if (a === 0 && b === 0) { + // zero, skip + } else if (a === b) { + // A equals B + someEquals = true; + } else if ((a & b) === 0) { + // A excludes B + someExcludes = true; + } else if ((a & b) === b) { + // A superset B + if (someSubset) return IntersectResult.Intersect; + someSuperset = true; + } else if ((a & b) === a) { + // A subset B + if (someSuperset) return IntersectResult.Intersect; + someSubset = true; + } else { + // A diff B + return IntersectResult.Intersect; + } + } + + // equals = MAYBE ONLY equals + if (!someExcludes && !someSuperset && !someSubset) return IntersectResult.Equal; + // excludes = MUST ONLY equals + if (!someEquals && someExcludes && !someSuperset && !someSubset) return IntersectResult.Exclusive; + // superset = MUST superset MAYBE equal + if (!someExcludes && someSuperset && !someSubset) return IntersectResult.Superset; + // subset = MUST subset MAYBE equal + if (!someExcludes && !someSuperset && someSubset) return IntersectResult.Subset; + // intersect = any other condition + else return IntersectResult.Intersect; +} diff --git a/packages/lodestar/test/perf/chain/opPools/aggregatedAttestationPool.test.ts b/packages/lodestar/test/perf/chain/opPools/aggregatedAttestationPool.test.ts index 01166df9e9b2..4c699937a206 100644 --- a/packages/lodestar/test/perf/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/lodestar/test/perf/chain/opPools/aggregatedAttestationPool.test.ts @@ -77,7 +77,7 @@ function getAggregatedAttestationPool(state: CachedBeaconStateAltair): Aggregate const committee = state.epochCtx.getBeaconCommittee(slot, committeeIndex); // all attestation has full participation so getAttestationsForBlock() has to do a lot of filter // aggregate_and_proof messages - pool.add(attestation, committee, committee); + pool.add(attestation, committee.length, committee); } } return pool; diff --git a/packages/lodestar/test/perf/util/bitArray.test.ts b/packages/lodestar/test/perf/util/bitArray.test.ts new file mode 100644 index 000000000000..1a2bf72f236f --- /dev/null +++ b/packages/lodestar/test/perf/util/bitArray.test.ts @@ -0,0 +1,54 @@ +import {BitArray} from "@chainsafe/ssz"; +import {itBench, setBenchOpts} from "@dapplion/benchmark"; +import {intersectUint8Arrays} from "../../../src/util/bitArray"; + +/** + * 16_000 items: push then shift - LinkedList is >200x faster than regular array + * push then pop - LinkedList is >10x faster than regular array + * 24_000 items: push then shift - LinkedList is >350x faster than regular array + * push then pop - LinkedList is >10x faster than regular array + */ +describe("Intersect BitArray vs Array+Set", () => { + setBenchOpts({noThreshold: true}); + + for (const bitLen of [8 * 1, 8 * 16]) { + const aBA = BitArray.fromBoolArray(Array.from({length: bitLen}, (_, i) => i % 2 === 0)); + const bBA = BitArray.fromBoolArray(Array.from({length: bitLen}, (_, i) => i % 4 === 0)); + + itBench({ + id: `intersect bitArray bitLen ${bitLen}`, + runsFactor: 1000, + fn: () => { + for (let i = 0; i < 1000; i++) { + intersectUint8Arrays(aBA.uint8Array, bBA.uint8Array); + } + }, + }); + + const setValid = new Set(linspace(0, bitLen, 2)); + const indices = Array.from({length: bitLen}, (_, i) => i); + + itBench({ + id: `intersect array and set length ${bitLen}`, + runsFactor: 1000, + fn: () => { + for (let i = 0; i < 1000; i++) { + const intersected: number[] = []; + for (let i = 0; i < indices.length; i++) { + if (setValid.has(indices[i])) { + intersected.push(indices[i]); + } + } + } + }, + }); + } +}); + +function linspace(start: number, end: number, step: number): number[] { + const arr: number[] = []; + for (let i = start; i < end; i += step) { + arr.push(i); + } + return arr; +} diff --git a/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts index 9c1e25d60213..37e560c31ebb 100644 --- a/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts +++ b/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts @@ -1,5 +1,5 @@ import {bls, SecretKey} from "@chainsafe/bls"; -import {BitArray} from "@chainsafe/ssz"; +import {BitArray, fromHexString} from "@chainsafe/ssz"; import {initBLS} from "@chainsafe/lodestar-cli/src/util"; import {createIChainForkConfig, defaultChainConfig} from "@chainsafe/lodestar-config"; import {CachedBeaconStateAllForks} from "@chainsafe/lodestar-beacon-state-transition"; @@ -9,13 +9,19 @@ import {ssz, phase0} from "@chainsafe/lodestar-types"; import { AggregatedAttestationPool, aggregateInto, + getParticipationFn, MatchingDataAttestationGroup, } from "../../../../src/chain/opPools/aggregatedAttestationPool"; import {InsertOutcome} from "../../../../src/chain/opPools/types"; +import {linspace} from "../../../../src/util/numpy"; import {generateAttestation, generateEmptyAttestation} from "../../../utils/attestation"; import {generateCachedState} from "../../../utils/state"; import {renderBitArray} from "../../../utils/render"; -import sinon from "sinon"; + +/** Valid signature of random data to prevent BLS errors */ +const validSignature = fromHexString( + "0xb2afb700f6c561ce5e1b4fedaec9d7c06b822d38c720cf588adfda748860a940adf51634b6788f298c552de40183b5a203b2bbe8b7dd147f0bb5bc97080a12efbb631c8888cb31a99cc4706eb3711865b8ea818c10126e4d818b542e9dbf9ae8" +); describe("AggregatedAttestationPool", function () { let pool: AggregatedAttestationPool; @@ -38,22 +44,27 @@ describe("AggregatedAttestationPool", function () { altairState = originalState.clone(); }); - this.afterEach(() => { - sinon.restore(); + it("getParticipationFn", () => { + // previousEpochParticipation and currentEpochParticipation is created inside generateCachedState + // 0 and 1 are fully participated + const participationFn = getParticipationFn(altairState); + const participation = participationFn(currentEpoch, committee); + expect(participation).to.deep.equal(new Set([0, 1]), "Wrong participation set"); }); // previousEpochParticipation and currentEpochParticipation is created inside generateCachedState // 0 and 1 are fully participated - const testCases: {name: string; attestingIndices: number[]; expected: phase0.Attestation[]}[] = [ - {name: "all validators are seen", attestingIndices: [0, 1], expected: []}, - {name: "all validators are NOT seen", attestingIndices: [2, 3], expected: [attestation]}, - {name: "one is seen and one is NOT", attestingIndices: [1, 2], expected: [attestation]}, + const testCases: {name: string; attestingBits: number[]; isReturned: boolean}[] = [ + {name: "all validators are seen", attestingBits: [0b00000011], isReturned: false}, + {name: "all validators are NOT seen", attestingBits: [0b00001100], isReturned: true}, + {name: "one is seen and one is NOT", attestingBits: [0b00001100], isReturned: true}, ]; - for (const {name, attestingIndices, expected} of testCases) { + for (const {name, attestingBits, isReturned} of testCases) { it(name, function () { - pool.add(attestation, attestingIndices, committee); - expect(pool.getAttestationsForBlock(altairState)).to.be.deep.equal(expected, "incorrect returned attestations"); + const aggregationBits = new BitArray(new Uint8Array(attestingBits), 8); + pool.add({...attestation, aggregationBits}, aggregationBits.getTrueBitIndexes().length, committee); + expect(pool.getAttestationsForBlock(altairState).length > 0).to.equal(isReturned, "Wrong attestation isReturned"); }); } @@ -61,135 +72,146 @@ describe("AggregatedAttestationPool", function () { altairState.currentJustifiedCheckpoint.epoch = 1000; // all attesters are not seen const attestingIndices = [2, 3]; - pool.add(attestation, attestingIndices, committee); + pool.add(attestation, attestingIndices.length, committee); expect(pool.getAttestationsForBlock(altairState)).to.be.deep.equal([], "no attestation since incorrect source"); }); }); -describe("MatchingDataAttestationGroup", function () { - let attestationGroup: MatchingDataAttestationGroup; - const committee = [100, 200, 300]; - const attestationSeed = generateEmptyAttestation(); - const attestationDataRoot = ssz.phase0.AttestationData.serialize(attestationSeed.data); - let sk1: SecretKey; - const attestation1: phase0.Attestation = { - ...attestationSeed, - ...{aggregationBits: BitArray.fromBoolArray([true, true, false])}, - }; +describe("MatchingDataAttestationGroup.add()", () => { + const testCases: {id: string; attestationsToAdd: {bits: number[]; res: InsertOutcome; isKept: boolean}[]}[] = [ + { + id: "2 intersecting", + attestationsToAdd: [ + {bits: [0b11111100], res: InsertOutcome.NewData, isKept: true}, + {bits: [0b00111111], res: InsertOutcome.NewData, isKept: true}, + ], + }, + { + id: "New is superset", + attestationsToAdd: [ + {bits: [0b11111100], res: InsertOutcome.NewData, isKept: false}, + {bits: [0b11111111], res: InsertOutcome.NewData, isKept: true}, + ], + }, + { + id: "New is subset", + attestationsToAdd: [ + {bits: [0b11111111], res: InsertOutcome.NewData, isKept: true}, + {bits: [0b11111100], res: InsertOutcome.AlreadyKnown, isKept: false}, + ], + }, + { + id: "Aggregated", + attestationsToAdd: [ + // Attestation 0 is kept because it's mutated in place to aggregate attestation 1 + {bits: [0b00001111], res: InsertOutcome.NewData, isKept: true}, + {bits: [0b11110000], res: InsertOutcome.Aggregated, isKept: false}, + ], + // Corectly aggregating the resulting att is checked in "MatchingDataAttestationGroup aggregateInto" test + }, + ]; - function attestationFromBits(bitsBoolArr: boolean[]): phase0.Attestation { - return {...attestationSeed, ...{aggregationBits: BitArray.fromBoolArray(bitsBoolArr)}}; - } + const attestationData = generateEmptyAttestation().data; + const committee = linspace(0, 7); - before(async () => { - await initBLS(); - sk1 = bls.SecretKey.fromBytes(Buffer.alloc(32, 1)); - attestation1.signature = sk1.sign(attestationDataRoot).toBytes(); - }); + for (const {id, attestationsToAdd} of testCases) { + it(id, () => { + const attestationGroup = new MatchingDataAttestationGroup(committee, attestationData); - beforeEach(() => { - attestationGroup = new MatchingDataAttestationGroup(committee, attestation1.data); - attestationGroup.add({ - attestation: attestation1, - attestingIndices: new Set([100, 200]), - }); - }); + const attestations = attestationsToAdd.map( + ({bits}): phase0.Attestation => ({ + data: attestationData, + aggregationBits: new BitArray(new Uint8Array(bits), 8), + signature: validSignature, + }) + ); - it("add - new data, getAttestations() return 2", () => { - const attestation2 = attestationFromBits([true, false, true]); - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])}); - expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestations(); - expect(attestations).to.be.deep.equal([attestation1, attestation2], "Incorrect attestations for block"); - }); + const results = attestations.map((attestation) => + attestationGroup.add({attestation, trueBitsCount: attestation.aggregationBits.getTrueBitIndexes().length}) + ); - it("add - new data, remove existing attestation, getAttestations() return 1", () => { - const attestation2 = attestationFromBits([true, true, true]); - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set(committee)}); - expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestations(); - expect(attestations).to.be.deep.equal([attestation2], "should return only new attestation"); - }); + expect(results).to.deep.equal( + attestationsToAdd.map((e) => e.res), + "Wrong InsertOutcome results" + ); - it("add - already known, getAttestations() return 1", () => { - const attestation2 = attestationFromBits([true, false, false]); - // attestingIndices is subset of an existing one - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100])}); - expect(result).to.be.equal(InsertOutcome.AlreadyKnown, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestations(); - expect(attestations).to.be.deep.equal([attestation1], "expect exactly 1 attestation"); - }); + const attestationsAfterAdding = attestationGroup.getAttestations(); - it("add - aggregate into existing attestation, getAttestations() return 1", () => { - const attestation2 = attestationFromBits([false, false, true]); - const sk2 = bls.SecretKey.fromBytes(Buffer.alloc(32, 2)); - attestation2.signature = sk2.sign(attestationDataRoot).toBytes(); - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([300])}); - expect(result).to.be.equal(InsertOutcome.Aggregated, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestations(); - expect(attestations.length).to.be.equal(1, "expect exactly 1 aggregated attestation"); - expect(renderBitArray(attestations[0].aggregationBits)).to.be.deep.equal( - renderBitArray(BitArray.fromBoolArray([true, true, true])), - "incorrect aggregationBits" - ); - const aggregatedSignature = bls.Signature.fromBytes(attestations[0].signature, undefined, true); - expect( - aggregatedSignature.verifyAggregate([sk1.toPublicKey(), sk2.toPublicKey()], attestationDataRoot) - ).to.be.equal(true, "invalid aggregated signature"); - expect(attestations[0].data).to.be.deep.equal(attestation1.data, "incorrect AttestationData"); - }); - - it("getAttestationsForBlock - return 0", () => { - const attestations = attestationGroup.getAttestationsForBlock(new Set(committee)); - expect(attestations).to.be.deep.equal([], "all attesters are seen, should remove empty"); - }); + for (const [i, {isKept}] of attestationsToAdd.entries()) { + expect(attestationsAfterAdding.indexOf(attestations[i]) >= 0).to.equal(isKept, `Wrong attestation ${i} isKept`); + } + }); + } +}); - it("getAttestationsForBlock - return 1", () => { - const attestations = attestationGroup.getAttestationsForBlock(new Set([200])); - expect(attestations).to.be.deep.equal( - [ - { - attestation: attestation1, - attestingIndices: new Set([100, 200]), - notSeenAttesterCount: 1, - }, +describe("MatchingDataAttestationGroup.getAttestationsForBlock", () => { + const testCases: { + id: string; + seenAttestingBits: number[]; + attestationsToAdd: {bits: number[]; notSeenAttesterCount: number}[]; + }[] = [ + // Note: attestationsToAdd MUST intersect in order to not be aggregated and distort the results + { + id: "All have attested", + seenAttestingBits: [0b11111111], + attestationsToAdd: [ + {bits: [0b11111110], notSeenAttesterCount: 0}, + {bits: [0b00000011], notSeenAttesterCount: 0}, ], - "incorrect attestations" - ); - }); - - it("getAttestationsForBlock - return 2", () => { - const attestation2 = attestationFromBits([true, false, true]); - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])}); - expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestationsForBlock(new Set([200])); - expect(attestations).to.be.deep.equal( - [ - { - attestation: attestation2, - attestingIndices: new Set([100, 300]), - notSeenAttesterCount: 2, - }, - { - attestation: attestation1, - attestingIndices: new Set([100, 200]), - notSeenAttesterCount: 1, - }, + }, + { + id: "Some have attested", + seenAttestingBits: [0b11110001], // equals to indexes [ 0, 4, 5, 6, 7 ] + attestationsToAdd: [ + {bits: [0b11111110], notSeenAttesterCount: 3}, + {bits: [0b00000011], notSeenAttesterCount: 1}, ], - "incorrect attestations" - ); - }); + }, + { + id: "Non have attested", + seenAttestingBits: [0b00000000], + attestationsToAdd: [ + {bits: [0b11111110], notSeenAttesterCount: 7}, + {bits: [0b00000011], notSeenAttesterCount: 2}, + ], + }, + ]; - it("getAttestations", () => { - const attestation2 = attestationFromBits([true, false, true]); - const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])}); - expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome"); - const attestations = attestationGroup.getAttestations(); - expect(attestations).to.be.deep.equal([attestation1, attestation2]); - }); + const attestationData = generateEmptyAttestation().data; + const committee = linspace(0, 7); + + for (const {id, seenAttestingBits, attestationsToAdd} of testCases) { + it(id, () => { + const attestationGroup = new MatchingDataAttestationGroup(committee, attestationData); + + const attestations = attestationsToAdd.map( + ({bits}): phase0.Attestation => ({ + data: attestationData, + aggregationBits: new BitArray(new Uint8Array(bits), 8), + signature: validSignature, + }) + ); + + for (const attestation of attestations) { + attestationGroup.add({attestation, trueBitsCount: attestation.aggregationBits.getTrueBitIndexes().length}); + } + + const indices = new BitArray(new Uint8Array(seenAttestingBits), 8).intersectValues(committee); + const attestationsForBlock = attestationGroup.getAttestationsForBlock(new Set(indices)); + + for (const [i, {notSeenAttesterCount}] of attestationsToAdd.entries()) { + const attestation = attestationsForBlock.find((a) => a.attestation === attestations[i]); + // If notSeenAttesterCount === 0 the attestation is not returned + expect(attestation ? attestation.notSeenAttesterCount : 0).to.equal( + notSeenAttesterCount, + `attestation ${i} wrong returned notSeenAttesterCount` + ); + } + }); + } }); -describe("aggregateInto", function () { +describe("MatchingDataAttestationGroup aggregateInto", function () { const attestationSeed = generateEmptyAttestation(); const attestation1 = {...attestationSeed, ...{aggregationBits: BitArray.fromBoolArray([false, true])}}; const attestation2 = {...attestationSeed, ...{aggregationBits: BitArray.fromBoolArray([true, false])}}; @@ -197,6 +219,7 @@ describe("aggregateInto", function () { const attestationDataRoot = ssz.phase0.AttestationData.serialize(attestationSeed.data); let sk1: SecretKey; let sk2: SecretKey; + before("Init BLS", async () => { await initBLS(); sk1 = bls.SecretKey.fromBytes(Buffer.alloc(32, 1)); @@ -206,13 +229,10 @@ describe("aggregateInto", function () { }); it("should aggregate 2 attestations", () => { - const attWithIndex1 = {attestation: attestation1, attestingIndices: new Set([100])}; - const attWithIndex2 = {attestation: attestation2, attestingIndices: new Set([200])}; + const attWithIndex1 = {attestation: attestation1, trueBitsCount: 1}; + const attWithIndex2 = {attestation: attestation2, trueBitsCount: 1}; aggregateInto(attWithIndex1, attWithIndex2); - expect(setToArr(attWithIndex1.attestingIndices)).to.be.deep.equal( - [100, 200], - "invalid aggregated attestingIndices" - ); + expect(renderBitArray(attWithIndex1.attestation.aggregationBits)).to.be.deep.equal( renderBitArray(mergedBitArray), "invalid aggregationBits" @@ -223,7 +243,3 @@ describe("aggregateInto", function () { ).to.be.equal(true, "invalid aggregated signature"); }); }); - -function setToArr(set: Set): T[] { - return Array.from(set.values()); -} diff --git a/packages/lodestar/test/unit/util/bitArray.test.ts b/packages/lodestar/test/unit/util/bitArray.test.ts new file mode 100644 index 000000000000..7fa0a414c7c7 --- /dev/null +++ b/packages/lodestar/test/unit/util/bitArray.test.ts @@ -0,0 +1,70 @@ +import {expect} from "chai"; +import {IntersectResult, intersectUint8Arrays} from "../../../src/util/bitArray"; + +describe("util / bitArray / intersectUint8Arrays", () => { + const testCases: {id?: string; a: number[]; b: number[]; res: IntersectResult}[] = [ + // Single byte + {a: [0b00000000], b: [0b00000000], res: IntersectResult.Equal}, + {a: [0b00001111], b: [0b00001111], res: IntersectResult.Equal}, + {a: [0b00001111], b: [0b00000011], res: IntersectResult.Superset}, + {a: [0b00000011], b: [0b00001111], res: IntersectResult.Subset}, + {a: [0b00000011], b: [0b11110000], res: IntersectResult.Exclusive}, + {a: [0b00111111], b: [0b11111100], res: IntersectResult.Intersect}, + // Multi-byte + { + a: [0b00000000, 0b00000000, 0b00001111, 0b00001111], + b: [0b00000000, 0b00000000, 0b00001111, 0b00001111], + res: IntersectResult.Equal, + }, + { + // zero equal superset superset + a: [0b00000000, 0b11111111, 0b11111111, 0b11110000], + b: [0b00000000, 0b11111111, 0b00111100, 0b11000000], + res: IntersectResult.Superset, + }, + { + // zero equal subset subset + a: [0b00000000, 0b11111111, 0b00111100, 0b11000000], + b: [0b00000000, 0b11111111, 0b11111111, 0b11110000], + res: IntersectResult.Subset, + }, + { + // zero exclusive exclusive zero + a: [0b00000000, 0b00001111, 0b11110000, 0b00000000], + b: [0b00000000, 0b11110000, 0b00001111, 0b00000000], + res: IntersectResult.Exclusive, + }, + { + // zero equal superset subset + a: [0b00000000, 0b11111111, 0b11111111, 0b11000000], + b: [0b00000000, 0b11111111, 0b00111100, 0b11110000], + res: IntersectResult.Intersect, + }, + { + // zero equal exclusive exclusive + a: [0b00000000, 0b11111111, 0b00001111, 0b11110000], + b: [0b00000000, 0b11111111, 0b11110000, 0b00001111], + res: IntersectResult.Intersect, + }, + { + // zero superset exclusive exclusive + a: [0b00000000, 0b11111111, 0b00001111, 0b11110000], + b: [0b00000000, 0b00111100, 0b11110000, 0b00001111], + res: IntersectResult.Intersect, + }, + ]; + + for (const {id, a, b, res} of testCases) { + it(id ?? toId(a, b), () => { + const aUA = new Uint8Array(a); + const bUA = new Uint8Array(b); + + // Use IntersectResult[] to get the actual name of IntersectResult + expect(IntersectResult[intersectUint8Arrays(aUA, bUA)]).to.equal(IntersectResult[res]); + }); + } +}); + +function toId(a: number[], b: number[]): string { + return [a, b].map((arr) => arr.map((n) => n.toString(2).padStart(8, "0")).join(",")).join(" - "); +}