From 385788b3c2fbffb2b61555cac8613490451d98ae Mon Sep 17 00:00:00 2001
From: dapplion <35266934+dapplion@users.noreply.github.com>
Date: Tue, 17 May 2022 13:45:01 +0700
Subject: [PATCH 1/5] Use BitArrays for aggregate merging

---
 .../lodestar/src/api/impl/validator/index.ts  |   6 +-
 .../opPools/aggregatedAttestationPool.ts      | 126 ++++++++++--------
 .../src/network/gossip/handlers/index.ts      |   8 +-
 packages/lodestar/src/util/bitArray.ts        |  64 +++++++++
 4 files changed, 141 insertions(+), 63 deletions(-)
 create mode 100644 packages/lodestar/src/util/bitArray.ts

diff --git a/packages/lodestar/src/api/impl/validator/index.ts b/packages/lodestar/src/api/impl/validator/index.ts
index 06b7b1d72723..39b6766c2e1f 100644
--- a/packages/lodestar/src/api/impl/validator/index.ts
+++ b/packages/lodestar/src/api/impl/validator/index.ts
@@ -439,11 +439,7 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:
               signedAggregateAndProof
             );
 
-            chain.aggregatedAttestationPool.add(
-              signedAggregateAndProof.message.aggregate,
-              indexedAttestation.attestingIndices,
-              committeeIndices
-            );
+            chain.aggregatedAttestationPool.add(signedAggregateAndProof.message.aggregate, committeeIndices);
             const sentPeers = await network.gossip.publishBeaconAggregateAndProof(signedAggregateAndProof);
             metrics?.submitAggregatedAttestation(seenTimestampSec, indexedAttestation, sentPeers);
           } catch (e) {
diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
index 8ecd286c036d..5fda7290ca0a 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 {intersectBitArrays, 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, 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: attestation.aggregationBits.getTrueBitIndexes().length,
+    });
   }
 
   /** Remove attestations which are too old to be included in a block. */
@@ -144,7 +148,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),
           }))
         );
 
@@ -241,12 +245,19 @@ export class AggregatedAttestationPool {
 // eslint-disable-next-line @typescript-eslint/naming-convention
 interface AttestationWithIndex {
   attestation: phase0.Attestation;
-  attestingIndices: Set<ValidatorIndex>;
+  trueBitsCount: number;
   // this is <= attestingIndices.count since some attesters may be seen by the chain
   // this is only updated and used in removeBySeenValidators function
   notSeenAttesterCount?: 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;
+};
+
 /**
  * Maintain a pool of AggregatedAttestation which all share the same AttestationData.
  * Preaggregate into smallest number of attestations.
@@ -265,64 +276,79 @@ 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 (intersectBitArrays(newBits, prevBits)) {
+        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.Exclude:
+          // 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<ValidatorIndex>): AttestationWithIndex[] {
-    const attestations: AttestationWithIndex[] = [];
+  getAttestationsForBlock(seenAttestingIndices: Set<ValidatorIndex>): AttestationNonParticipant[] {
+    const attestations: AttestationNonParticipant[] = [];
+
+    const committeeLen = this.committee.length;
+    const committeeSeenAttesting = new Array<boolean>(committeeLen);
+
+    // 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) {
+    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++) {
+        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 +359,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);
 
diff --git a/packages/lodestar/src/network/gossip/handlers/index.ts b/packages/lodestar/src/network/gossip/handlers/index.ts
index 51c15bf5dc60..26554ad4e957 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";
@@ -166,11 +166,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH
       metrics?.registerGossipAggregatedAttestation(seenTimestampSec, signedAggregateAndProof, indexedAttestation);
       const aggregatedAttestation = signedAggregateAndProof.message.aggregate;
 
-      chain.aggregatedAttestationPool.add(
-        aggregatedAttestation,
-        indexedAttestation.attestingIndices as ValidatorIndex[],
-        committeeIndices
-      );
+      chain.aggregatedAttestationPool.add(aggregatedAttestation, committeeIndices);
 
       if (!options.dontSendGossipAttestationsToForkchoice) {
         try {
diff --git a/packages/lodestar/src/util/bitArray.ts b/packages/lodestar/src/util/bitArray.ts
new file mode 100644
index 000000000000..3517858147fd
--- /dev/null
+++ b/packages/lodestar/src/util/bitArray.ts
@@ -0,0 +1,64 @@
+import {BitArray} from "@chainsafe/ssz";
+
+export enum IntersectResult {
+  Equal,
+  Superset,
+  Subset,
+  Exclude,
+  Diff,
+}
+
+/**
+ * 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 diff B     |
+ *
+ * For all bytes in BitArray:
+ * - equals = (equals)[]
+ * - excludes = (excludes)[]
+ * - superset = (Superset | equal)[]
+ * - subset = (Subset | equal)[]
+ * - diff = (diff | *)[]
+ */
+export function intersectBitArrays(aBA: BitArray, bBA: BitArray): IntersectResult {
+  const aUA = aBA.uint8Array;
+  const bUA = bBA.uint8Array;
+  const len = aBA.uint8Array.length;
+
+  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 === b) {
+      // A equals B
+    } else if ((a & b) === 0) {
+      // A excludes B
+      someExcludes = true;
+    } else if ((a & b) === a) {
+      // A superset B
+      if (someSubset) return IntersectResult.Diff;
+      someSuperset = true;
+    } else if ((a & b) === b) {
+      // A subset B
+      if (someSuperset) return IntersectResult.Diff;
+      someSubset = true;
+    } else {
+      // A diff B
+      return IntersectResult.Diff;
+    }
+  }
+
+  if (!someExcludes && !someSuperset && !someSubset) return IntersectResult.Equal;
+  if (someExcludes && !someSuperset && !someSubset) return IntersectResult.Exclude;
+  if (!someExcludes && someSuperset && !someSubset) return IntersectResult.Superset;
+  if (!someExcludes && !someSuperset && someSubset) return IntersectResult.Subset;
+  else return IntersectResult.Diff;
+}

