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

Add TOO_MANY_SKIPPED_SLOTS attestation error #5390

Merged
merged 1 commit into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 6 additions & 6 deletions packages/beacon-node/src/chain/blocks/importBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,12 @@ export async function importBlock(
this.clock.currentSlot,
executionStatus
);
this.logger.verbose("Added block to forkchoice", {slot: block.message.slot, root: blockRootHex});

// This adds the state necessary to process the next block
// Some block event handlers require state being in state cache so need to do this before emitting EventType.block
this.stateCache.add(postState);

this.logger.verbose("Added block to forkchoice and state cache", {slot: block.message.slot, root: blockRootHex});
this.emitter.emit(routes.events.EventType.block, {
block: toHexString(this.config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message)),
slot: block.message.slot,
Expand Down Expand Up @@ -325,11 +330,6 @@ export async function importBlock(
}
}

// 7. Add post state to stateCache
//
// This adds the state necessary to process the next block
this.stateCache.add(postState);

if (!isStateValidatorsNodesPopulated(postState)) {
this.logger.verbose("After importBlock caching postState without SSZ cache", {slot: postState.slot});
}
Expand Down
5 changes: 4 additions & 1 deletion packages/beacon-node/src/chain/errors/attestationError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ export enum AttestationErrorCode {
* Invalid ssz bytes.
*/
INVALID_SERIALIZED_BYTES = "ATTESTATION_ERROR_INVALID_SERIALIZED_BYTES",
/** Too many skipped slots. */
TOO_MANY_SKIPPED_SLOTS = "ATTESTATION_ERROR_TOO_MANY_SKIPPED_SLOTS",
}

export type AttestationErrorType =
Expand Down Expand Up @@ -163,7 +165,8 @@ export type AttestationErrorType =
| {code: AttestationErrorCode.MISSING_ATTESTATION_HEAD_STATE; error: Error}
| {code: AttestationErrorCode.INVALID_AGGREGATOR}
| {code: AttestationErrorCode.INVALID_INDEXED_ATTESTATION}
| {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES};
| {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES}
| {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot};

