fix: extend slashing of bad attestations

Author: alexghr

yarn-project/aztec-node/src/aztec-node/server.ts

@@ -757,7 +757,10 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb
           watchers.push(dataWithholdingWatcher);
         }
 
-        if (config.slashBroadcastedInvalidCheckpointProposalPenalty > 0n) {
+        if (
+          config.slashBroadcastedInvalidCheckpointProposalPenalty > 0n ||
+          config.slashAttestInvalidCheckpointProposalPenalty > 0n
+        ) {
           broadcastedInvalidCheckpointProposalWatcher = new BroadcastedInvalidCheckpointProposalWatcher(
             p2pClient,
             archiver,

yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts

@@ -394,11 +394,6 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
         validator: badProposer,
         offenseType: OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL,
       },
-      {
-        description: 'lazy validator attested to invalid checkpoint proposal',
-        validator: lazyValidator,
-        offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL,
-      },
     ];
 
     const offensesWithExpectedSlashes = await retryUntil(
@@ -415,7 +410,7 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
           ? currentOffenses
           : undefined;
       },
-      'honest validator slash offenses for invalid proposal attestation',
+      'honest validator slash offenses for invalid block proposal',
       OFFENSE_DETECTION_TIMEOUT,
       1,
     );
@@ -425,6 +420,14 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
       expect(offense.amount).toBeGreaterThan(0n);
       t.logger.warn(`Observed expected slash offense: ${description}`, { offense });
     }
+    expect(
+      findSlashOffense(
+        offensesWithExpectedSlashes,
+        lazyValidator,
+        OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL,
+        targetSlot,
+      ),
+    ).toBeUndefined();
 
     return {
       rollup,
@@ -439,7 +442,7 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
     };
   }
 