From 05f91255ff3b67d1e0f35d41f25a80c97d1f3573 Mon Sep 17 00:00:00 2001
From: dapplion <35266934+dapplion@users.noreply.github.com>
Date: Tue, 17 May 2022 23:59:54 +0700
Subject: [PATCH 2/5] Test intersectUint8Arrays

---
 .../opPools/aggregatedAttestationPool.ts      |  6 +-
 packages/lodestar/src/util/bitArray.ts        | 64 ++++++++++-------
 .../lodestar/test/perf/util/bitArray.test.ts  | 54 ++++++++++++++
 .../lodestar/test/unit/util/bitArray.test.ts  | 70 +++++++++++++++++++
 4 files changed, 164 insertions(+), 30 deletions(-)
 create mode 100644 packages/lodestar/test/perf/util/bitArray.test.ts
 create mode 100644 packages/lodestar/test/unit/util/bitArray.test.ts

diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
index 5fda7290ca0a..c1c807e72ab8 100644
--- a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
+++ b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
@@ -16,7 +16,7 @@ import {
 } from "@chainsafe/lodestar-beacon-state-transition";
 import {toHexString} from "@chainsafe/ssz";
 import {MapDef} from "../../util/map";
-import {intersectBitArrays, IntersectResult} from "../../util/bitArray";
+import {intersectUint8Arrays, IntersectResult} from "../../util/bitArray";
 import {pruneBySlot} from "./utils";
 import {InsertOutcome} from "./types";
 
