fix: get seen AttData key from SignedAggregateAndProof electra (#6802)

* fix: get seen AttData key from SignedAggregateAndProof electra

* chore: revert the naming change to COMMITTEE_BITS_SIZE and add comment

* fix: add toBase64() util

Author: twoeths

packages/beacon-node/src/chain/validation/aggregateAndProof.ts

@@ -9,7 +9,9 @@ import {
 import {IBeaconChain} from "..";
 import {AttestationError, AttestationErrorCode, GossipAction} from "../errors/index.js";
 import {RegenCaller} from "../regen/index.js";
-import {getAttDataBase64FromSignedAggregateAndProofSerialized} from "../../util/sszBytes.js";
+import {
+  getSeenAttDataKeyFromSignedAggregateAndProof,
+} from "../../util/sszBytes.js";
 import {getSelectionProofSignatureSet, getAggregateAndProofSignatureSet} from "./signatureSets/index.js";
 import {
   getAttestationDataSigningRoot,
@@ -71,8 +73,10 @@ async function validateAggregateAndProof(
   const attData = aggregate.data;
   const attSlot = attData.slot;
 
-  const attDataBase64 = serializedData ? getAttDataBase64FromSignedAggregateAndProofSerialized(serializedData) : null;
-  const cachedAttData = attDataBase64 ? chain.seenAttestationDatas.get(attSlot, attDataBase64) : null;
+  const seenAttDataKey = serializedData
+    ? getSeenAttDataKeyFromSignedAggregateAndProof(ForkSeq[fork], serializedData)
+    : null;
+  const cachedAttData = seenAttDataKey ? chain.seenAttestationDatas.get(attSlot, seenAttDataKey) : null;
 
   let attIndex;
   if (ForkSeq[fork] >= ForkSeq.electra) {

packages/beacon-node/src/util/sszBytes.ts

@@ -42,6 +42,7 @@ const ATTESTATION_BEACON_BLOCK_ROOT_OFFSET = VARIABLE_FIELD_OFFSET + 8 + 8;
 const ROOT_SIZE = 32;
 const SLOT_SIZE = 8;
 const ATTESTATION_DATA_SIZE = 128;
+// MAX_COMMITTEES_PER_SLOT is in bit, need to convert to byte
 const COMMITTEE_BITS_SIZE = Math.max(Math.ceil(MAX_COMMITTEES_PER_SLOT / 8), 1);
 const SIGNATURE_SIZE = 96;
 
@@ -88,7 +89,7 @@ export function getSeenAttDataKeyElectra(electraAttestationBytes: Uint8Array): A
     return null;
   }
 
-  return Buffer.from(electraAttestationBytes.subarray(startIndex, startIndex + seenKeyLength)).toString("base64");
+  return toBase64(electraAttestationBytes.subarray(startIndex, startIndex + seenKeyLength));
 }
 
 /**
@@ -171,8 +172,9 @@ const SIGNED_AGGREGATE_AND_PROOF_SLOT_OFFSET = AGGREGATE_OFFSET + VARIABLE_FIELD
 const SIGNED_AGGREGATE_AND_PROOF_BLOCK_ROOT_OFFSET = SIGNED_AGGREGATE_AND_PROOF_SLOT_OFFSET + 8 + 8;
 
 /**
- * Extract slot from signed aggregate and proof serialized bytes.
- * Return null if data is not long enough to extract slot.
+ * Extract slot from signed aggregate and proof serialized bytes
+ * Return null if data is not long enough to extract slot
+ * This works for both phase + electra
  */
 export function getSlotFromSignedAggregateAndProofSerialized(data: Uint8Array): Slot | null {
   if (data.length < SIGNED_AGGREGATE_AND_PROOF_SLOT_OFFSET + SLOT_SIZE) {
@@ -183,8 +185,9 @@ export function getSlotFromSignedAggregateAndProofSerialized(data: Uint8Array):
 }
 
 /**
- * Extract block root from signed aggregate and proof serialized bytes.
- * Return null if data is not long enough to extract block root.
+ * Extract block root from signed aggregate and proof serialized bytes
+ * Return null if data is not long enough to extract block root
+ * This works for both phase + electra
  */
 export function getBlockRootFromSignedAggregateAndProofSerialized(data: Uint8Array): BlockRootHex | null {
   if (data.length < SIGNED_AGGREGATE_AND_PROOF_BLOCK_ROOT_OFFSET + ROOT_SIZE) {
@@ -199,11 +202,39 @@ export function getBlockRootFromSignedAggregateAndProofSerialized(data: Uint8Arr
   );
 }
 
+/**
+ * Extract attestation data key from SignedAggregateAndProof Uint8Array to use cached data from SeenAttestationDatas
+ */
+export function getSeenAttDataKeyFromSignedAggregateAndProof(
+  forkSeq: ForkSeq,
+  data: Uint8Array
+): SeenAttDataKey | null {
+  return forkSeq >= ForkSeq.electra
+    ? getSeenAttDataKeyFromSignedAggregateAndProofElectra(data)
+    : getSeenAttDataKeyFromSignedAggregateAndProofPhase0(data);
+}
+
+/**
+ * Extract AttestationData + CommitteeBits from SignedAggregateAndProof for electra
+ * Return null if data is not long enough
+ */
+export function getSeenAttDataKeyFromSignedAggregateAndProofElectra(data: Uint8Array): SeenAttDataKey | null {
+  const startIndex = SIGNED_AGGREGATE_AND_PROOF_SLOT_OFFSET;
+  const endIndex = startIndex + ATTESTATION_DATA_SIZE + COMMITTEE_BITS_SIZE;
+
+  if (data.length < endIndex) {
+    return null;
+  }
+
+  // base64 is a bit efficient than hex
+  return toBase64(data.subarray(startIndex, endIndex));
+}
+
 /**
  * Extract attestation data base64 from signed aggregate and proof serialized bytes.
  * Return null if data is not long enough to extract attestation data.
  */
-export function getAttDataBase64FromSignedAggregateAndProofSerialized(data: Uint8Array): AttDataBase64 | null {
+export function getSeenAttDataKeyFromSignedAggregateAndProofPhase0(data: Uint8Array): AttDataBase64 | null {
   if (data.length < SIGNED_AGGREGATE_AND_PROOF_SLOT_OFFSET + ATTESTATION_DATA_SIZE) {
     return null;
   }

packages/beacon-node/test/unit/util/sszBytes.test.ts

@@ -1,11 +1,11 @@
 import {describe, it, expect} from "vitest";
 import {BitArray} from "@chainsafe/ssz";
-import {allForks, deneb, Epoch, isElectraAttestation, phase0, RootHex, Slot, ssz} from "@lodestar/types";
+import {allForks, deneb, electra, Epoch, isElectraAttestation, phase0, RootHex, Slot, ssz} from "@lodestar/types";
 import {fromHex, toHex} from "@lodestar/utils";
 import {ForkName, MAX_COMMITTEES_PER_SLOT} from "@lodestar/params";
 import {
   getSeenAttDataKeyPhase0,
-  getAttDataBase64FromSignedAggregateAndProofSerialized,
+  getSeenAttDataKeyFromSignedAggregateAndProofPhase0,
   getAggregationBitsFromAttestationSerialized as getAggregationBitsFromAttestationSerialized,
   getBlockRootFromAttestationSerialized,
   getBlockRootFromSignedAggregateAndProofSerialized,
@@ -15,6 +15,7 @@ import {
   getSlotFromSignedBeaconBlockSerialized,
   getSlotFromBlobSidecarSerialized,
   getCommitteeBitsFromAttestationSerialized,
+  getSeenAttDataKeyFromSignedAggregateAndProofElectra,
 } from "../../../src/util/sszBytes.js";
 
 describe("attestation SSZ serialized picking", () => {
@@ -104,16 +105,15 @@ describe("attestation SSZ serialized picking", () => {
   });
 });
 
-describe("aggregateAndProof SSZ serialized picking", () => {
+describe("phase0 SignedAggregateAndProof SSZ serialized picking", () => {
   const testCases: phase0.SignedAggregateAndProof[] = [
     ssz.phase0.SignedAggregateAndProof.defaultValue(),
-    signedAggregateAndProofFromValues(
+    phase0SignedAggregateAndProofFromValues(
       4_000_000,
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
       200_00,
       "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeffffffffffffffffffffffffffffffff"
     ),
-    ssz.electra.SignedAggregateAndProof.defaultValue(),
   ];
 
   for (const [i, signedAggregateAndProof] of testCases.entries()) {
@@ -128,7 +128,7 @@ describe("aggregateAndProof SSZ serialized picking", () => {
       );
 
       const attDataBase64 = ssz.phase0.AttestationData.serialize(signedAggregateAndProof.message.aggregate.data);
-      expect(getAttDataBase64FromSignedAggregateAndProofSerialized(bytes)).toBe(
+      expect(getSeenAttDataKeyFromSignedAggregateAndProofPhase0(bytes)).toBe(
         Buffer.from(attDataBase64).toString("base64")
       );
     });
@@ -151,7 +151,60 @@ describe("aggregateAndProof SSZ serialized picking", () => {
   it("getAttDataBase64FromSignedAggregateAndProofSerialized - invalid data", () => {
     const invalidAttDataBase64DataSizes = [0, 4, 100, 128, 339];
     for (const size of invalidAttDataBase64DataSizes) {
-      expect(getAttDataBase64FromSignedAggregateAndProofSerialized(Buffer.alloc(size))).toBeNull();
+      expect(getSeenAttDataKeyFromSignedAggregateAndProofPhase0(Buffer.alloc(size))).toBeNull();
+    }
+  });
+});
+
+describe("electra SignedAggregateAndProof SSZ serialized picking", () => {
+  const testCases: electra.SignedAggregateAndProof[] = [
+    ssz.electra.SignedAggregateAndProof.defaultValue(),
+    electraSignedAggregateAndProofFromValues(
+      4_000_000,
+      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+      200_00,
+      "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeffffffffffffffffffffffffffffffff"
+    ),
+  ];
+
+  for (const [i, signedAggregateAndProof] of testCases.entries()) {
+    it(`signedAggregateAndProof ${i}`, () => {
+      const bytes = ssz.electra.SignedAggregateAndProof.serialize(signedAggregateAndProof);
+
+      expect(getSlotFromSignedAggregateAndProofSerialized(bytes)).toBe(
+        signedAggregateAndProof.message.aggregate.data.slot
+      );
+      expect(getBlockRootFromSignedAggregateAndProofSerialized(bytes)).toBe(
+        toHex(signedAggregateAndProof.message.aggregate.data.beaconBlockRoot)
+      );
+
+      const attDataBase64 = ssz.phase0.AttestationData.serialize(signedAggregateAndProof.message.aggregate.data);
+      const committeeBits = ssz.electra.CommitteeBits.serialize(
+        signedAggregateAndProof.message.aggregate.committeeBits
+      );
+      const seenKey = Buffer.concat([attDataBase64, committeeBits]).toString("base64");
+      expect(getSeenAttDataKeyFromSignedAggregateAndProofElectra(bytes)).toBe(seenKey);
+    });
+  }
+
+  it("getSlotFromSignedAggregateAndProofSerialized - invalid data", () => {
+    const invalidSlotDataSizes = [0, 4, 11];
+    for (const size of invalidSlotDataSizes) {
+      expect(getSlotFromSignedAggregateAndProofSerialized(Buffer.alloc(size))).toBeNull();
+    }
+  });
+
+  it("getBlockRootFromSignedAggregateAndProofSerialized - invalid data", () => {
+    const invalidBlockRootDataSizes = [0, 4, 20, 227];
+    for (const size of invalidBlockRootDataSizes) {
+      expect(getBlockRootFromSignedAggregateAndProofSerialized(Buffer.alloc(size))).toBeNull();
+    }
+  });
+
+  it("getAttDataBase64FromSignedAggregateAndProofSerialized - invalid data", () => {
+    const invalidAttDataBase64DataSizes = [0, 4, 100, 128, 339];
+    for (const size of invalidAttDataBase64DataSizes) {
+      expect(getSeenAttDataKeyFromSignedAggregateAndProofPhase0(Buffer.alloc(size))).toBeNull();
     }
   });
 });
@@ -206,7 +259,7 @@ function attestationFromValues(
   return attestation;
 }
 
-function signedAggregateAndProofFromValues(
+function phase0SignedAggregateAndProofFromValues(
   slot: Slot,
   blockRoot: RootHex,
   targetEpoch: Epoch,
@@ -220,6 +273,21 @@ function signedAggregateAndProofFromValues(
   return signedAggregateAndProof;
 }
 
+function electraSignedAggregateAndProofFromValues(
+  slot: Slot,
+  blockRoot: RootHex,
+  targetEpoch: Epoch,
+  targetRoot: RootHex
+): electra.SignedAggregateAndProof {
+  const signedAggregateAndProof = ssz.electra.SignedAggregateAndProof.defaultValue();
+  signedAggregateAndProof.message.aggregate.data.slot = slot;
+  signedAggregateAndProof.message.aggregate.data.beaconBlockRoot = fromHex(blockRoot);
+  signedAggregateAndProof.message.aggregate.data.target.epoch = targetEpoch;
+  signedAggregateAndProof.message.aggregate.data.target.root = fromHex(targetRoot);
+  signedAggregateAndProof.message.aggregate.committeeBits = BitArray.fromSingleBit(MAX_COMMITTEES_PER_SLOT, 1);
+  return signedAggregateAndProof;
+}
+
 function signedBeaconBlockFromValues(slot: Slot): phase0.SignedBeaconBlock {
   const signedBeaconBlock = ssz.phase0.SignedBeaconBlock.defaultValue();
   signedBeaconBlock.message.slot = slot;