fix: epoch cache error should be treated with care in gossips validat…

…ion (#5939) * Add custom type for epocch cache errors * Fix failing unit test
ChainSafe · Sep 11, 2023 · 80c001b · 80c001b
1 parent 8f308c0
commit 80c001b
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 22 deletions.
diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts
@@ -8,6 +8,8 @@ import {
   getAttestationDataSigningRoot,
   createSingleSignatureSetFromComponents,
   SingleSignatureSet,
+  EpochCacheError,
+  EpochCacheErrorCode,
 } from "@lodestar/state-transition";
 import {AttestationError, AttestationErrorCode, GossipAction} from "../errors/index.js";
 import {MAXIMUM_GOSSIP_CLOCK_DISPARITY_SEC} from "../../constants/index.js";
@@ -202,18 +204,28 @@ export async function validateAttestation(
   subnet: number | null,
   prioritizeBls = false
 ): Promise<AttestationValidationResult> {
-  const step0Result = await validateGossipAttestationNoSignatureCheck(fork, chain, attestationOrBytes, subnet);
-  const {attestation, signatureSet, validatorIndex} = step0Result;
-  const isValid = await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls});
+  try {
+    const step0Result = await validateGossipAttestationNoSignatureCheck(fork, chain, attestationOrBytes, subnet);
+    const {attestation, signatureSet, validatorIndex} = step0Result;
+    const isValid = await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls});
 
-  if (isValid) {
-    const targetEpoch = attestation.data.target.epoch;
-    chain.seenAttesters.add(targetEpoch, validatorIndex);
-    return step0Result;
-  } else {
-    throw new AttestationError(GossipAction.IGNORE, {
-      code: AttestationErrorCode.INVALID_SIGNATURE,
-    });
+    if (isValid) {
+      const targetEpoch = attestation.data.target.epoch;
+      chain.seenAttesters.add(targetEpoch, validatorIndex);
+      return step0Result;
+    } else {
+      throw new AttestationError(GossipAction.IGNORE, {
+        code: AttestationErrorCode.INVALID_SIGNATURE,
+      });
+    }
+  } catch (err) {
+    if (err instanceof EpochCacheError && err.type.code === EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE) {
+      throw new AttestationError(GossipAction.IGNORE, {
+        code: AttestationErrorCode.BAD_TARGET_EPOCH,
+      });
+    } else {
+      throw err;
+    }
   }
 }
 

diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts
@@ -569,9 +569,11 @@ export class EpochCache {
   getBeaconProposer(slot: Slot): ValidatorIndex {
     const epoch = computeEpochAtSlot(slot);
     if (epoch !== this.currentShuffling.epoch) {
-      throw new Error(
-        `Requesting beacon proposer for different epoch current shuffling: ${epoch} != ${this.currentShuffling.epoch}`
-      );
+      throw new EpochCacheError({
+        code: EpochCacheErrorCode.PROPOSER_EPOCH_MISMATCH,
+        currentEpoch: this.currentShuffling.epoch,
+        requestedEpoch: epoch,
+      });
     }
     return this.proposers[slot % SLOTS_PER_EPOCH];
   }
@@ -732,7 +734,11 @@ export class EpochCache {
   getShufflingAtEpoch(epoch: Epoch): EpochShuffling {
     const shuffling = this.getShufflingAtEpochOrNull(epoch);
     if (shuffling === null) {
-      throw new Error(`Requesting slot committee out of range epoch: ${epoch} current: ${this.currentShuffling.epoch}`);
+      throw new EpochCacheError({
+        code: EpochCacheErrorCode.COMMITTEE_EPOCH_OUT_OF_RANGE,
+        currentEpoch: this.currentShuffling.epoch,
+        requestedEpoch: epoch,
+      });
     }
 
     return shuffling;
@@ -772,7 +778,7 @@ export class EpochCache {
       case this.syncPeriod + 1:
         return this.nextSyncCommitteeIndexed;
       default:
-        throw new Error(`No sync committee for epoch ${epoch}`);
+        throw new EpochCacheError({code: EpochCacheErrorCode.NO_SYNC_COMMITTEE, epoch});
     }
   }
 
@@ -817,13 +823,31 @@ type AttesterDuty = {
 
 export enum EpochCacheErrorCode {
   COMMITTEE_INDEX_OUT_OF_RANGE = "EPOCH_CONTEXT_ERROR_COMMITTEE_INDEX_OUT_OF_RANGE",
+  COMMITTEE_EPOCH_OUT_OF_RANGE = "EPOCH_CONTEXT_ERROR_COMMITTEE_EPOCH_OUT_OF_RANGE",
+  NO_SYNC_COMMITTEE = "EPOCH_CONTEXT_ERROR_NO_SYNC_COMMITTEE",
+  PROPOSER_EPOCH_MISMATCH = "EPOCH_CONTEXT_ERROR_PROPOSER_EPOCH_MISMATCH",
 }
 
-type EpochCacheErrorType = {
-  code: EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE;
-  index: number;
-  maxIndex: number;
-};
+type EpochCacheErrorType =
+  | {
+      code: EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE;
+      index: number;
+      maxIndex: number;
+    }
+  | {
+      code: EpochCacheErrorCode.COMMITTEE_EPOCH_OUT_OF_RANGE;
+      requestedEpoch: Epoch;
+      currentEpoch: Epoch;
+    }
+  | {
+      code: EpochCacheErrorCode.NO_SYNC_COMMITTEE;
+      epoch: Epoch;
+    }
+  | {
+      code: EpochCacheErrorCode.PROPOSER_EPOCH_MISMATCH;
+      requestedEpoch: Epoch;
+      currentEpoch: Epoch;
+    };
 
 export class EpochCacheError extends LodestarError<EpochCacheErrorType> {}
 

diff --git a/packages/state-transition/src/index.ts b/packages/state-transition/src/index.ts
@@ -30,7 +30,13 @@ export {
   isStateBalancesNodesPopulated,
   isStateValidatorsNodesPopulated,
 } from "./cache/stateCache.js";
-export {EpochCache, EpochCacheImmutableData, createEmptyEpochCacheImmutableData} from "./cache/epochCache.js";
+export {
+  EpochCache,
+  EpochCacheImmutableData,
+  createEmptyEpochCacheImmutableData,
+  EpochCacheError,
+  EpochCacheErrorCode,
+} from "./cache/epochCache.js";
 export {EpochTransitionCache, beforeProcessEpoch} from "./cache/epochTransitionCache.js";
 
 // Aux data-structures