Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gossip attestation validation: handle no committee error #4589

Merged
merged 3 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/beacon-node/src/chain/validation/aggregateAndProof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -84,7 +85,19 @@ export async function validateGossipAggregateAndProof(
});
});

const committeeIndices = getCommitteeIndices(attHeadState, attSlot, attData.index);
let committeeIndices: number[];
try {
committeeIndices = getCommitteeIndices(attHeadState, attSlot, attIndex);
} catch (e) {
// 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: attIndex,
slot: attSlot,
});
}

const attestingIndices = aggregate.aggregationBits.intersectValues(committeeIndices);
const indexedAttestation: phase0.IndexedAttestation = {
attestingIndices,
Expand Down
14 changes: 13 additions & 1 deletion packages/beacon-node/src/chain/validation/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,19 @@ export async function validateGossipAttestation(
// [REJECT] The committee index is within the expected range
// -- i.e. data.index < get_committee_count_per_slot(state, data.target.epoch)
const attIndex = attData.index;
const committeeIndices = getCommitteeIndices(attHeadState, attSlot, attIndex);
let committeeIndices: number[];
try {
committeeIndices = getCommitteeIndices(attHeadState, attSlot, attIndex);
} catch (e) {
// 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: attIndex,
slot: attSlot,
});
}

const validatorIndex = committeeIndices[bitIndex];

// [REJECT] The number of aggregation bits matches the committee size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ function getGossipValidatorFn<K extends GossipType>(
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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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<IStateRegenerator>) 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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<IStateRegenerator>) 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
Expand Down