export class AttestationError extends GossipActionError<AttestationErrorType> {
getMetadata(): Record<string, string | number | null> {
Expand Down
3 changes: 3 additions & 0 deletions packages/beacon-node/src/chain/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,7 @@ export const defaultChainOptions: IChainOptions = {
assertCorrectProgressiveBalances: false,
archiveStateEpochFrequency: 1024,
emitPayloadAttributes: false,
// for gossip block validation, it's unlikely we see a reorg with 32 slots
// for attestation validation, having this value ensures we don't have to regen states most of the time
maxSkipSlots: 32,
};
11 changes: 10 additions & 1 deletion packages/beacon-node/src/chain/validation/aggregateAndProof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,16 @@ export async function validateGossipAggregateAndProof(

// [IGNORE] The block being voted for (attestation.data.beacon_block_root) has been seen (via both gossip
// and non-gossip sources) (a client MAY queue attestations for processing once block is retrieved).
const attHeadBlock = verifyHeadBlockAndTargetRoot(chain, attData.beaconBlockRoot, attTarget.root, attEpoch);
// Lighthouse doesn't check maxSkipSlots option here but Lodestar wants to be more strict
// to be more DOS protection
const attHeadBlock = verifyHeadBlockAndTargetRoot(
chain,
attData.beaconBlockRoot,
attTarget.root,
attSlot,
attEpoch,
chain.opts.maxSkipSlots
);

// [IGNORE] The current finalized_checkpoint is an ancestor of the block defined by aggregate.data.beacon_block_root
// -- i.e. get_ancestor(store, aggregate.data.beacon_block_root, compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)) == store.finalized_checkpoint.root
Expand Down
19 changes: 17 additions & 2 deletions packages/beacon-node/src/chain/validation/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ export async function validateGossipAttestation(
chain,
attestationOrCache.attestation.data.beaconBlockRoot,
attestationOrCache.attestation.data.target.root,
attEpoch
attSlot,
attEpoch,
chain.opts.maxSkipSlots
);

// [REJECT] The block being voted for (attestation.data.beacon_block_root) passes validation.
Expand Down Expand Up @@ -334,9 +336,22 @@ export function verifyHeadBlockAndTargetRoot(
chain: IBeaconChain,
beaconBlockRoot: Root,
targetRoot: Root,
attestationEpoch: Epoch
attestationSlot: Slot,
attestationEpoch: Epoch,
maxSkipSlots?: number
): ProtoBlock {
const headBlock = verifyHeadBlockIsKnown(chain, beaconBlockRoot);
// Lighthouse rejects the attestation, however Lodestar only ignores considering it's not against the spec
// it's more about a DOS protection to us
// With verifyPropagationSlotRange() and maxSkipSlots = 32, it's unlikely we have to regenerate states in queue
// to validate beacon_attestation and aggregate_and_proof
if (maxSkipSlots !== undefined && attestationSlot - headBlock.slot > maxSkipSlots) {
throw new AttestationError(GossipAction.IGNORE, {
code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS,
attestationSlot,
headBlockSlot: headBlock.slot,
});
}
verifyAttestationTargetRoot(headBlock, targetRoot, attestationEpoch);
return headBlock;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/beacon-node/src/metrics/metrics/lodestar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ export function createLodestarMetrics(
help: "Current count of jobs being run on network processor for topic",
labelNames: ["topic"],
}),
gossipValidationErrorTooManySkippedSlots: register.gauge<"topic">({
name: "lodestar_gossip_validation_error_too_many_skipped_slots_total",
help: "Count of total gossip validation errors due to too many skipped slots",
labelNames: ["topic"],
}),

networkProcessor: {
executeWorkCalls: register.gauge({
Expand Down
20 changes: 15 additions & 5 deletions packages/beacon-node/src/network/processor/gossipHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,13 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH
try {
validationResult = await validateGossipAggregateAndProof(chain, signedAggregateAndProof, false, serializedData);
} catch (e) {
if (e instanceof AttestationError && e.action === GossipAction.REJECT) {
chain.persistInvalidSszValue(ssz.phase0.SignedAggregateAndProof, signedAggregateAndProof, "gossip_reject");
if (e instanceof AttestationError) {
if (e.action === GossipAction.REJECT) {
chain.persistInvalidSszValue(ssz.phase0.SignedAggregateAndProof, signedAggregateAndProof, "gossip_reject");
}
if (e.type.code === AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS) {
metrics?.gossipValidationErrorTooManySkippedSlots.inc({topic: topic.type});
}
}
throw e;
}
Expand Down Expand Up @@ -234,7 +239,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH
}
},

[GossipType.beacon_attestation]: async ({serializedData, msgSlot}, {subnet}, _peer, seenTimestampSec) => {
[GossipType.beacon_attestation]: async ({serializedData, msgSlot}, {type, subnet}, _peer, seenTimestampSec) => {
if (msgSlot === undefined) {
throw Error("msgSlot is undefined for beacon_attestation topic");
}
Expand All @@ -247,8 +252,13 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH
subnet
);
} catch (e) {
if (e instanceof AttestationError && e.action === GossipAction.REJECT) {
chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject");
if (e instanceof AttestationError) {
if (e.action === GossipAction.REJECT) {
chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject");
}
if (e.type.code === AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS) {
metrics?.gossipValidationErrorTooManySkippedSlots.inc({topic: type});
}
}
throw e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {signCached} from "../cache.js";
import {ClockStatic} from "../clock.js";
import {SeenAggregatedAttestations} from "../../../src/chain/seenCache/seenAggregateAndProof.js";
import {SeenAttestationDatas} from "../../../src/chain/seenCache/seenAttestationData.js";
import {defaultChainOptions} from "../../../src/chain/options.js";

export type AttestationValidDataOpts = {
currentSlot?: Slot;
Expand Down Expand Up @@ -125,6 +126,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): {
bls: new BlsSingleThreadVerifier({metrics: null}),
waitForBlock: () => Promise.resolve(false),
index2pubkey: state.epochCtx.index2pubkey,
opts: defaultChainOptions,
} as Partial<IBeaconChain> as IBeaconChain;

return {chain, attestation, subnet, validatorIndex};
Expand Down