From 090158476d0aecc9cedd6293f0492b3fbb0fa069 Mon Sep 17 00:00:00 2001 From: Cayman Date: Thu, 3 Sep 2026 16:53:41 -0400 Subject: [PATCH 1/2] fix: reject attestation source epochs below the min-span lookback Min-span entries are only written within 4096 epochs below each recorded source epoch, so min-max surround cannot detect a surround vote with an older source epoch. Reject any source epoch below the lookback window of the latest recorded attestation, read with a single reverse-range query that also replaces the same-target point read in the common case. Validate an interchange against existing spans before storing its attestations so a rejected import leaves nothing behind. --- .../attestationByTargetRepository.ts | 12 ++ .../slashingProtection/attestation/errors.ts | 10 ++ .../slashingProtection/attestation/index.ts | 31 +++- .../minMaxSurround/minMaxSurround.ts | 12 +- .../slashingProtection/attestation.test.ts | 138 ++++++++++++++++++ 5 files changed, 195 insertions(+), 8 deletions(-) create mode 100644 packages/validator/test/unit/slashingProtection/attestation.test.ts diff --git a/packages/validator/src/slashingProtection/attestation/attestationByTargetRepository.ts b/packages/validator/src/slashingProtection/attestation/attestationByTargetRepository.ts index e97ff5f96fad..ffccb166b408 100644 --- a/packages/validator/src/slashingProtection/attestation/attestationByTargetRepository.ts +++ b/packages/validator/src/slashingProtection/attestation/attestationByTargetRepository.ts @@ -41,6 +41,18 @@ export class AttestationByTargetRepository { return attestations.map((attestation) => this.type.deserialize(attestation)); } + /** Attestation with the highest target epoch recorded for `pubkey`, or null if none */ + async getLatest(pubkey: BLSPubkey): Promise { + const [att] = await this.db.values({ + gte: this.encodeKey(pubkey, 0), + lt: this.encodeKey(pubkey, Number.MAX_SAFE_INTEGER), + reverse: true, + limit: 1, + bucketId: this.bucketId, + }); + return att ? this.type.deserialize(att) : null; + } + async get(pubkey: BLSPubkey, targetEpoch: Epoch): Promise { const att = await this.db.get(this.encodeKey(pubkey, targetEpoch), this.dbReqOpts); return att && this.type.deserialize(att); diff --git a/packages/validator/src/slashingProtection/attestation/errors.ts b/packages/validator/src/slashingProtection/attestation/errors.ts index 34e11c190c8a..5a4fb3fcfd96 100644 --- a/packages/validator/src/slashingProtection/attestation/errors.ts +++ b/packages/validator/src/slashingProtection/attestation/errors.ts @@ -29,6 +29,11 @@ export enum InvalidAttestationErrorCode { * bound on target epochs for this validator. */ TARGET_LESS_THAN_OR_EQ_LOWER_BOUND = "ERR_INVALID_ATTESTATION_TARGET_LESS_THAN_OR_EQ_LOWER_BOUND", + /** + * The attestation is invalid because its source epoch is below the min-max span lookback window of the + * latest attestation of this validator, where a surround vote could not be detected. + */ + SOURCE_BELOW_MIN_SPAN_LOOKBACK = "ERR_INVALID_ATTESTATION_SOURCE_BELOW_MIN_SPAN_LOOKBACK", } type InvalidAttestationErrorType = @@ -61,6 +66,11 @@ type InvalidAttestationErrorType = code: InvalidAttestationErrorCode.TARGET_LESS_THAN_OR_EQ_LOWER_BOUND; targetEpoch: Epoch; minTargetEpoch: Epoch; + } + | { + code: InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK; + sourceEpoch: Epoch; + minSourceEpoch: Epoch; }; export class InvalidAttestationError extends LodestarError {} diff --git a/packages/validator/src/slashingProtection/attestation/index.ts b/packages/validator/src/slashingProtection/attestation/index.ts index 8b2fec1bccab..36454ea32007 100644 --- a/packages/validator/src/slashingProtection/attestation/index.ts +++ b/packages/validator/src/slashingProtection/attestation/index.ts @@ -56,9 +56,16 @@ export class SlashingProtectionAttestationService { throw new InvalidAttestationError({code: InvalidAttestationErrorCode.SOURCE_EXCEEDS_TARGET}); } + const latestAtt = await this.attestationByTarget.getLatest(pubKey); + // Check for a double vote. Namely, an existing attestation with the same target epoch, - // and a different signing root. - const sameTargetAtt = await this.attestationByTarget.get(pubKey, attestation.targetEpoch); + // and a different signing root. No db read needed if the latest attestation has a lower target epoch. + let sameTargetAtt: SlashingProtectionAttestation | null = null; + if (latestAtt && latestAtt.targetEpoch === attestation.targetEpoch) { + sameTargetAtt = latestAtt; + } else if (latestAtt && latestAtt.targetEpoch > attestation.targetEpoch) { + sameTargetAtt = await this.attestationByTarget.get(pubKey, attestation.targetEpoch); + } if (sameTargetAtt) { // Interchange format allows for attestations without signing_root, then assume root is equal if (isEqualNonZeroRoot(sameTargetAtt.signingRoot, attestation.signingRoot)) { @@ -71,6 +78,20 @@ export class SlashingProtectionAttestationService { }); } + // Min-span entries only exist within the lookback window below each recorded source epoch, a surround vote + // with an older source epoch is undetectable by min-max surround and must be rejected outright. Recorded + // attestations are surround-free, so the latest one has the highest source epoch and the widest window. + if (latestAtt) { + const minSourceEpoch = this.minMaxSurround.minSpanCoverageStart(latestAtt.sourceEpoch); + if (attestation.sourceEpoch < minSourceEpoch) { + throw new InvalidAttestationError({ + code: InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK, + sourceEpoch: attestation.sourceEpoch, + minSourceEpoch, + }); + } + } + // Check for a surround vote try { await this.minMaxSurround.assertNoSurround(pubKey, attestation); @@ -143,13 +164,13 @@ export class SlashingProtectionAttestationService { * Interchange import / export functionality */ async importAttestations(pubkey: BLSPubkey, attestations: SlashingProtectionAttestation[]): Promise { - await this.attestationByTarget.set(pubkey, attestations); - - // Pre-compute spans for all attestations + // Pre-compute spans for all attestations, before storing them so a rejected interchange leaves none behind for (const attestation of attestations) { await this.minMaxSurround.insertAttestation(pubkey, attestation); } + await this.attestationByTarget.set(pubkey, attestations); + // Pre-compute and store lower-bound const minSourceEpoch = minEpoch(attestations.map((attestation) => attestation.sourceEpoch)); const minTargetEpoch = minEpoch(attestations.map((attestation) => attestation.targetEpoch)); diff --git a/packages/validator/src/slashingProtection/minMaxSurround/minMaxSurround.ts b/packages/validator/src/slashingProtection/minMaxSurround/minMaxSurround.ts index 87976f734ba6..51fe63e83f5c 100644 --- a/packages/validator/src/slashingProtection/minMaxSurround/minMaxSurround.ts +++ b/packages/validator/src/slashingProtection/minMaxSurround/minMaxSurround.ts @@ -1,4 +1,4 @@ -import {BLSPubkey} from "@lodestar/types"; +import {BLSPubkey, Epoch} from "@lodestar/types"; import {SurroundAttestationError, SurroundAttestationErrorCode} from "./errors.js"; import {DistanceEntry, IDistanceStore, IMinMaxSurround, MinMaxSurroundAttestation} from "./interface.js"; @@ -9,7 +9,8 @@ import {DistanceEntry, IDistanceStore, IMinMaxSurround, MinMaxSurroundAttestatio * Number of epochs in the past to check for surrounding attestations. * * This value can be limited to a reasonable high amount as Lodestar does not solely rely on this strategy but also - * implements the minimal strategy which has been formally proven to be safe (https://github.com/michaelsproul/slashing-proofs). + * rejects any source epoch below `minSpanCoverageStart` of the latest recorded attestation, which is the minimal + * strategy (formally proven safe, https://github.com/michaelsproul/slashing-proofs) relaxed by this lookback. * * Limiting this value is required due to practical reasons as otherwise there would be a min-span DB read and write * for each validator from current epoch until genesis which massively increases DB size and causes I/O lag, resulting in @@ -30,6 +31,11 @@ export class MinMaxSurround implements IMinMaxSurround { this.maxEpochLookback = options?.maxEpochLookback ?? DEFAULT_MAX_EPOCH_LOOKBACK; } + /** Lowest epoch with a min-span entry after inserting an attestation with `sourceEpoch` */ + minSpanCoverageStart(sourceEpoch: Epoch): Epoch { + return Math.max(0, sourceEpoch - 1 - this.maxEpochLookback); + } + async assertNoSurround(pubKey: BLSPubkey, attestation: MinMaxSurroundAttestation): Promise { await this.assertNotSurrounding(pubKey, attestation); await this.assertNotSurrounded(pubKey, attestation); @@ -45,7 +51,7 @@ export class MinMaxSurround implements IMinMaxSurround { private async updateMinSpan(pubKey: BLSPubkey, attestation: MinMaxSurroundAttestation): Promise { await this.assertNotSurrounding(pubKey, attestation); - const untilEpoch = Math.max(0, attestation.sourceEpoch - 1 - this.maxEpochLookback); + const untilEpoch = this.minSpanCoverageStart(attestation.sourceEpoch); const values: DistanceEntry[] = []; for (let epoch = attestation.sourceEpoch - 1; epoch >= untilEpoch; epoch--) { diff --git a/packages/validator/test/unit/slashingProtection/attestation.test.ts b/packages/validator/test/unit/slashingProtection/attestation.test.ts new file mode 100644 index 000000000000..c2a4b5b8f40f --- /dev/null +++ b/packages/validator/test/unit/slashingProtection/attestation.test.ts @@ -0,0 +1,138 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import {rimraf} from "rimraf"; +import {afterEach, beforeEach, describe, expect, it} from "vitest"; +import {LevelDbController} from "@lodestar/db/controller/level"; +import {ssz} from "@lodestar/types"; +import {toHex} from "@lodestar/utils"; +import { + InvalidAttestationErrorCode, + SlashingProtection, + SlashingProtectionAttestation, +} from "../../../src/slashingProtection/index.js"; +import {testLogger} from "../../utils/logger.js"; + +/** + * Min-span entries only exist within `DEFAULT_MAX_EPOCH_LOOKBACK` (4096) epochs below each recorded source epoch. + * A surround vote with an older source epoch, as a malicious or buggy beacon node could serve, is undetectable + * by min-max surround and must be rejected by the lookback window of the latest recorded attestation. + */ +describe("SlashingProtection attestation min-span lookback", () => { + const pubkey = ssz.BLSPubkey.defaultValue(); + let dbLocation: string; + let db: LevelDbController; + let slashingProtection: SlashingProtection; + + beforeEach(async () => { + dbLocation = fs.mkdtempSync(path.join(os.tmpdir(), "lodestar-slashing-protection-")); + db = await LevelDbController.create({name: dbLocation}, {logger: testLogger()}); + slashingProtection = new SlashingProtection(db); + }); + + afterEach(async () => { + await db.close(); + rimraf.sync(dbLocation); + }); + + async function sign(sourceEpoch: number, targetEpoch: number, root = 1): Promise { + const attestation: SlashingProtectionAttestation = {sourceEpoch, targetEpoch, signingRoot: Buffer.alloc(32, root)}; + await slashingProtection.checkAndInsertAttestation(pubkey, attestation); + } + + function importInterchange(attestations: [source: number, target: number][]): Promise { + return slashingProtection.importInterchange( + { + metadata: {interchange_format_version: "5", genesis_validators_root: toHex(ssz.Root.defaultValue())}, + data: [ + { + pubkey: toHex(pubkey), + signed_blocks: [], + signed_attestations: attestations.map(([source, target]) => ({ + source_epoch: String(source), + target_epoch: String(target), + })), + }, + ], + }, + ssz.Root.defaultValue() + ); + } + + function rejectsWith(promise: Promise, code: InvalidAttestationErrorCode): Promise { + return expect(promise).rejects.toThrow(expect.objectContaining({type: expect.objectContaining({code})})); + } + + it("accepts the next attestation of an honest chain", async () => { + await sign(299_999, 300_000); + await expect(sign(300_000, 300_001)).resolves.toBeUndefined(); + }); + + it("rejects a surrounding attestation whose source is older than the min-max span lookback", async () => { + await sign(299_999, 300_000); + await sign(300_000, 300_001); + + // Source 0 surrounds every attestation above but has no minSpan entry (300_000 - 4097 > 0) + await rejectsWith(sign(0, 300_002), InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK); + }); + + it("rejects a surrounding attestation inside the min-max span lookback via min-max spans", async () => { + await sign(10_000, 10_001); + await rejectsWith(sign(9_000, 10_002), InvalidAttestationErrorCode.NEW_SURROUNDS_PREV); + }); + + it("rejects a surrounding attestation inside an offline gap larger than the lookback", async () => { + await sign(99_999, 100_000); + // Validator offline for ~10_000 epochs, then resumes + await sign(109_999, 110_000); + + // Surrounds (109_999, 110_000); minSpan has no entry for 100_500 (below 109_999 - 4097) + await rejectsWith(sign(100_500, 110_001), InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK); + }); + + it("accounts for attestations added by an interchange import", async () => { + await sign(10, 11); + // (0, 1) keeps the interchange lower bound loose so only the lookback window can reject below + await importInterchange([ + [0, 1], + [20_000, 20_001], + ]); + + // Surrounds the imported (20000, 20001); no minSpan entry for 5000 (below 20000 - 4097) + await rejectsWith(sign(5_000, 20_002), InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK); + }); + + it("records nothing from an interchange import that fails validation", async () => { + await sign(10, 11); + await sign(11, 12); + // (9, 14) surrounds (10, 11) so the import must be rejected as a whole, including (12, 13) + await expect( + importInterchange([ + [12, 13], + [9, 14], + ]) + ).rejects.toThrow(); + + expect(await slashingProtection.hasAttestedInEpoch(pubkey, 13)).toBe(false); + await expect(sign(12, 13)).resolves.toBeUndefined(); + }); + + it("rejects a double vote for a target recorded by an interchange import", async () => { + await sign(10, 11); + await sign(11, 12); + await importInterchange([[12, 13]]); + + await rejectsWith(sign(12, 13, 2), InvalidAttestationErrorCode.DOUBLE_VOTE); + }); + + // The oldest epoch with a min-span entry for a recorded source `s` is `s - 1 - DEFAULT_MAX_EPOCH_LOOKBACK` (4096) + it("accepts a non-slashable source epoch at the edge of the min-max span lookback window", async () => { + await sign(10_000, 10_001); + await expect(sign(5_903, 10_000)).resolves.toBeUndefined(); + }); + + it("rejects a source epoch just outside the min-max span lookback window", async () => { + await sign(10_000, 10_001); + await rejectsWith(sign(5_902, 10_000), InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK); + }); +}); From ad3ad0fbc3fda65c6522a1428abccb7bf5ccb1e1 Mon Sep 17 00:00:00 2001 From: Cayman Date: Thu, 3 Sep 2026 17:15:51 -0400 Subject: [PATCH 2/2] fix: reject interchange imports whose latest attestation surrounds one beyond the lookback The source epoch check relies on the highest target attestation having the highest source epoch. Min-max surround does not detect a surround vote beyond its lookback on import, so validate that explicitly before storing the interchange. Restore the original import write order, partial state left behind by a rejected interchange is tracked separately. --- .../slashingProtection/attestation/index.ts | 24 +++++++++++++++++-- .../slashingProtection/attestation.test.ts | 20 +++++++--------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/validator/src/slashingProtection/attestation/index.ts b/packages/validator/src/slashingProtection/attestation/index.ts index 36454ea32007..683ba1932eb2 100644 --- a/packages/validator/src/slashingProtection/attestation/index.ts +++ b/packages/validator/src/slashingProtection/attestation/index.ts @@ -164,13 +164,33 @@ export class SlashingProtectionAttestationService { * Interchange import / export functionality */ async importAttestations(pubkey: BLSPubkey, attestations: SlashingProtectionAttestation[]): Promise { - // Pre-compute spans for all attestations, before storing them so a rejected interchange leaves none behind + // Min-max surround misses a surround vote beyond its lookback, require the highest target attestation to + // have the highest source epoch as `checkAttestation` relies on it + let latestAtt = await this.attestationByTarget.getLatest(pubkey); + let maxSourceAtt = latestAtt; for (const attestation of attestations) { - await this.minMaxSurround.insertAttestation(pubkey, attestation); + if (latestAtt === null || attestation.targetEpoch >= latestAtt.targetEpoch) { + latestAtt = attestation; + } + if (maxSourceAtt === null || attestation.sourceEpoch > maxSourceAtt.sourceEpoch) { + maxSourceAtt = attestation; + } + } + if (latestAtt && maxSourceAtt && latestAtt.sourceEpoch < maxSourceAtt.sourceEpoch) { + throw new InvalidAttestationError({ + code: InvalidAttestationErrorCode.NEW_SURROUNDS_PREV, + attestation: latestAtt, + prev: maxSourceAtt, + }); } await this.attestationByTarget.set(pubkey, attestations); + // Pre-compute spans for all attestations + for (const attestation of attestations) { + await this.minMaxSurround.insertAttestation(pubkey, attestation); + } + // Pre-compute and store lower-bound const minSourceEpoch = minEpoch(attestations.map((attestation) => attestation.sourceEpoch)); const minTargetEpoch = minEpoch(attestations.map((attestation) => attestation.targetEpoch)); diff --git a/packages/validator/test/unit/slashingProtection/attestation.test.ts b/packages/validator/test/unit/slashingProtection/attestation.test.ts index c2a4b5b8f40f..71c2d1291efd 100644 --- a/packages/validator/test/unit/slashingProtection/attestation.test.ts +++ b/packages/validator/test/unit/slashingProtection/attestation.test.ts @@ -102,19 +102,15 @@ describe("SlashingProtection attestation min-span lookback", () => { await rejectsWith(sign(5_000, 20_002), InvalidAttestationErrorCode.SOURCE_BELOW_MIN_SPAN_LOOKBACK); }); - it("records nothing from an interchange import that fails validation", async () => { - await sign(10, 11); - await sign(11, 12); - // (9, 14) surrounds (10, 11) so the import must be rejected as a whole, including (12, 13) - await expect( + it("rejects an interchange import whose highest target attestation surrounds one beyond the lookback", async () => { + // (0, 20000) surrounds (10000, 10001) but 0 is below its min-span coverage + await rejectsWith( importInterchange([ - [12, 13], - [9, 14], - ]) - ).rejects.toThrow(); - - expect(await slashingProtection.hasAttestedInEpoch(pubkey, 13)).toBe(false); - await expect(sign(12, 13)).resolves.toBeUndefined(); + [10_000, 10_001], + [0, 20_000], + ]), + InvalidAttestationErrorCode.NEW_SURROUNDS_PREV + ); }); it("rejects a double vote for a target recorded by an interchange import", async () => {