@@ -283,13 +283,13 @@ export class MatchingDataAttestationGroup {
     for (const [i, prevAttestation] of this.attestations.entries()) {
       const prevBits = prevAttestation.attestation.aggregationBits;
 
-      switch (intersectBitArrays(newBits, prevBits)) {
+      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.Exclude:
+        case IntersectResult.Exclusive:
           // no intersection
           aggregateInto(prevAttestation, attestation);
           return InsertOutcome.Aggregated;
diff --git a/packages/lodestar/src/util/bitArray.ts b/packages/lodestar/src/util/bitArray.ts
index 3517858147fd..ab56c2b6b060 100644
--- a/packages/lodestar/src/util/bitArray.ts
+++ b/packages/lodestar/src/util/bitArray.ts
@@ -1,34 +1,36 @@
-import {BitArray} from "@chainsafe/ssz";
-
 export enum IntersectResult {
   Equal,
+  /** All elements in set B are in set A  */
   Superset,
+  /** All elements in set A are in set B  */
   Subset,
-  Exclude,
-  Diff,
+  /** 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 diff 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 = (equals)[]
- * - excludes = (excludes)[]
- * - superset = (Superset | equal)[]
- * - subset = (Subset | equal)[]
- * - diff = (diff | *)[]
+ * - equals = MAYBE ONLY equals
+ * - excludes = MUST ONLY equals
+ * - superset = MUST superset MAYBE equal
+ * - subset = MUST subset MAYBE equal
+ * - intersect = any other condition
  */
-export function intersectBitArrays(aBA: BitArray, bBA: BitArray): IntersectResult {
-  const aUA = aBA.uint8Array;
-  const bUA = bBA.uint8Array;
-  const len = aBA.uint8Array.length;
+export function intersectUint8Arrays(aUA: Uint8Array, bUA: Uint8Array): IntersectResult {
+  const len = aUA.length;
 
+  let someEquals = false;
   let someExcludes = false;
   let someSuperset = false;
   let someSubset = false;
@@ -37,28 +39,36 @@ export function intersectBitArrays(aBA: BitArray, bBA: BitArray): IntersectResul
     const a = aUA[i];
     const b = bUA[i];
 
-    if (a === b) {
+    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) === a) {
+    } else if ((a & b) === b) {
       // A superset B
-      if (someSubset) return IntersectResult.Diff;
+      if (someSubset) return IntersectResult.Intersect;
       someSuperset = true;
-    } else if ((a & b) === b) {
+    } else if ((a & b) === a) {
       // A subset B
-      if (someSuperset) return IntersectResult.Diff;
+      if (someSuperset) return IntersectResult.Intersect;
       someSubset = true;
     } else {
       // A diff B
-      return IntersectResult.Diff;
+      return IntersectResult.Intersect;
     }
   }
 
+  // equals = MAYBE ONLY equals
   if (!someExcludes && !someSuperset && !someSubset) return IntersectResult.Equal;
-  if (someExcludes && !someSuperset && !someSubset) return IntersectResult.Exclude;
+  // 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;
-  else return IntersectResult.Diff;
+  // intersect = any other condition
+  else return IntersectResult.Intersect;
 }
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/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(" - ");
+}

From 14b529227aedfc0fb1e7db0745abaef0d08d9977 Mon Sep 17 00:00:00 2001
From: dapplion <35266934+dapplion@users.noreply.github.com>
Date: Wed, 18 May 2022 11:51:57 +0700
Subject: [PATCH 3/5] Review PR

---
 .../lodestar/src/api/impl/validator/index.ts  |  6 +++-
 .../opPools/aggregatedAttestationPool.ts      |  7 ++--
 .../src/network/gossip/handlers/index.ts      |  6 +++-
 .../opPools/aggregatedAttestationPool.test.ts |  2 +-
 .../opPools/aggregatedAttestationPool.test.ts | 34 +++++++------------
 5 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/packages/lodestar/src/api/impl/validator/index.ts b/packages/lodestar/src/api/impl/validator/index.ts
index 39b6766c2e1f..b9c32476de80 100644
--- a/packages/lodestar/src/api/impl/validator/index.ts
+++ b/packages/lodestar/src/api/impl/validator/index.ts
@@ -439,7 +439,11 @@ export function getValidatorApi({chain, config, logger, metrics, network, sync}:
               signedAggregateAndProof
             );
 
-            chain.aggregatedAttestationPool.add(signedAggregateAndProof.message.aggregate, committeeIndices);
+            chain.aggregatedAttestationPool.add(
+              signedAggregateAndProof.message.aggregate,
+              indexedAttestation.attestingIndices.length,
+              committeeIndices
+            );
             const sentPeers = await network.gossip.publishBeaconAggregateAndProof(signedAggregateAndProof);
             metrics?.submitAggregatedAttestation(seenTimestampSec, indexedAttestation, sentPeers);
           } catch (e) {
diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
index c1c807e72ab8..bf0ee9d95438 100644
--- a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
+++ b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
@@ -55,7 +55,7 @@ export class AggregatedAttestationPool {
   );
   private lowestPermissibleSlot = 0;
 
-  add(attestation: phase0.Attestation, committee: ValidatorIndex[]): InsertOutcome {
+  add(attestation: phase0.Attestation, attestingIndicesCount: number, committee: ValidatorIndex[]): InsertOutcome {
     const slot = attestation.data.slot;
     const lowestPermissibleSlot = this.lowestPermissibleSlot;
 
@@ -76,7 +76,7 @@ export class AggregatedAttestationPool {
 
     return attestationGroup.add({
       attestation,
-      trueBitsCount: attestation.aggregationBits.getTrueBitIndexes().length,
+      trueBitsCount: attestingIndicesCount,
     });
   }
 
@@ -246,9 +246,6 @@ export class AggregatedAttestationPool {
 interface AttestationWithIndex {
   attestation: phase0.Attestation;
   trueBitsCount: number;
-  // this is <= attestingIndices.count since some attesters may be seen by the chain
-  // this is only updated and used in removeBySeenValidators function
-  notSeenAttesterCount?: number;
 }
 
 type AttestationNonParticipant = {
diff --git a/packages/lodestar/src/network/gossip/handlers/index.ts b/packages/lodestar/src/network/gossip/handlers/index.ts
index 26554ad4e957..f7982f4452bd 100644
--- a/packages/lodestar/src/network/gossip/handlers/index.ts
+++ b/packages/lodestar/src/network/gossip/handlers/index.ts
@@ -166,7 +166,11 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH
       metrics?.registerGossipAggregatedAttestation(seenTimestampSec, signedAggregateAndProof, indexedAttestation);
       const aggregatedAttestation = signedAggregateAndProof.message.aggregate;
 
-      chain.aggregatedAttestationPool.add(aggregatedAttestation, committeeIndices);
+      chain.aggregatedAttestationPool.add(
+        aggregatedAttestation,
+        indexedAttestation.attestingIndices.length,
+        committeeIndices
+      );
 
       if (!options.dontSendGossipAttestationsToForkchoice) {
         try {
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/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts
index 9c1e25d60213..4f3cef973287 100644
--- a/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts
+++ b/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts
@@ -52,7 +52,7 @@ describe("AggregatedAttestationPool", function () {
 
   for (const {name, attestingIndices, expected} of testCases) {
     it(name, function () {
-      pool.add(attestation, attestingIndices, committee);
+      pool.add(attestation, attestingIndices.length, committee);
       expect(pool.getAttestationsForBlock(altairState)).to.be.deep.equal(expected, "incorrect returned attestations");
     });
   }
@@ -61,7 +61,7 @@ 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");
   });
 });
@@ -89,15 +89,12 @@ describe("MatchingDataAttestationGroup", function () {
 
   beforeEach(() => {
     attestationGroup = new MatchingDataAttestationGroup(committee, attestation1.data);
-    attestationGroup.add({
-      attestation: attestation1,
-      attestingIndices: new Set([100, 200]),
-    });
+    attestationGroup.add({attestation: attestation1, trueBitsCount: 2});
   });
 
   it("add - new data, getAttestations() return 2", () => {
     const attestation2 = attestationFromBits([true, false, true]);
-    const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: 2});
     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");
@@ -105,7 +102,7 @@ describe("MatchingDataAttestationGroup", function () {
 
   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)});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: committee.length});
     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");
@@ -114,7 +111,7 @@ describe("MatchingDataAttestationGroup", function () {
   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])});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: 1});
     expect(result).to.be.equal(InsertOutcome.AlreadyKnown, "incorrect InsertOutcome");
     const attestations = attestationGroup.getAttestations();
     expect(attestations).to.be.deep.equal([attestation1], "expect exactly 1 attestation");
@@ -124,7 +121,7 @@ describe("MatchingDataAttestationGroup", function () {
     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])});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: 1});
     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");
