From 80c001b5d4f5219885a429663852140dc1800f4d Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Mon, 11 Sep 2023 20:17:40 +0200 Subject: [PATCH] fix: epoch cache error should be treated with care in gossips validation (#5939) * Add custom type for epocch cache errors * Fix failing unit test --- .../src/chain/validation/attestation.ts | 34 +++++++++----- .../state-transition/src/cache/epochCache.ts | 44 ++++++++++++++----- packages/state-transition/src/index.ts | 8 +++- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 60b51b40a0c6..0b642101f010 100644 --- 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 { - 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 index da41631afe29..aeefc4769aca 100644 --- 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 {} diff --git a/packages/state-transition/src/index.ts b/packages/state-transition/src/index.ts index 98ab4b4fe607..8c9a296ebd9f 100644 --- 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