fix: Do not aggregate validator offenses across epochs in a vote (#16695)

spalladino · ludamad · commit f78d0bc4e117 · 2025-09-02T12:59:19.000Z
diff --git a/yarn-project/slasher/src/tally_slasher_client.test.ts b/yarn-project/slasher/src/tally_slasher_client.test.ts
@@ -245,7 +245,7 @@ describe('TallySlasherClient', () => {
         const targetRound = 3n;
 
         await addPendingOffense({
-          epochOrSlot: targetRound * BigInt(roundSize),
+          epochOrSlot: targetRound * BigInt(roundSize) + BigInt(epochDuration),
           offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
         });
 
diff --git a/yarn-project/slasher/src/tally_slasher_client.ts b/yarn-project/slasher/src/tally_slasher_client.ts
@@ -9,6 +9,7 @@ import type { Prettify } from '@aztec/foundation/types';
 import type { SlasherConfig } from '@aztec/stdlib/interfaces/server';
 import {
   type Offense,
+  OffenseType,
   type ProposerSlashAction,
   type ProposerSlashActionProvider,
   type SlashPayloadRound,
@@ -253,6 +254,7 @@ export class TallySlasherClient implements ProposerSlashActionProvider, SlasherC
     const offensesFromAlwaysSlash = (this.config.slashValidatorsAlways ?? []).map(validator => ({
       validator,
       amount: this.settings.slashingAmounts[2],
+      offenseType: OffenseType.UNKNOWN,
     }));
     const [offensesToForgive, offensesToSlash] = partition([...offensesForRound, ...offensesFromAlwaysSlash], offense =>
       this.config.slashValidatorsNever?.some(v => v.equals(offense.validator)),
@@ -291,7 +293,8 @@ export class TallySlasherClient implements ProposerSlashActionProvider, SlasherC
     });
 
     const committees = await this.collectCommitteesActiveDuringRound(slashedRound);
-    const votes = getSlashConsensusVotesFromOffenses(offensesToSlash, committees, this.settings);
+    const epochsForCommittees = getEpochsForRound(slashedRound, this.settings);
+    const votes = getSlashConsensusVotesFromOffenses(offensesToSlash, committees, epochsForCommittees, this.settings);
     if (votes.every(v => v === 0)) {
       this.log.warn(`Computed votes for offenses are all zero. Skipping vote.`, {
         slotNumber,
diff --git a/yarn-project/stdlib/src/slashing/helpers.ts b/yarn-project/stdlib/src/slashing/helpers.ts
@@ -106,12 +106,23 @@ export function getSlotForOffense(
   return getTimeUnitForOffense(offenseType) === 'epoch' ? epochOrSlot * BigInt(constants.epochDuration) : epochOrSlot;
 }
 
-/** Returns the epoch for a given offense. */
+/** Returns the epoch for a given offense. If the offense type or epoch is not defined, returns undefined. */
 export function getEpochForOffense(
   offense: Pick<Offense, 'epochOrSlot' | 'offenseType'>,
   constants: Pick<L1RollupConstants, 'epochDuration'>,
-): bigint {
+): bigint;
+export function getEpochForOffense(
+  offense: Partial<Pick<Offense, 'epochOrSlot' | 'offenseType'>>,
+  constants: Pick<L1RollupConstants, 'epochDuration'>,
+): bigint | undefined;
+export function getEpochForOffense(
+  offense: Partial<Pick<Offense, 'epochOrSlot' | 'offenseType'>>,
+  constants: Pick<L1RollupConstants, 'epochDuration'>,
+): bigint | undefined {
   const { epochOrSlot, offenseType } = offense;
+  if (epochOrSlot === undefined || offenseType === undefined) {
+    return undefined;
+  }
   return getTimeUnitForOffense(offenseType) === 'epoch' ? epochOrSlot : epochOrSlot / BigInt(constants.epochDuration);
 }
 
diff --git a/yarn-project/stdlib/src/slashing/tally.test.ts b/yarn-project/stdlib/src/slashing/tally.test.ts
@@ -10,7 +10,10 @@ describe('TallySlashingHelpers', () => {
   const mockValidator4 = EthAddress.fromString('0x4567890123456789012345678901234567890123');
 
   describe('getSlashConsensusVotesFromOffenses', () => {
-    const settings = { slashingAmounts: [10n, 20n, 30n] as [bigint, bigint, bigint] };
+    const settings = {
+      slashingAmounts: [10n, 20n, 30n] as [bigint, bigint, bigint],
+      epochDuration: 32,
+    };
 
     it('creates votes based on offenses and committees', () => {
       const offenses: Offense[] = [
@@ -35,12 +38,13 @@ describe('TallySlashingHelpers', () => {
       ];
 
       const committees = [[mockValidator1, mockValidator2, mockValidator3]];
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees = [5n]; // Committee for epoch 5
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toHaveLength(3);
-      expect(votes[0]).toEqual(3); // 30 / 10 = 3 slash units for validator1
-      expect(votes[1]).toEqual(0); // 5 / 10 = 0 slash units for validator2
-      expect(votes[2]).toEqual(0); // 0 / 10 = 0 slash units for validator3
+      expect(votes[0]).toEqual(2); // Only 25n from epoch 5 offense for validator1
+      expect(votes[1]).toEqual(0); // Offense is in slot 10, which is epoch 0, not 5
+      expect(votes[2]).toEqual(0); // No offenses for validator3
     });
 
     it('caps slash units at maximum per validator', () => {
@@ -54,7 +58,8 @@ describe('TallySlashingHelpers', () => {
       ];
 
       const committees = [[mockValidator1]];
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees = [5n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toHaveLength(1);
       expect(votes[0]).toEqual(3); // Capped at MAX_SLASH_UNITS_PER_VALIDATOR
@@ -81,7 +86,8 @@ describe('TallySlashingHelpers', () => {
         [mockValidator3, mockValidator4],
       ];
 
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees = [5n, 6n]; // Committees for epochs 5 and 6
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toHaveLength(4);
       expect(votes[0]).toEqual(2); // validator1 in committee1
@@ -90,7 +96,7 @@ describe('TallySlashingHelpers', () => {
       expect(votes[3]).toEqual(3); // validator4 in committee2
     });
 
-    it('does not repeat slashes for the same validator in different committees', () => {
+    it('correctly handles validators appearing in multiple committees with different epochs', () => {
       const offenses: Offense[] = [
         {
           validator: mockValidator1,
@@ -110,13 +116,14 @@ describe('TallySlashingHelpers', () => {
         [mockValidator1, mockValidator2],
         [mockValidator1, mockValidator3],
       ];
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees = [5n, 6n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toHaveLength(4);
-      expect(votes[0]).toEqual(3); // validator1 in committee1
-      expect(votes[1]).toEqual(0); // validator2 in committee1
-      expect(votes[2]).toEqual(0); // validator1 in committee2
-      expect(votes[3]).toEqual(0); // validator3 in committee2
+      expect(votes[0]).toEqual(2); // validator1 in committee1, epoch 5 offense (20n)
+      expect(votes[1]).toEqual(0); // validator2 in committee1, no offenses
+      expect(votes[2]).toEqual(1); // validator1 in committee2, epoch 6 offense (10n)
+      expect(votes[3]).toEqual(0); // validator3 in committee2, no offenses
     });
 
     it('returns empty votes for empty committees', () => {
@@ -130,7 +137,8 @@ describe('TallySlashingHelpers', () => {
       ];
 
       const committees: EthAddress[][] = [];
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees: bigint[] = [];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toEqual([]);
     });
@@ -146,12 +154,211 @@ describe('TallySlashingHelpers', () => {
       ];
 
       const committees = [[mockValidator2, mockValidator3]];
-      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, settings);
+      const epochsForCommittees = [5n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
 
       expect(votes).toHaveLength(2);
       expect(votes[0]).toEqual(0); // validator2 has no offenses
       expect(votes[1]).toEqual(0); // validator3 has no offenses
     });
+
+    it('handles offenses without epochOrSlot (slashValidatorsAlways)', () => {
+      const offenses = [
+        {
+          validator: mockValidator1,
+          amount: 30n,
+          offenseType: OffenseType.UNKNOWN,
+          epochOrSlot: undefined, // No epoch/slot for always-slash validators
+        },
+        {
+          validator: mockValidator2,
+          amount: 10n,
+          offenseType: OffenseType.INACTIVITY,
+          epochOrSlot: 5n,
+        },
+      ];
+
+      const committees = [
+        [mockValidator1, mockValidator2],
+        [mockValidator1, mockValidator3],
+      ];
+      const epochsForCommittees = [5n, 6n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(4);
+      expect(votes[0]).toEqual(3); // validator1 in committee1, always-slash (30n)
+      expect(votes[1]).toEqual(1); // validator2 in committee1, epoch 5 offense (10n)
+      expect(votes[2]).toEqual(3); // validator1 in committee2, always-slash (30n)
+      expect(votes[3]).toEqual(0); // validator3 in committee2, no offenses
+    });
+
+    it('correctly converts slot-based offenses to epochs', () => {
+      const offenses: Offense[] = [
+        {
+          validator: mockValidator1,
+          amount: 15n,
+          offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
+          epochOrSlot: 64n, // slot 64 = epoch 2 (64/32)
+        },
+        {
+          validator: mockValidator2,
+          amount: 20n,
+          offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
+          epochOrSlot: 95n, // slot 95 = epoch 2 (95/32 = 2.96... -> 2)
+        },
+      ];
+
+      const committees = [[mockValidator1, mockValidator2, mockValidator3]];
+      const epochsForCommittees = [2n]; // Committee for epoch 2
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(3);
+      expect(votes[0]).toEqual(1); // validator1: 15n offense maps to epoch 2
+      expect(votes[1]).toEqual(2); // validator2: 20n offense maps to epoch 2
+      expect(votes[2]).toEqual(0); // validator3: no offenses
+    });
+
+    it('handles mixed epoch and slot-based offenses resolving to same epoch', () => {
+      const offenses: Offense[] = [
+        {
+          validator: mockValidator1,
+          amount: 10n,
+          offenseType: OffenseType.INACTIVITY, // epoch-based
+          epochOrSlot: 2n, // epoch 2
+        },
+        {
+          validator: mockValidator1,
+          amount: 15n,
+          offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
+          epochOrSlot: 75n, // slot 75 = epoch 2 (75/32 = 2.34... -> 2)
+        },
+      ];
+
+      const committees = [[mockValidator1, mockValidator2]];
+      const epochsForCommittees = [2n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(2);
+      expect(votes[0]).toEqual(2); // validator1: 10n + 15n = 25n total for epoch 2
+      expect(votes[1]).toEqual(0); // validator2: no offenses
+    });
+
+    it('sums multiple offenses for same validator in same epoch', () => {
+      const offenses: Offense[] = [
+        {
+          validator: mockValidator1,
+          amount: 8n,
+          offenseType: OffenseType.INACTIVITY,
+          epochOrSlot: 3n,
+        },
+        {
+          validator: mockValidator1,
+          amount: 7n,
+          offenseType: OffenseType.DATA_WITHHOLDING,
+          epochOrSlot: 3n,
+        },
+        {
+          validator: mockValidator1,
+          amount: 5n,
+          offenseType: OffenseType.VALID_EPOCH_PRUNED,
+          epochOrSlot: 3n,
+        },
+      ];
+
+      const committees = [[mockValidator1, mockValidator2]];
+      const epochsForCommittees = [3n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(2);
+      expect(votes[0]).toEqual(2); // validator1: 8n + 7n + 5n = 20n total
+      expect(votes[1]).toEqual(0); // validator2: no offenses
+    });
+
+    it('handles always-slash validator with additional epoch-specific offenses', () => {
+      const offenses = [
+        {
+          validator: mockValidator1,
+          amount: 20n, // always-slash
+          offenseType: OffenseType.UNKNOWN,
+          epochOrSlot: undefined,
+        },
+        {
+          validator: mockValidator1,
+          amount: 15n, // epoch-specific
+          offenseType: OffenseType.INACTIVITY,
+          epochOrSlot: 5n,
+        },
+      ];
+
+      const committees = [
+        [mockValidator1, mockValidator2],
+        [mockValidator1, mockValidator3],
+      ];
+      const epochsForCommittees = [5n, 6n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(4);
+      expect(votes[0]).toEqual(3); // validator1 committee1: 20n(always) + 15n(epoch5) = 35n
+      expect(votes[1]).toEqual(0); // validator2: no offenses
+      expect(votes[2]).toEqual(2); // validator1 committee2: 20n(always) only
+      expect(votes[3]).toEqual(0); // validator3: no offenses
+    });
+
+    it('handles epoch boundary conditions', () => {
+      const offenses: Offense[] = [
+        {
+          validator: mockValidator1,
+          amount: 15n,
+          offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
+          epochOrSlot: 31n, // slot 31 = epoch 0 (31/32 = 0.96... -> 0)
+        },
+        {
+          validator: mockValidator2,
+          amount: 20n,
+          offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, // slot-based
+          epochOrSlot: 32n, // slot 32 = epoch 1 (32/32 = 1)
+        },
+      ];
+
+      const committees = [
+        [mockValidator1, mockValidator2],
+        [mockValidator1, mockValidator2],
+      ];
+      const epochsForCommittees = [0n, 1n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(4);
+      expect(votes[0]).toEqual(1); // validator1 epoch0: 15n offense
+      expect(votes[1]).toEqual(0); // validator2 epoch0: no matching offenses
+      expect(votes[2]).toEqual(0); // validator1 epoch1: no matching offenses
+      expect(votes[3]).toEqual(2); // validator2 epoch1: 20n offense
+    });
+
+    it('handles zero amount offenses', () => {
+      const offenses: Offense[] = [
+        {
+          validator: mockValidator1,
+          amount: 0n,
+          offenseType: OffenseType.INACTIVITY,
+          epochOrSlot: 5n,
+        },
+        {
+          validator: mockValidator2,
+          amount: 15n,
+          offenseType: OffenseType.INACTIVITY,
+          epochOrSlot: 5n,
+        },
+      ];
+
+      const committees = [[mockValidator1, mockValidator2, mockValidator3]];
+      const epochsForCommittees = [5n];
+      const votes = getSlashConsensusVotesFromOffenses(offenses, committees, epochsForCommittees, settings);
+
+      expect(votes).toHaveLength(3);
+      expect(votes[0]).toEqual(0); // validator1: 0n amount = 0 slash units
+      expect(votes[1]).toEqual(1); // validator2: 15n amount = 1 slash unit
+      expect(votes[2]).toEqual(0); // validator3: no offenses
+    });
   });
 
   describe('encodeSlashConsensusVotes', () => {
diff --git a/yarn-project/stdlib/src/slashing/tally.ts b/yarn-project/stdlib/src/slashing/tally.ts