@@ -160,7 +157,7 @@ describe("MatchingDataAttestationGroup", function () {
 
   it("getAttestationsForBlock - return 2", () => {
     const attestation2 = attestationFromBits([true, false, true]);
-    const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: 2});
     expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome");
     const attestations = attestationGroup.getAttestationsForBlock(new Set([200]));
     expect(attestations).to.be.deep.equal(
@@ -182,7 +179,7 @@ describe("MatchingDataAttestationGroup", function () {
 
   it("getAttestations", () => {
     const attestation2 = attestationFromBits([true, false, true]);
-    const result = attestationGroup.add({attestation: attestation2, attestingIndices: new Set([100, 300])});
+    const result = attestationGroup.add({attestation: attestation2, trueBitsCount: 2});
     expect(result).to.be.equal(InsertOutcome.NewData, "incorrect InsertOutcome");
     const attestations = attestationGroup.getAttestations();
     expect(attestations).to.be.deep.equal([attestation1, attestation2]);
@@ -206,13 +203,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 +217,3 @@ describe("aggregateInto", function () {
     ).to.be.equal(true, "invalid aggregated signature");
   });
 });
-
-function setToArr<T>(set: Set<T>): T[] {
-  return Array.from(set.values());
-}

From f864ac683f28d660edaee1baf60e04bceb432e27 Mon Sep 17 00:00:00 2001
From: dapplion <35266934+dapplion@users.noreply.github.com>
Date: Wed, 18 May 2022 14:25:51 +0700
Subject: [PATCH 4/5] Update tests

---
 .../opPools/aggregatedAttestationPool.ts      |  63 ++++-
 .../opPools/aggregatedAttestationPool.test.ts | 264 ++++++++++--------
 2 files changed, 204 insertions(+), 123 deletions(-)

diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
index bf0ee9d95438..701be3509f8b 100644
--- a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
+++ b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
@@ -94,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[] = [];
 
@@ -132,7 +130,7 @@ export class AggregatedAttestationPool {
         ) {
           continue;
         }
-        const participation = getParticipationFn(epoch, attestationGroup.committee);
+        const participation = getParticipation(epoch, attestationGroup.committee);
         if (participation === null) {
           continue;
         }
@@ -334,6 +332,7 @@ export class MatchingDataAttestationGroup {
       let notSeenAttesterCount = 0;
 
       for (let i = 0; i < committeeLen; i++) {
+        // TODO: Optimize aggregationBits.get() in bulk for the entire BitArray
         if (!committeeSeenAttesting[i] && aggregationBits.get(i)) {
           notSeenAttesterCount++;
         }
@@ -364,6 +363,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<ValidatorIndex>();
+      for (const validatorIndex of committee) {
+        if (flagIsTimelySource(participationStatus[validatorIndex])) {
+          seenValidatorIndices.add(validatorIndex);
+        }
+      }
+      return seenValidatorIndices;
+    };
+  }
+}
+
 export function extractParticipation(
   attestations: phase0.PendingAttestation[],
   state: CachedBeaconStateAllForks
diff --git a/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts b/packages/lodestar/test/unit/chain/opPools/aggregatedAttestationPool.test.ts
index 4f3cef973287..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.length, 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");
     });
   }
 
@@ -66,127 +77,141 @@ describe("AggregatedAttestationPool", function () {
   });
 });
 