-  it('slashes a lazy attester for an invalid checkpoint and clears it on delayed equivocation', async () => {
+  it('does not slash a lazy checkpoint attester for invalid block evidence', async () => {
     const {
       rollup,
       badProposerNode,
@@ -531,7 +534,7 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
     const offensesAfterClear = await retryUntil(
       async () => {
         const currentOffenses = await honestValidatorNode.getSlashOffenses('all');
-        const badAttestationOffense = findSlashOffense(
+        const lazyAttestationOffense = findSlashOffense(
           currentOffenses,
           lazyValidator,
           OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL,
@@ -544,16 +547,16 @@ describe('e2e_slashing_attested_invalid_proposal', () => {
           targetSlot,
         );
 
-        t.logger.warn('Waiting for delayed equivocation to clear bad attestation slash', {
+        t.logger.warn('Waiting for delayed equivocation without a lazy attestation slash', {
           targetSlot,
-          badAttestationOffense,
+          lazyAttestationOffense,
           duplicateProposalOffense,
           currentOffenses,
         });
 
-        return !badAttestationOffense && duplicateProposalOffense ? currentOffenses : undefined;
+        return !lazyAttestationOffense && duplicateProposalOffense ? currentOffenses : undefined;
       },
-      'bad attestation slash cleared after delayed block proposal equivocation',
+      'delayed block proposal equivocation recorded without lazy attester slash',
       OFFENSE_DETECTION_TIMEOUT,
       1,
     );

yarn-project/end-to-end/src/e2e_slashing/broadcasted_invalid_checkpoint_proposal_slash.test.ts

@@ -77,7 +77,7 @@ async function awaitBroadcastedInvalidCheckpointOffense({
       const offenses = await node.getSlashOffenses('all');
       return findBroadcastedInvalidCheckpointOffense(offenses, validator, slot);
     },
-    `A-520 offense for slot ${slot}`,
+    `broadcasted invalid checkpoint proposal offense for slot ${slot}`,
     AZTEC_SLOT_DURATION * 3,
     1,
   );

yarn-project/slasher/README.md

@@ -123,8 +123,8 @@ List of all slashable offenses in the system:
 **Time Unit**: Slot-based offense.
 
 ### ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL
-**Description**: A committee member attested to a checkpoint proposal in a slot where this node detected a slashable invalid block proposal.
-**Detection**: ValidatorClient marks slots with invalid block proposals detected via reexecution and slashes checkpoint attesters seen for that slot. If proposal equivocation is later detected for the slot, pending bad-attestation offenses are cleared.
+**Description**: A committee member attested to a checkpoint proposal in a slot where this node detected a slashable invalid block or checkpoint proposal.
+**Detection**: ValidatorClient marks slots with invalid block proposals detected via reexecution and invalid checkpoint proposals detected via deterministic validation, then slashes checkpoint attesters seen for that slot. BroadcastedInvalidCheckpointProposalWatcher also scans retained truncated-checkpoint evidence and retained attestations for the same slot. If proposal equivocation is later detected for the slot, pending bad-attestation offenses are cleared.
 **Target**: Committee members who attested in the invalid proposal slot.
 **Time Unit**: Slot-based offense.
 

yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts

@@ -10,6 +10,7 @@ import { OffenseType } from '@aztec/stdlib/slashing';
 import {
   makeBlockHeader,
   makeBlockProposal,
+  makeCheckpointAttestation,
   makeCheckpointHeader,
   makeCheckpointProposal,
 } from '@aztec/stdlib/testing';
@@ -22,15 +23,16 @@ import { WANT_TO_SLASH_EVENT, type WantToSlashArgs } from '../watcher.js';
 import { BroadcastedInvalidCheckpointProposalWatcher } from './broadcasted_invalid_checkpoint_proposal_watcher.js';
 
 describe('BroadcastedInvalidCheckpointProposalWatcher', () => {
-  let p2pClient: MockProxy<Pick<P2PClient, 'getProposalsForSlot'>>;
+  let p2pClient: MockProxy<Pick<P2PClient, 'getCheckpointAttestationsForSlot' | 'getProposalsForSlot'>>;
   let l2BlockSource: MockProxy<Pick<L2BlockSource, 'getSyncedL2SlotNumber'>>;
   let epochCache: MockProxy<Pick<EpochCacheInterface, 'getSlotNow' | 'getL1Constants'>>;
   let config: SlasherConfig;
   let watcher: BroadcastedInvalidCheckpointProposalWatcher;
   let handler: jest.MockedFunction<(args: WantToSlashArgs[]) => void>;
 
   beforeEach(() => {
-    p2pClient = mock<Pick<P2PClient, 'getProposalsForSlot'>>();
+    p2pClient = mock<Pick<P2PClient, 'getCheckpointAttestationsForSlot' | 'getProposalsForSlot'>>();
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([]);
     l2BlockSource = mock<Pick<L2BlockSource, 'getSyncedL2SlotNumber'>>();
     l2BlockSource.getSyncedL2SlotNumber.mockResolvedValue(SlotNumber(12));
     epochCache = mock<Pick<EpochCacheInterface, 'getSlotNow' | 'getL1Constants'>>();
@@ -43,6 +45,7 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => {
     config = {
       ...DefaultSlasherConfig,
       slashBroadcastedInvalidCheckpointProposalPenalty: 11n,
+      slashAttestInvalidCheckpointProposalPenalty: 13n,
     };
     watcher = new BroadcastedInvalidCheckpointProposalWatcher(p2pClient, l2BlockSource, epochCache, config, 4);
     handler = jest.fn();
@@ -112,6 +115,121 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => {
     ]);
   });
 
+  it('does not slash attesters from retained truncated-checkpoint evidence', async () => {
+    const signer = Secp256k1Signer.random();
+    const attester = Secp256k1Signer.random();
+    const slot = SlotNumber(10);
+    const blocks = await makeBlocks(signer, slot, 4);
+    const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]);
+    const attestation = makeCheckpointAttestation({
+      header: makeCheckpointHeader(1, { slotNumber: slot }),
+      attesterSigner: attester,
+    });
+    mockProposals(slot, blocks, [checkpoint]);
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]);
+
+    await watcher.scanSlot(slot);
+
+    expect(handler).toHaveBeenCalledWith([
+      {
+        validator: signer.address,
+        amount: 11n,
+        offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL,
+        epochOrSlot: 10n,
+      },
+    ]);
+  });
+
+  it('does not emit attester offenses when proposer checkpoint slashing is disabled', async () => {
+    watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n });
+    const signer = Secp256k1Signer.random();
+    const attester = Secp256k1Signer.random();
+    const slot = SlotNumber(10);
+    const blocks = await makeBlocks(signer, slot, 4);
+    const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]);
+    const attestation = makeCheckpointAttestation({
+      header: makeCheckpointHeader(1, { slotNumber: slot }),
+      attesterSigner: attester,
+    });
+    mockProposals(slot, blocks, [checkpoint]);
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]);
+
+    await watcher.scanSlot(slot);
+
+    expect(handler).not.toHaveBeenCalled();
+  });
+
+  it('does not slash attesters when bad attestation slashing is disabled', async () => {
+    watcher.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 0n });
+    const signer = Secp256k1Signer.random();
+    const attester = Secp256k1Signer.random();
+    const slot = SlotNumber(10);
+    const blocks = await makeBlocks(signer, slot, 4);
+    const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]);
+    const attestation = makeCheckpointAttestation({
+      header: makeCheckpointHeader(1, { slotNumber: slot }),
+      attesterSigner: attester,
+    });
+    mockProposals(slot, blocks, [checkpoint]);
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]);
+
+    await watcher.scanSlot(slot);
+
+    expect(handler).toHaveBeenCalledWith([
+      {
+        validator: signer.address,
+        amount: 11n,
+        offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL,
+        epochOrSlot: 10n,
+      },
+    ]);
+  });
+
+  it('does not emit bad attestation offenses for equivocated checkpoint proposal slots', async () => {
+    watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n });
+    const signer = Secp256k1Signer.random();
+    const attester = Secp256k1Signer.random();
+    const slot = SlotNumber(10);
+    const blocks = await makeBlocks(signer, slot, 4);
+    const truncatedCheckpoint = await makeCheckpointCore(signer, slot, blocks[1]);
+    const otherCheckpoint = await makeCheckpointCore(signer, slot, blocks[3]);
+    const attestation = makeCheckpointAttestation({
+      header: makeCheckpointHeader(1, { slotNumber: slot }),
+      attesterSigner: attester,
+    });
+    mockProposals(slot, blocks, [truncatedCheckpoint, otherCheckpoint]);
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]);
+
+    await watcher.scanSlot(slot);
+
+    expect(handler).not.toHaveBeenCalled();
+  });
+
+  it('does not emit bad attestation offenses for equivocated block proposal slots', async () => {
+    watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n });
+    const signer = Secp256k1Signer.random();
+    const attester = Secp256k1Signer.random();
+    const slot = SlotNumber(10);
+    const blocks = await makeBlocks(signer, slot, 4);
+    const equivocatedBlock = await makeBlockProposal({
+      signer,
+      blockHeader: makeBlockHeader(99, { slotNumber: slot }),
+      archiveRoot: Fr.random(),
+      indexWithinCheckpoint: IndexWithinCheckpoint(2),
+    });
+    const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]);
+    const attestation = makeCheckpointAttestation({
+      header: makeCheckpointHeader(1, { slotNumber: slot }),
+      attesterSigner: attester,
+    });
+    mockProposals(slot, [...blocks, equivocatedBlock], [checkpoint]);
+    p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]);
+
+    await watcher.scanSlot(slot);
+
+    expect(handler).not.toHaveBeenCalled();
+  });
+
   it('slashes when a higher-index proposal arrives after an earlier non-slashing scan', async () => {
     const signer = Secp256k1Signer.random();
     const slot = SlotNumber(10);

yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts

@@ -2,7 +2,6 @@ import type { EpochCacheInterface } from '@aztec/epoch-cache';
 import { SlotNumber } from '@aztec/foundation/branded-types';
 import { merge, pick } from '@aztec/foundation/collection';
 import type { EthAddress } from '@aztec/foundation/eth-address';
-import { FifoSet } from '@aztec/foundation/fifo-set';
 import { type Logger, createLogger } from '@aztec/foundation/log';
 import { RunningPromise } from '@aztec/foundation/running-promise';
 import type { L2BlockSource } from '@aztec/stdlib/block';
@@ -16,6 +15,7 @@ import { WANT_TO_SLASH_EVENT, type WantToSlashArgs, type Watcher, type WatcherEm
 
 const BroadcastedInvalidCheckpointProposalWatcherConfigKeys = [
   'slashBroadcastedInvalidCheckpointProposalPenalty',
+  'slashAttestInvalidCheckpointProposalPenalty',
 ] as const;
 
 const SCAN_SLOT_LAG = 1;
@@ -34,14 +34,14 @@ type SignedBlockProposal = {
   signer: EthAddress;
 };
 
-/** Detects truncated-checkpoint proposal offenses from retained signed P2P proposals. */
+/** Detects truncated-checkpoint proposer offenses from retained P2P evidence. */
 export class BroadcastedInvalidCheckpointProposalWatcher
   extends (EventEmitter as new () => WatcherEmitter)
   implements Watcher
 {
   private readonly log: Logger = createLogger('broadcasted-invalid-checkpoint-proposal-watcher');
   private readonly runningPromise: RunningPromise;
-  private readonly emittedOffenses: FifoSet<string>;
+  private readonly emittedOffensesBySlot = new Map<bigint, Set<string>>();
   private readonly scanSlotLookback: number;
   private config: BroadcastedInvalidCheckpointProposalWatcherConfig;
   private lastScannedSlot: SlotNumber | undefined;
@@ -58,11 +58,6 @@ export class BroadcastedInvalidCheckpointProposalWatcher
     this.config = pick(config, ...BroadcastedInvalidCheckpointProposalWatcherConfigKeys);
     this.scanSlotLookback = Math.max(1, scanSlotLookback);
 
-    // Bound emitted offenses to the number of slots we rescan. This watcher currently tracks one offense type,
-    // and at most one offense of that type can be emitted per slot.
-    const offenseTypes = 1;
-    this.emittedOffenses = FifoSet.withLimit<string>(offenseTypes * this.scanSlotLookback);
-
     const intervalMs = Math.max(1000, (constants.ethereumSlotDuration * 1000) / 4);
     this.runningPromise = new RunningPromise(() => this.scan(), this.log, intervalMs);
     this.log.info('BroadcastedInvalidCheckpointProposalWatcher initialized', {
@@ -107,6 +102,7 @@ export class BroadcastedInvalidCheckpointProposalWatcher
       await this.scanSlot(SlotNumber(slot));
     }
     this.lastScannedSlot = newestSlotToConsider;
+    this.pruneEmittedOffensesBefore(oldestLookbackSlot);
   }
 
   /** Scans a single slot. Public for tests. */
@@ -134,7 +130,10 @@ export class BroadcastedInvalidCheckpointProposalWatcher
 
   private getSlashArgsForProposals(slot: SlotNumber, proposals: ProposalsForSlot): WantToSlashArgs[] {
     const offenders = this.findOffenders(proposals.blockProposals, proposals.checkpointProposals);
-    // we expect one proposer per slot today.
+    if (offenders.size === 0) {
+      return [];
+    }
+
     return [...offenders.values()].map(validator => ({
       validator,
       amount: this.config.slashBroadcastedInvalidCheckpointProposalPenalty,
@@ -193,7 +192,23 @@ export class BroadcastedInvalidCheckpointProposalWatcher
   }
 
   private markAsNewOffense(args: WantToSlashArgs): boolean {
-    const key = `${args.validator.toString()}-${args.offenseType}-${args.epochOrSlot}`;
-    return this.emittedOffenses.addIfAbsent(key);
+    const key = `${args.validator.toString()}-${args.offenseType}`;
+    const slotOffenses = this.emittedOffensesBySlot.get(args.epochOrSlot) ?? new Set<string>();
+    if (slotOffenses.has(key)) {
+      return false;
+    }
+
+    slotOffenses.add(key);
+    this.emittedOffensesBySlot.set(args.epochOrSlot, slotOffenses);
+    return true;
+  }
+
+  private pruneEmittedOffensesBefore(slot: SlotNumber): void {
+    const oldestRetainedSlot = BigInt(slot);
+    for (const emittedSlot of this.emittedOffensesBySlot.keys()) {
+      if (emittedSlot < oldestRetainedSlot) {
+        this.emittedOffensesBySlot.delete(emittedSlot);
+      }
+    }
   }
 }

yarn-project/stdlib/src/slashing/types.ts

@@ -22,7 +22,7 @@ export enum OffenseType {
   DUPLICATE_PROPOSAL = 8,
   /** A validator signed attestations for different proposals at the same slot (equivocation) */
   DUPLICATE_ATTESTATION = 9,
-  /** A committee member attested to a checkpoint proposal in a slot with an invalid block proposal */
+  /** A committee member attested to a checkpoint proposal in a slot with an invalid block or checkpoint proposal */
   ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL = 10,
   /** A proposer broadcast an invalid checkpoint proposal, detected by retained evidence or deterministic recomputation */
   BROADCASTED_INVALID_CHECKPOINT_PROPOSAL = 11,