diff --git a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts index bbbf89e45cd..451a2e3e5dc 100644 --- a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts +++ b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts @@ -29,6 +29,7 @@ export async function validateGossipAggregateAndProof( const attData = aggregate.data; const attDataRoot = toHexString(ssz.phase0.AttestationData.hashTreeRoot(attData)); const attSlot = attData.slot; + const attIndex = attData.index; const attEpoch = computeEpochAtSlot(attSlot); const attTarget = attData.target; const targetEpoch = attTarget.epoch; @@ -84,7 +85,8 @@ export async function validateGossipAggregateAndProof( }); }); - const committeeIndices = getCommitteeIndices(attHeadState, attSlot, attData.index); + const committeeIndices: number[] = getCommitteeIndices(attHeadState, attSlot, attIndex); + const attestingIndices = aggregate.aggregationBits.intersectValues(committeeIndices); const indexedAttestation: phase0.IndexedAttestation = { attestingIndices, diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 60ef8d27878..3e6b057346f 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -89,6 +89,7 @@ export async function validateGossipAttestation( // -- i.e. data.index < get_committee_count_per_slot(state, data.target.epoch) const attIndex = attData.index; const committeeIndices = getCommitteeIndices(attHeadState, attSlot, attIndex); + const validatorIndex = committeeIndices[bitIndex]; // [REJECT] The number of aggregation bits matches the committee size @@ -284,7 +285,18 @@ export function getCommitteeIndices( attestationSlot: Slot, attestationIndex: number ): number[] { - const {committees} = attestationTargetState.epochCtx.getShufflingAtSlot(attestationSlot); + const shuffling = attestationTargetState.epochCtx.getShufflingAtSlotOrNull(attestationSlot); + if (shuffling === null) { + // this may come from an out-of-synced node, the spec did not define it so should not REJECT + // see https://github.com/ChainSafe/lodestar/issues/4396 + throw new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.NO_COMMITTEE_FOR_SLOT_AND_INDEX, + index: attestationIndex, + slot: attestationSlot, + }); + } + + const {committees} = shuffling; const slotCommittees = committees[attestationSlot % SLOTS_PER_EPOCH]; if (attestationIndex >= slotCommittees.length) { diff --git a/packages/beacon-node/src/network/gossip/validation/index.ts b/packages/beacon-node/src/network/gossip/validation/index.ts index 75ee58759c8..48709e15c00 100644 --- a/packages/beacon-node/src/network/gossip/validation/index.ts +++ b/packages/beacon-node/src/network/gossip/validation/index.ts @@ -88,7 +88,8 @@ function getGossipValidatorFn( return MessageAcceptance.Accept; } catch (e) { if (!(e instanceof GossipActionError)) { - logger.error(`Gossip validation ${type} threw a non-GossipActionError`, {}, e as Error); + // not deserve to log error here, it looks too dangerous to users + logger.debug(`Gossip validation ${type} threw a non-GossipActionError`, {}, e as Error); return MessageAcceptance.Ignore; } diff --git a/packages/beacon-node/test/unit/chain/validation/aggregateAndProof.test.ts b/packages/beacon-node/test/unit/chain/validation/aggregateAndProof.test.ts index dce1588dc62..632fe45ddd9 100644 --- a/packages/beacon-node/test/unit/chain/validation/aggregateAndProof.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/aggregateAndProof.test.ts @@ -1,6 +1,7 @@ import {toHexString} from "@chainsafe/ssz"; import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {phase0, ssz} from "@lodestar/types"; +import {processSlots} from "@lodestar/state-transition"; import {IBeaconChain} from "../../../../src/chain/index.js"; import {AttestationErrorCode} from "../../../../src/chain/errors/index.js"; import {validateGossipAggregateAndProof} from "../../../../src/chain/validation/index.js"; @@ -12,6 +13,7 @@ import { getAggregateAndProofValidData, AggregateAndProofValidDataOpts, } from "../../../utils/validationData/aggregateAndProof.js"; +import {IStateRegenerator} from "../../../../src/chain/regen/interface.js"; describe("chain / validation / aggregateAndProof", () => { const vc = 64; @@ -108,6 +110,22 @@ describe("chain / validation / aggregateAndProof", () => { await expectError(chain, signedAggregateAndProof, AttestationErrorCode.INVALID_TARGET_ROOT); }); + it("NO_COMMITTEE_FOR_SLOT_AND_INDEX", async () => { + const {chain, signedAggregateAndProof} = getValidData(); + // slot is out of the commitee range + // simulate https://github.com/ChainSafe/lodestar/issues/4396 + // this way we cannot get committeeIndices + const committeeState = processSlots( + getState(), + signedAggregateAndProof.message.aggregate.data.slot + 2 * SLOTS_PER_EPOCH + ); + (chain as {regen: IStateRegenerator}).regen = ({ + getState: async () => committeeState, + } as Partial) as IStateRegenerator; + + await expectError(chain, signedAggregateAndProof, AttestationErrorCode.NO_COMMITTEE_FOR_SLOT_AND_INDEX); + }); + it("EMPTY_AGGREGATION_BITFIELD", async () => { const {chain, signedAggregateAndProof} = getValidData(); // Unset all aggregationBits diff --git a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts index b33cffcdf74..d9845e522fc 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts @@ -1,6 +1,7 @@ import {SLOTS_PER_EPOCH} from "@lodestar/params"; import {phase0} from "@lodestar/types"; import {BitArray} from "@chainsafe/ssz"; +import {processSlots} from "@lodestar/state-transition"; import {IBeaconChain} from "../../../../src/chain/index.js"; import {AttestationErrorCode} from "../../../../src/chain/errors/index.js"; import {validateGossipAttestation} from "../../../../src/chain/validation/index.js"; @@ -9,6 +10,7 @@ import {expectRejectedWithLodestarError} from "../../../utils/errors.js"; import {generateTestCachedBeaconStateOnlyValidators} from "../../../../../state-transition/test/perf/util.js"; import {memoOnce} from "../../../utils/cache.js"; import {getAttestationValidData, AttestationValidDataOpts} from "../../../utils/validationData/attestation.js"; +import {IStateRegenerator} from "../../../../src/chain/regen/interface.js"; describe("chain / validation / attestation", () => { const vc = 64; @@ -97,6 +99,19 @@ describe("chain / validation / attestation", () => { await expectError(chain, attestation, subnet, AttestationErrorCode.INVALID_TARGET_ROOT); }); + it("NO_COMMITTEE_FOR_SLOT_AND_INDEX", async () => { + const {chain, attestation, subnet} = getValidData(); + // slot is out of the commitee range + // simulate https://github.com/ChainSafe/lodestar/issues/4396 + // this way we cannot get committeeIndices + const committeeState = processSlots(getState(), attestation.data.slot + 2 * SLOTS_PER_EPOCH); + (chain as {regen: IStateRegenerator}).regen = ({ + getState: async () => committeeState, + } as Partial) as IStateRegenerator; + + await expectError(chain, attestation, subnet, AttestationErrorCode.NO_COMMITTEE_FOR_SLOT_AND_INDEX); + }); + it("WRONG_NUMBER_OF_AGGREGATION_BITS", async () => { const {chain, attestation, subnet} = getValidData(); // Increase the length of aggregationBits beyond the committee size diff --git a/packages/state-transition/src/cache/epochContext.ts b/packages/state-transition/src/cache/epochContext.ts index fbdc1340d16..a5e79b5be4a 100644 --- a/packages/state-transition/src/cache/epochContext.ts +++ b/packages/state-transition/src/cache/epochContext.ts @@ -709,7 +709,21 @@ export class EpochContext { return this.getShufflingAtEpoch(epoch); } + getShufflingAtSlotOrNull(slot: Slot): IEpochShuffling | null { + const epoch = computeEpochAtSlot(slot); + return this.getShufflingAtEpochOrNull(epoch); + } + getShufflingAtEpoch(epoch: Epoch): IEpochShuffling { + const shuffling = this.getShufflingAtEpochOrNull(epoch); + if (shuffling === null) { + throw new Error(`Requesting slot committee out of range epoch: ${epoch} current: ${this.currentShuffling.epoch}`); + } + + return shuffling; + } + + getShufflingAtEpochOrNull(epoch: Epoch): IEpochShuffling | null { if (epoch === this.previousShuffling.epoch) { return this.previousShuffling; } else if (epoch === this.currentShuffling.epoch) { @@ -717,7 +731,7 @@ export class EpochContext { } else if (epoch === this.nextShuffling.epoch) { return this.nextShuffling; } else { - throw new Error(`Requesting slot committee out of range epoch: ${epoch} current: ${this.currentShuffling.epoch}`); + return null; } }