-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, trueBitsCount: 2});
-  });
+      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, trueBitsCount: 2});
-    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, trueBitsCount: committee.length});
-    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, trueBitsCount: 1});
-    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, trueBitsCount: 1});
-    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, trueBitsCount: 2});
-    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, trueBitsCount: 2});
-    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])}};
@@ -194,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));

From b2538077f28ed7e050b733c00f03e05b54fa90b2 Mon Sep 17 00:00:00 2001
From: dapplion <35266934+dapplion@users.noreply.github.com>
Date: Thu, 19 May 2022 10:19:28 +0700
Subject: [PATCH 5/5] Remove un-used code

---
 .../opPools/aggregatedAttestationPool.ts      | 61 -------------------
 1 file changed, 61 deletions(-)

diff --git a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
index 701be3509f8b..73f7e42b9211 100644
--- a/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
+++ b/packages/lodestar/src/chain/opPools/aggregatedAttestationPool.ts
@@ -185,59 +185,6 @@ 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<ValidatorIndex>();
-      for (const validatorIndex of committee) {
-        if (flagIsTimelySource(participationStatus[validatorIndex])) {
-          seenValidatorIndices.add(validatorIndex);
-        }
-      }
-      return seenValidatorIndices;
-    };
-  }
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention
@@ -439,14 +386,6 @@ export function extractParticipation(
   return allParticipants;
 }
 
-export function intersection(bigSet: Set<ValidatorIndex>, smallSet: Set<ValidatorIndex>): 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.