diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index f3070003ed83..f4be06db0b6e 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -743,34 +743,30 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb if (!proverOnly) { validatorsSentinel = await createSentinel(epochCache, archiver, p2pClient, reexecutionTracker, config); - if (validatorsSentinel && config.slashInactivityPenalty > 0n) { + if (validatorsSentinel) { watchers.push(validatorsSentinel); } - if (config.slashDataWithholdingPenalty > 0n) { - dataWithholdingWatcher = new DataWithholdingWatcher( - epochCache, - archiver, - p2pClient.getTxProvider(), - p2pClient, - reexecutionTracker, - { chainId: config.l1ChainId, rollupAddress: config.rollupAddress }, - config, - ); - watchers.push(dataWithholdingWatcher); - } + dataWithholdingWatcher = new DataWithholdingWatcher( + epochCache, + archiver, + p2pClient.getTxProvider(), + p2pClient, + reexecutionTracker, + { chainId: config.l1ChainId, rollupAddress: config.rollupAddress }, + config, + ); + watchers.push(dataWithholdingWatcher); - if (config.slashBroadcastedInvalidCheckpointProposalPenalty > 0n) { - broadcastedInvalidCheckpointProposalWatcher = new BroadcastedInvalidCheckpointProposalWatcher( - p2pClient, - archiver, - epochCache, - config, - ); - watchers.push(broadcastedInvalidCheckpointProposalWatcher); - } + broadcastedInvalidCheckpointProposalWatcher = new BroadcastedInvalidCheckpointProposalWatcher( + p2pClient, + archiver, + epochCache, + config, + ); + watchers.push(broadcastedInvalidCheckpointProposalWatcher); - if (validatorClient && config.slashAttestInvalidCheckpointProposalPenalty > 0n) { + if (validatorClient) { attestedInvalidProposalWatcher = new AttestedInvalidProposalWatcher( p2pClient, validatorClient, @@ -782,19 +778,11 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, AztecNodeDeb watchers.push(attestedInvalidProposalWatcher); } - if (config.slashDuplicateProposalPenalty > 0n) { - checkpointEquivocationWatcher = new CheckpointEquivocationWatcher(archiver, epochCache, config); - watchers.push(checkpointEquivocationWatcher); - } + checkpointEquivocationWatcher = new CheckpointEquivocationWatcher(archiver, epochCache, config); + watchers.push(checkpointEquivocationWatcher); - // We assume we want to slash for invalid attestations unless all max penalties are set to 0 - if ( - config.slashProposeInvalidAttestationsPenalty > 0n || - config.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty > 0n - ) { - attestationsBlockWatcher = new AttestationsBlockWatcher(archiver, epochCache, config, log.getBindings()); - watchers.push(attestationsBlockWatcher); - } + attestationsBlockWatcher = new AttestationsBlockWatcher(archiver, epochCache, config, log.getBindings()); + watchers.push(attestationsBlockWatcher); } const watchersToStart = compactArray([ diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts index d28d49bc81a8..966a72b34fb9 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts @@ -986,6 +986,24 @@ describe('sentinel', () => { ]); }); + it('emits zero-amount inactivity offenses when the penalty is zero', async () => { + sentinel.updateConfig({ slashInactivityPenalty: 0n, slashInactivityConsecutiveEpochThreshold: 1 }); + const emitSpy = jest.spyOn(sentinel, 'emit'); + + await sentinel.handleEpochPerformance(EpochNumber(5), { + [validator1.toString()]: { missed: 8, total: 10 }, + }); + + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_SLASH_EVENT, [ + { + validator: validator1, + amount: 0n, + offenseType: OffenseType.INACTIVITY, + epochOrSlot: 5n, + }, + ]); + }); + it('should not slash when no validators meet consecutive threshold', async () => { // Update config to require 3 consecutive epochs sentinel.updateConfig({ slashInactivityConsecutiveEpochThreshold: 3 }); diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.ts b/yarn-project/aztec-node/src/sentinel/sentinel.ts index 53d1dcc70099..a9746cfa0643 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.ts @@ -335,10 +335,6 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme } protected async handleEpochPerformance(epoch: EpochNumber, performance: ValidatorsEpochPerformance) { - if (this.config.slashInactivityPenalty === 0n) { - return; - } - const inactiveValidators = getEntries(performance) .filter(([_, { missed, total }]) => total > 0 && missed / total >= this.config.slashInactivityTargetPercentage) .map(([address]) => address); @@ -363,8 +359,8 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme if (criminals.length > 0) { this.logger.verbose( - `Identified ${criminals.length} validators to slash due to inactivity in at least ${epochThreshold} consecutive epochs`, - { ...args, epochThreshold }, + `Identified ${criminals.length} inactivity offenses in at least ${epochThreshold} consecutive epochs`, + { offenses: args, epochThreshold }, ); this.emit(WANT_TO_SLASH_EVENT, args); } diff --git a/yarn-project/slasher/src/config.ts b/yarn-project/slasher/src/config.ts index ddc934f02b62..64ca1e97c6c1 100644 --- a/yarn-project/slasher/src/config.ts +++ b/yarn-project/slasher/src/config.ts @@ -71,7 +71,7 @@ export const slasherConfigMappings: ConfigMappingsType = { }, slashDataWithholdingPenalty: { env: 'SLASH_DATA_WITHHOLDING_PENALTY', - description: 'Penalty amount for slashing validators for data withholding (set to 0 to disable).', + description: 'Penalty for data withholding (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashDataWithholdingPenalty), }, slashDataWithholdingToleranceSlots: { @@ -125,29 +125,28 @@ export const slasherConfigMappings: ConfigMappingsType = { }, slashInactivityPenalty: { env: 'SLASH_INACTIVITY_PENALTY', - description: 'Penalty amount for slashing an inactive validator (set to 0 to disable).', + description: 'Penalty for an inactive validator (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashInactivityPenalty), }, slashProposeInvalidAttestationsPenalty: { env: 'SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY', - description: 'Penalty amount for slashing a proposer that proposed invalid attestations (set to 0 to disable).', + description: 'Penalty for proposing invalid attestations (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashProposeInvalidAttestationsPenalty), }, slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty: { env: 'SLASH_PROPOSE_DESCENDANT_OF_CHECKPOINT_WITH_INVALID_ATTESTATIONS_PENALTY', description: - 'Penalty amount for slashing a proposer that published a checkpoint building on an invalid checkpoint (set to 0 to disable).', + 'Penalty for publishing a checkpoint building on an invalid checkpoint (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashProposeDescendantOfCheckpointWithInvalidAttestationsPenalty), }, slashAttestInvalidCheckpointProposalPenalty: { env: 'SLASH_ATTEST_INVALID_CHECKPOINT_PROPOSAL_PENALTY', - description: - 'Penalty amount for slashing a validator that attested to an invalid checkpoint proposal (set to 0 to disable).', + description: 'Penalty for attesting to an invalid checkpoint proposal (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashAttestInvalidCheckpointProposalPenalty), }, slashUnknownPenalty: { env: 'SLASH_UNKNOWN_PENALTY', - description: 'Penalty amount for slashing a validator for an unknown offense (set to 0 to disable).', + description: 'Penalty for an unknown offense (0 records offenses without slash votes).', ...bigintConfigHelper(DefaultSlasherConfig.slashUnknownPenalty), }, slashOffenseExpirationRounds: { diff --git a/yarn-project/slasher/src/slasher_client.test.ts b/yarn-project/slasher/src/slasher_client.test.ts index fa657c55bdc5..beb78674a5f7 100644 --- a/yarn-project/slasher/src/slasher_client.test.ts +++ b/yarn-project/slasher/src/slasher_client.test.ts @@ -301,6 +301,24 @@ describe('SlasherClient', () => { const actions = await slasherClient.getProposerActions(SlotNumber.fromBigInt(currentSlot)); expect(actions).toHaveLength(0); }); + + it('should not return any action for zero-amount offenses', async () => { + const currentRound = 5n; + const currentSlot = currentRound * BigInt(roundSize); + const targetRound = 3n; + + await offensesStore.addOffense( + createOffense({ + validator: committee[0], + epochOrSlot: targetRound * BigInt(roundSize), + offenseType: OffenseType.PROPOSED_INSUFFICIENT_ATTESTATIONS, + amount: 0n, + }), + ); + + const actions = await slasherClient.getProposerActions(SlotNumber.fromBigInt(currentSlot)); + expect(actions).toHaveLength(0); + }); }); describe('execute-slash', () => { diff --git a/yarn-project/slasher/src/slasher_client.ts b/yarn-project/slasher/src/slasher_client.ts index 4ec78bb55b79..f0b748a9cec7 100644 --- a/yarn-project/slasher/src/slasher_client.ts +++ b/yarn-project/slasher/src/slasher_client.ts @@ -343,7 +343,7 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient return undefined; } - this.log.info(`Voting to slash ${offensesToSlash.length} offenses`, { + this.log.debug(`Computing slash votes for ${offensesToSlash.length} offenses`, { slotNumber, currentRound, slashedRound, @@ -371,6 +371,14 @@ export class SlasherClient implements ProposerSlashActionProvider, SlasherClient return undefined; } + this.log.info(`Voting to slash ${offensesToSlash.length} offenses`, { + slotNumber, + slashedRound, + currentRound, + votes, + offensesToSlash, + }); + this.log.debug(`Computed votes for slashing ${offensesToSlash.length} offenses`, { slashedRound, currentRound, diff --git a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts index 1c377785d01b..639b73ca4153 100644 --- a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.test.ts @@ -88,6 +88,51 @@ describe('AttestedInvalidProposalWatcher', () => { ]); }); + it('emits zero-amount offenses when the penalty is zero', async () => { + const slot = SlotNumber(10); + const attesterSigner = Secp256k1Signer.random(); + invalidProposalSlots.add(slot); + watcher.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 0n }); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([await makeAttestation(slot, attesterSigner)]); + + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledWith([ + { + validator: attesterSigner.address, + amount: 0n, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: 10n, + }, + ]); + }); + + it('deduplicates repeated scans for the same attester and slot', async () => { + const slot = SlotNumber(10); + invalidProposalSlots.add(slot); + const attestation = await makeAttestation(slot); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]); + + await watcher.scanSlot(slot); + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('deduplicates repeated scans for the same offense when the penalty changes', async () => { + const slot = SlotNumber(10); + invalidProposalSlots.add(slot); + const attestation = await makeAttestation(slot); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]); + + watcher.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 0n }); + await watcher.scanSlot(slot); + watcher.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 13n }); + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledTimes(1); + }); + it('scans only marked invalid proposal slots once they are past the scan lag', async () => { watcher = new AttestedInvalidProposalWatcher( p2pClient, diff --git a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts index 400b47b863a8..1d6fbe082431 100644 --- a/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts +++ b/yarn-project/slasher/src/watchers/attested_invalid_proposal_watcher.ts @@ -2,6 +2,7 @@ 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'; @@ -17,6 +18,7 @@ const AttestedInvalidProposalWatcherConfigKeys = ['slashAttestInvalidCheckpointP const SCAN_SLOT_LAG = 1; const DEFAULT_SCAN_SLOT_LOOKBACK = 4; +const MAX_TRACKED_BAD_ATTESTATIONS = 10_000; type AttestedInvalidProposalWatcherConfig = Pick< SlasherConfig, @@ -38,6 +40,7 @@ export type InvalidProposalSlotSource = { export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => WatcherEmitter) implements Watcher { private readonly log: Logger; private readonly runningPromise: RunningPromise; + private readonly emittedOffenses = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); private readonly scanSlotLookback: number; private config: AttestedInvalidProposalWatcherConfig; private lastScannedSlot: SlotNumber | undefined; @@ -76,10 +79,6 @@ export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => W } public async scan(): Promise { - if (this.config.slashAttestInvalidCheckpointProposalPenalty <= 0n) { - return; - } - const currentSlot = (await this.l2BlockSource.getSyncedL2SlotNumber()) ?? this.epochCache.getSlotNow(); // genesis if (currentSlot <= SlotNumber(SCAN_SLOT_LAG)) { @@ -105,7 +104,6 @@ export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => W /** Scans a single invalid-proposal slot. */ public async scanSlot(slot: SlotNumber): Promise { if ( - this.config.slashAttestInvalidCheckpointProposalPenalty <= 0n || this.invalidProposalSlotSource.hasProposalEquivocation(slot) || !this.invalidProposalSlotSource.hasInvalidProposals(slot) ) { @@ -122,15 +120,21 @@ export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => W const slashArgs = attestations .map(attestation => this.getSlashArgs(slot, attestation)) - .filter((args): args is WantToSlashArgs => args !== undefined); + .filter((args): args is WantToSlashArgs => args !== undefined) + .filter(args => this.markAsNewOffense(args)); if (slashArgs.length === 0) { return; } - this.log.warn('Slashing attesters for attesting to invalid checkpoint proposal', { + this.log.warn('Detected attestations to invalid checkpoint proposal', { slot, - attesters: slashArgs.map(args => args.validator.toString()), + offenses: slashArgs.map(args => ({ + validator: args.validator.toString(), + amount: args.amount, + offenseType: args.offenseType, + epochOrSlot: args.epochOrSlot, + })), }); this.emit(WANT_TO_SLASH_EVENT, slashArgs); } @@ -156,4 +160,9 @@ export class AttestedInvalidProposalWatcher extends (EventEmitter as new () => W epochOrSlot: BigInt(slot), }; } + + private markAsNewOffense(args: WantToSlashArgs): boolean { + const key = `${args.validator.toString()}-${args.offenseType}-${args.epochOrSlot}`; + return this.emittedOffenses.addIfAbsent(key); + } } diff --git a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts index 5d8432b9d3d2..e8611f430582 100644 --- a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.test.ts @@ -195,6 +195,26 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => { expect(handler).not.toHaveBeenCalled(); }); + it('emits zero-amount offenses when the penalty is zero', async () => { + const signer = Secp256k1Signer.random(); + const slot = SlotNumber(10); + const blocks = await makeBlocks(signer, slot, 4); + const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]); + watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); + mockProposals(slot, blocks, [checkpoint]); + + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledWith([ + { + validator: signer.address, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: 10n, + }, + ]); + }); + it('does not emit duplicate offenses on repeated scans', async () => { const signer = Secp256k1Signer.random(); const slot = SlotNumber(10); @@ -208,6 +228,21 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => { expect(handler).toHaveBeenCalledTimes(1); }); + it('deduplicates repeated scans for the same offense when the penalty changes', async () => { + const signer = Secp256k1Signer.random(); + const slot = SlotNumber(10); + const blocks = await makeBlocks(signer, slot, 4); + const checkpoint = await makeCheckpointCore(signer, slot, blocks[1]); + mockProposals(slot, blocks, [checkpoint]); + + watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); + await watcher.scanSlot(slot); + watcher.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 11n }); + await watcher.scanSlot(slot); + + expect(handler).toHaveBeenCalledTimes(1); + }); + it('scans a lookback of closed slots', async () => { const signer = Secp256k1Signer.random(); const slot = SlotNumber(10); diff --git a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts index 1e9249b2cdf4..64dacf75e059 100644 --- a/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts +++ b/yarn-project/slasher/src/watchers/broadcasted_invalid_checkpoint_proposal_watcher.ts @@ -89,10 +89,6 @@ export class BroadcastedInvalidCheckpointProposalWatcher * `currentSlot` at the archiver's last synced L2 slot. */ public async scan(): Promise { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return; - } - const currentSlot = (await this.l2BlockSource.getSyncedL2SlotNumber()) ?? this.epochCache.getSlotNow(); if (currentSlot <= SlotNumber(SCAN_SLOT_LAG)) { return; @@ -111,10 +107,6 @@ export class BroadcastedInvalidCheckpointProposalWatcher /** Scans a single slot. Public for tests. */ public async scanSlot(slot: SlotNumber): Promise { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return; - } - const proposals = await this.p2pClient.getProposalsForSlot(slot); const slashArgs = this.getSlashArgsForProposals(slot, proposals).filter(args => this.markAsNewOffense(args)); if (slashArgs.length === 0) { @@ -125,6 +117,7 @@ export class BroadcastedInvalidCheckpointProposalWatcher slot, offenses: slashArgs.map(args => ({ validator: args.validator.toString(), + amount: args.amount, offenseType: args.offenseType, epochOrSlot: args.epochOrSlot, })), diff --git a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts index 8b6fe48f2fde..11c00a2d271b 100644 --- a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.test.ts @@ -86,9 +86,11 @@ describe('CheckpointEquivocationWatcher', () => { expect(handler).not.toHaveBeenCalled(); }); - it('does not emit when the penalty is zero', async () => { + it('emits a zero-amount offense when the penalty is zero', async () => { await watcher.stop(); + const proposer = EthAddress.random(); config = { ...config, slashDuplicateProposalPenalty: 0n }; + epochCache.getProposerAttesterAddressInSlot.mockResolvedValueOnce(proposer); watcher = new CheckpointEquivocationWatcher(l2BlockSource, epochCache, config); handler = jest.fn(); watcher.on(WANT_TO_SLASH_EVENT, handler); @@ -96,7 +98,14 @@ describe('CheckpointEquivocationWatcher', () => { await emitAndFlush(makeEvent()); - expect(handler).not.toHaveBeenCalled(); + expect(handler).toHaveBeenCalledWith([ + { + validator: proposer, + amount: 0n, + offenseType: OffenseType.DUPLICATE_PROPOSAL, + epochOrSlot: 10n, + }, + ]); }); it('emits separately for distinct slots', async () => { diff --git a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts index 887986a8ba7e..6e611bcc12d2 100644 --- a/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts +++ b/yarn-project/slasher/src/watchers/checkpoint_equivocation_watcher.ts @@ -66,10 +66,6 @@ export class CheckpointEquivocationWatcher extends (EventEmitter as new () => Wa /** Public for tests. */ public async onEquivocationDetected(event: CheckpointEquivocationDetectedEvent): Promise { - if (this.config.slashDuplicateProposalPenalty <= 0n) { - return; - } - const proposer = await this.epochCache.getProposerAttesterAddressInSlot(event.slotNumber); if (!proposer) { this.log.warn(`Cannot attribute checkpoint equivocation: no proposer for slot ${event.slotNumber}`, { @@ -89,6 +85,8 @@ export class CheckpointEquivocationWatcher extends (EventEmitter as new () => Wa this.log.info(`Detected checkpoint equivocation offense`, { slotNumber: event.slotNumber, checkpointNumber: event.checkpointNumber, + amount: slashArgs.amount, + offenseType: slashArgs.offenseType, l1ArchiveRoot: event.l1ArchiveRoot.toString(), proposedArchiveRoot: event.proposedArchiveRoot.toString(), validator: proposer.toString(), diff --git a/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts b/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts index 310eaf3c3b62..5db77003fa20 100644 --- a/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts +++ b/yarn-project/slasher/src/watchers/data_withholding_watcher.test.ts @@ -291,16 +291,32 @@ describe('DataWithholdingWatcher', () => { expect(l2BlockSource.getCheckpoint).toHaveBeenCalledTimes(1); }); - it('respects penalty=0 as a disable switch', async () => { + it('emits zero-amount offenses when the penalty is zero', async () => { watcher.updateConfig({ slashDataWithholdingPenalty: 0n }); await startAtSlot(10); - setSyncedSlot(10 + TOLERANCE + 5); + setSyncedSlot(11 + TOLERANCE + 1); + + const slot = 11; + const published = makePublished(slot, 1); + const missing = published.checkpoint.blocks[0].body.txEffects[0].txHash; + const attester = EthAddress.random(); + l2BlockSource.getCheckpoint.mockResolvedValue(published); + mockMissing([missing]); + watcher.attestersBySlot.set(slot, [attester]); const captured = captureEmits(); await watcher.work(); - expect(l2BlockSource.getCheckpoint).not.toHaveBeenCalled(); - expect(captured).toHaveLength(0); + expect(captured).toEqual([ + [ + { + validator: attester, + amount: 0n, + offenseType: OffenseType.DATA_WITHHOLDING, + epochOrSlot: BigInt(slot), + }, + ], + ]); }); it('does not slash a checkpoint with no recoverable attesters even if txs are missing', async () => { diff --git a/yarn-project/slasher/src/watchers/data_withholding_watcher.ts b/yarn-project/slasher/src/watchers/data_withholding_watcher.ts index 091c2be823f6..30588400309c 100644 --- a/yarn-project/slasher/src/watchers/data_withholding_watcher.ts +++ b/yarn-project/slasher/src/watchers/data_withholding_watcher.ts @@ -85,10 +85,6 @@ export class DataWithholdingWatcher extends (EventEmitter as new () => WatcherEm return; } - if (this.config.slashDataWithholdingPenalty === 0n) { - return; // disabled - } - // tolerance is the number of full slots that must elapse after the checkpoint's slot // before we declare its data missing. For checkpoint slot S, we therefore process S // only once we are in slot `S + tolerance + 1` or later. Drive this off the archiver's @@ -175,9 +171,11 @@ export class DataWithholdingWatcher extends (EventEmitter as new () => WatcherEm return; } - this.log.warn(`Detected data withholding at slot ${slot}. Slashing ${attesters.length} attesters.`, { + this.log.warn(`Detected data withholding offense at slot ${slot}`, { slot, checkpointNumber, + amount: this.config.slashDataWithholdingPenalty, + offenseType: OffenseType.DATA_WITHHOLDING, missingTxs: missingTxs.map(h => h.toString()), records: collectionRecords, attesters: attesters.map(a => a.toString()), diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index d344399a14ac..a4af734ccc5b 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -417,6 +417,15 @@ describe('ValidatorClient', () => { Array.isArray(args) && args[0]?.offenseType === OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, ); + const getAttestedToInvalidCheckpointProposalSlashEvents = ( + emitSpy: jest.SpiedFunction, + ) => + emitSpy.mock.calls.filter( + ([event, args]) => + event === WANT_TO_SLASH_EVENT && + Array.isArray(args) && + args[0]?.offenseType === OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + ); beforeEach(async () => { const emptyInHash = computeInHashFromL1ToL2Messages([]); const blockHeader = makeBlockHeader(1, { blockNumber: BlockNumber(100), slotNumber: SlotNumber(100) }); @@ -897,15 +906,24 @@ describe('ValidatorClient', () => { expect(blockSource.getBlockData).toHaveBeenCalledWith({ number: blockNumber }); }); - it('should not emit WANT_TO_SLASH_EVENT if slashing is disabled', async () => { + it('emits zero-amount invalid block proposal offenses when the penalty is zero', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidBlockPenalty: 0n }); const emitSpy = jest.spyOn(validatorClient, 'emit'); blockBuildResult.block.archive.root = Fr.random(); const isValid = await validatorClient.validateBlockProposal(proposal, sender); + const proposer = proposal.getSender(); expect(isValid).toBe(false); - expect(emitSpy).not.toHaveBeenCalled(); + expect(proposer).toBeDefined(); + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_SLASH_EVENT, [ + { + validator: proposer!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL, + epochOrSlot: expect.any(BigInt), + }, + ]); }); it('marks invalid block proposal slots for delayed attestation slashing', async () => { @@ -939,19 +957,17 @@ describe('ValidatorClient', () => { ]); }); - it('does not mark invalid proposal slots when the bad attestation penalty is disabled', async () => { + it('marks invalid proposal slots when the bad attestation penalty is zero', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidBlockPenalty: 0n, slashAttestInvalidCheckpointProposalPenalty: 0n, }); - const emitSpy = jest.spyOn(validatorClient, 'emit'); blockBuildResult.block.archive.root = Fr.random(); const isValid = await validatorClient.validateBlockProposal(proposal, sender); expect(isValid).toBe(false); - expect(validatorClient.hasInvalidProposals(proposal.slotNumber)).toBe(false); - expect(emitSpy).not.toHaveBeenCalled(); + expect(validatorClient.hasInvalidProposals(proposal.slotNumber)).toBe(true); }); it('reexecutes for bad attestation slashing when invalid block proposer slashing is disabled', async () => { @@ -965,6 +981,40 @@ describe('ValidatorClient', () => { expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalled(); }); + it('emits zero-amount bad attestation offenses when the bad attestation penalty is zero', async () => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; + validatorClient.updateConfig({ + slashBroadcastedInvalidBlockPenalty: 0n, + slashAttestInvalidCheckpointProposalPenalty: 0n, + }); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + const attesterSigner = Secp256k1Signer.random(); + const attestation = makeCheckpointAttestation({ + header: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber }), + attesterSigner, + }); + blockBuildResult.block.archive.root = Fr.random(); + + const isValid = await validatorClient.validateBlockProposal(proposal, sender); + attestationCallback(attestation); + + expect(isValid).toBe(false); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: attesterSigner.address, + amount: 0n, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(proposal.slotNumber), + }, + ], + ], + ]); + }); + it('emits WANT_TO_SLASH_EVENT for checkpoint_header_mismatch checkpoint proposals', async () => { const checkpointHandler = registerAllNodesCheckpointHandler(); const { checkpointProposal, disposeFork } = await makeCheckpointProposalWithHeaderMismatch(); @@ -1041,7 +1091,7 @@ describe('ValidatorClient', () => { ]); }); - it('does not emit checkpoint proposal slash event when the penalty is disabled', async () => { + it('emits zero-amount checkpoint proposal offenses when the penalty is zero', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); const checkpointHandler = registerAllNodesCheckpointHandler(); const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); @@ -1051,7 +1101,19 @@ describe('ValidatorClient', () => { const attestations = await validatorClient.attestToCheckpointProposal(checkpointProposal, sender); expect(attestations).toBeUndefined(); - expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: checkpointProposal.getSender()!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); }); it.each(['last_block_not_found', 'checkpoint_already_published'])( @@ -1099,7 +1161,19 @@ describe('ValidatorClient', () => { await checkpointHandler(checkpointProposal, sender); - expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: checkpointProposal.getSender()!, + amount: 0n, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); expect(validatorClient.hasInvalidProposals(checkpointProposal.slotNumber)).toBe(true); }); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 57f7253bfe6b..5baa390ae054 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -73,6 +73,7 @@ import { const MAX_PROPOSERS_OF_INVALID_BLOCKS = 1000; const MAX_TRACKED_INVALID_PROPOSAL_SLOTS = 1000; const MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS = 1000; +const MAX_TRACKED_BAD_ATTESTATIONS = 10_000; // What errors from the block proposal handler result in slashing const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ @@ -129,6 +130,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private proposersOfInvalidBlocks = FifoSet.withLimit(MAX_PROPOSERS_OF_INVALID_BLOCKS); private slotsWithInvalidProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); private invalidCheckpointProposalOffenseKeys = FifoSet.withLimit(MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS); + private badAttestationOffenseKeys = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); private slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); /** Tracks the last checkpoint proposal we attested to, to prevent equivocation. */ @@ -430,6 +432,10 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.handleDuplicateAttestation(info); }); + this.p2pClient.registerCheckpointAttestationCallback((attestation: CheckpointAttestation) => { + this.handleCheckpointAttestation(attestation); + }); + const myAddresses = this.getValidatorAddresses(); this.p2pClient.registerThisValidatorAddresses(myAddresses); @@ -476,27 +482,8 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) fishermanMode: this.config.fishermanMode || false, }); - // Reexecute txs if we are part of the committee, or if slashing is enabled, or if we are configured to always reexecute. - // In fisherman mode, we always reexecute to validate proposals. - const { - slashBroadcastedInvalidBlockPenalty, - slashAttestInvalidCheckpointProposalPenalty, - alwaysReexecuteBlockProposals, - fishermanMode, - } = this.config; - const shouldReexecute = - fishermanMode || - slashBroadcastedInvalidBlockPenalty > 0n || - slashAttestInvalidCheckpointProposalPenalty > 0n || - partOfCommittee || - alwaysReexecuteBlockProposals || - this.blobClient.canUpload(); - - const validationResult = await this.proposalHandler.handleBlockProposal( - proposal, - proposalSender, - !!shouldReexecute && !escapeHatchOpen, - ); + // Reexecute outside the escape hatch so slashing observers can detect invalid proposals even when penalties are 0. + const validationResult = await this.proposalHandler.handleBlockProposal(proposal, proposalSender, !escapeHatchOpen); if (!validationResult.isValid) { const reason = validationResult.reason || 'unknown'; @@ -524,13 +511,13 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) validationResult.reason && SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT.includes(validationResult.reason) ) { - if (slashBroadcastedInvalidBlockPenalty > 0n) { - this.log.warn(`Slashing proposer for invalid block proposal`, proposalInfo); - this.slashInvalidBlock(proposal); - } - if (slashAttestInvalidCheckpointProposalPenalty > 0n) { - this.markInvalidProposalSlot(proposal.slotNumber); - } + this.log.warn(`Detected invalid block proposal offense`, { + ...proposalInfo, + amount: this.config.slashBroadcastedInvalidBlockPenalty, + offenseType: OffenseType.BROADCASTED_INVALID_BLOCK_PROPOSAL, + }); + this.slashInvalidBlock(proposal); + this.markInvalidProposalSlot(proposal.slotNumber); } return false; } @@ -769,23 +756,19 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return; } - if (this.config.slashAttestInvalidCheckpointProposalPenalty > 0n) { - this.markInvalidProposalSlot(proposal.slotNumber); - } + this.markInvalidProposalSlot(proposal.slotNumber); if (this.slashInvalidCheckpointProposal(proposal)) { - this.log.warn(`Slashing proposer for invalid checkpoint proposal`, { + this.log.warn(`Detected invalid checkpoint proposal offense`, { ...proposalInfo, reason: result.reason, + amount: this.config.slashBroadcastedInvalidCheckpointProposalPenalty, + offenseType: OffenseType.BROADCASTED_INVALID_CHECKPOINT_PROPOSAL, }); } } private slashInvalidCheckpointProposal(proposal: CheckpointProposalCore): boolean { - if (this.config.slashBroadcastedInvalidCheckpointProposalPenalty <= 0n) { - return false; - } - const proposer = proposal.getSender(); if (!proposer) { this.log.warn(`Cannot slash checkpoint proposal with invalid signature`, { @@ -816,6 +799,47 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.slotsWithInvalidProposals.add(slotNumber); } + private handleCheckpointAttestation(attestation: CheckpointAttestation): void { + const slotNumber = attestation.slotNumber; + if (!this.slotsWithInvalidProposals.has(slotNumber) || this.slotsWithProposalEquivocation.has(slotNumber)) { + return; + } + + const attester = attestation.getSender(); + if (!attester) { + this.log.warn(`Cannot slash checkpoint attestation with invalid signature`, { + slotNumber, + archive: attestation.archive.toString(), + }); + return; + } + + this.slashAttestedToInvalidCheckpointProposal(slotNumber, attester); + } + + private slashAttestedToInvalidCheckpointProposal(slotNumber: SlotNumber, attester: EthAddress): void { + const offenseKey = `${attester.toString()}:${OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL}:${slotNumber}`; + if (!this.badAttestationOffenseKeys.addIfAbsent(offenseKey)) { + return; + } + + this.log.warn(`Detected attestation to invalid checkpoint proposal offense`, { + attester: attester.toString(), + slotNumber, + amount: this.config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + }); + + this.emit(WANT_TO_SLASH_EVENT, [ + { + validator: attester, + amount: this.config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(slotNumber), + }, + ]); + } + /** * Handle detection of a duplicate proposal (equivocation). * Emits a slash event when a proposer sends multiple proposals for the same position. @@ -824,13 +848,14 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) const { slot, proposer, type } = info; this.slotsWithProposalEquivocation.add(slot); - this.log.warn(`Triggering slash event for duplicate ${type} proposal from ${proposer.toString()} at slot ${slot}`, { + this.log.warn(`Detected duplicate ${type} proposal offense from ${proposer.toString()} at slot ${slot}`, { proposer: proposer.toString(), slot, type, + amount: this.config.slashDuplicateProposalPenalty, + offenseType: OffenseType.DUPLICATE_PROPOSAL, }); - // Emit slash event this.emit(WANT_TO_SLASH_EVENT, [ { validator: proposer, @@ -855,9 +880,11 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private handleDuplicateAttestation(info: DuplicateAttestationInfo): void { const { slot, attester } = info; - this.log.warn(`Triggering slash event for duplicate attestation from ${attester.toString()} at slot ${slot}`, { + this.log.warn(`Detected duplicate attestation offense from ${attester.toString()} at slot ${slot}`, { attester: attester.toString(), slot, + amount: this.config.slashDuplicateAttestationPenalty, + offenseType: OffenseType.DUPLICATE_ATTESTATION, }); this.emit(WANT_TO_SLASH_EVENT, [