diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index d9a218e90b2f..21583e2a23c5 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/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, diff --git a/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts b/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts index 3dc0f67ad3e5..f05b3d745706 100644 --- a/yarn-project/end-to-end/src/e2e_slashing/attested_invalid_proposal.test.ts +++ b/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, ); diff --git a/yarn-project/end-to-end/src/e2e_slashing/broadcasted_invalid_checkpoint_proposal_slash.test.ts b/yarn-project/end-to-end/src/e2e_slashing/broadcasted_invalid_checkpoint_proposal_slash.test.ts index 6a9d4fab22d1..b9b132d9347b 100644 --- a/yarn-project/end-to-end/src/e2e_slashing/broadcasted_invalid_checkpoint_proposal_slash.test.ts +++ b/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, ); diff --git a/yarn-project/slasher/README.md b/yarn-project/slasher/README.md index 44e245410813..0cb82b4bfb6b 100644 --- a/yarn-project/slasher/README.md +++ b/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. 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..d5701eb94fc4 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 @@ -10,6 +10,7 @@ import { OffenseType } from '@aztec/stdlib/slashing'; import { makeBlockHeader, makeBlockProposal, + makeCheckpointAttestation, makeCheckpointHeader, makeCheckpointProposal, } from '@aztec/stdlib/testing'; @@ -22,7 +23,7 @@ import { WANT_TO_SLASH_EVENT, type WantToSlashArgs } from '../watcher.js'; import { BroadcastedInvalidCheckpointProposalWatcher } from './broadcasted_invalid_checkpoint_proposal_watcher.js'; describe('BroadcastedInvalidCheckpointProposalWatcher', () => { - let p2pClient: MockProxy>; + let p2pClient: MockProxy>; let l2BlockSource: MockProxy>; let epochCache: MockProxy>; let config: SlasherConfig; @@ -30,7 +31,8 @@ describe('BroadcastedInvalidCheckpointProposalWatcher', () => { let handler: jest.MockedFunction<(args: WantToSlashArgs[]) => void>; beforeEach(() => { - p2pClient = mock>(); + p2pClient = mock>(); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([]); l2BlockSource = mock>(); l2BlockSource.getSyncedL2SlotNumber.mockResolvedValue(SlotNumber(12)); epochCache = mock>(); @@ -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); 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..f3e89cd35d7f 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 @@ -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; + private readonly emittedOffensesBySlot = new Map>(); 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(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(); + 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); + } + } } } diff --git a/yarn-project/stdlib/src/slashing/types.ts b/yarn-project/stdlib/src/slashing/types.ts index 32c2ff671251..08afb87d0029 100644 --- a/yarn-project/stdlib/src/slashing/types.ts +++ b/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, diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 63c0d5394359..95523353ea61 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -40,6 +40,7 @@ import { makeBlockHeader, makeBlockProposal, makeCheckpointAttestation, + makeCheckpointAttestationFromProposal, makeCheckpointHeader, makeCheckpointProposal, mockTx, @@ -416,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([]); @@ -793,7 +803,7 @@ describe('ValidatorClient', () => { expect(emitSpy).not.toHaveBeenCalled(); }); - it('slashes checkpoint attestations received after an invalid proposal slot is marked only once', async () => { + it('does not slash checkpoint attestations from invalid block proposals without checkpoint payload evidence', async () => { await validatorClient.registerHandlers(); const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; const emitSpy = jest.spyOn(validatorClient, 'emit'); @@ -801,29 +811,14 @@ describe('ValidatorClient', () => { await validatorClient.validateBlockProposal(proposal, sender); - const attesterSigner = Secp256k1Signer.random(); const attestation = makeCheckpointAttestation({ header: makeCheckpointHeader(1, { slotNumber: proposal.slotNumber }), - attesterSigner, + attesterSigner: Secp256k1Signer.random(), }); attestationCallback(attestation); attestationCallback(attestation); - const badAttestationEvents = emitSpy.mock.calls.filter( - ([event, args]) => - event === WANT_TO_SLASH_EVENT && - Array.isArray(args) && - args[0]?.offenseType === OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, - ); - expect(badAttestationEvents).toHaveLength(1); - expect(badAttestationEvents[0][1]).toEqual([ - { - validator: attesterSigner.address, - amount: config.slashAttestInvalidCheckpointProposalPenalty, - offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, - epochOrSlot: BigInt(proposal.slotNumber), - }, - ]); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); }); it('clears and suppresses bad attestation offenses when proposal equivocation is detected', async () => { @@ -914,6 +909,84 @@ describe('ValidatorClient', () => { ]); }); + it('slashes checkpoint attestations retained before a delayed invalid checkpoint proposal is processed', async () => { + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const attesterSigner = Secp256k1Signer.random(); + const attestation = makeCheckpointAttestationFromProposal(checkpointProposal, attesterSigner); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + expect(p2pClient.getCheckpointAttestationsForSlot).toHaveBeenCalledWith( + checkpointProposal.slotNumber, + checkpointProposal.getPayloadHash(), + ); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: attesterSigner.address, + amount: config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); + }); + + it('does not slash retained attestations for a different checkpoint payload in an invalid proposal slot', async () => { + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const attestation = makeCheckpointAttestation({ + header: makeCheckpointHeader(1, { slotNumber: checkpointProposal.slotNumber }), + archive: Fr.random(), + attesterSigner: Secp256k1Signer.random(), + }); + expect(attestation.getPayloadHash()).not.toEqual(checkpointProposal.getPayloadHash()); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([attestation]); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + expect(p2pClient.getCheckpointAttestationsForSlot).toHaveBeenCalledWith( + checkpointProposal.slotNumber, + checkpointProposal.getPayloadHash(), + ); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + }); + + it('slashes later checkpoint attestations after a checkpoint proposal payload is marked invalid', async () => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + const attesterSigner = Secp256k1Signer.random(); + const attestation = makeCheckpointAttestationFromProposal(checkpointProposal, attesterSigner); + attestationCallback(attestation); + + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toEqual([ + [ + WANT_TO_SLASH_EVENT, + [ + { + validator: attesterSigner.address, + amount: config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ], + ], + ]); + }); + it('emits WANT_TO_SLASH_EVENT for invalid fee asset price modifiers', async () => { const checkpointHandler = registerAllNodesCheckpointHandler(); const checkpointProposal = await makeCheckpointProposal({ @@ -970,6 +1043,39 @@ describe('ValidatorClient', () => { ]); }); + it('marks invalid fee asset price modifier payloads for bad attestation slashing', async () => { + const checkpointHandler = registerAllNodesCheckpointHandler(); + const checkpointProposal = await makeCheckpointProposal({ + archiveRoot: proposal.archive, + checkpointHeader: makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }), + lastBlock: { + blockHeader: makeBlockHeader(1, { blockNumber: BlockNumber(123), slotNumber: proposal.slotNumber }), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + txHashes: proposal.txHashes, + }, + feeAssetPriceModifier: MAX_FEE_ASSET_PRICE_MODIFIER_BPS + 1n, + }); + const attesterSigner = Secp256k1Signer.random(); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([ + makeCheckpointAttestationFromProposal(checkpointProposal, attesterSigner), + ]); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + expect(p2pClient.getCheckpointAttestationsForSlot).toHaveBeenCalledWith( + checkpointProposal.slotNumber, + checkpointProposal.getPayloadHash(), + ); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(1); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)[0][1][0]).toEqual({ + validator: attesterSigner.address, + amount: config.slashAttestInvalidCheckpointProposalPenalty, + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }); + }); + it('does not emit checkpoint proposal slash event when the penalty is disabled', async () => { validatorClient.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); const checkpointHandler = registerAllNodesCheckpointHandler(); @@ -983,9 +1089,46 @@ describe('ValidatorClient', () => { expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); }); + it('slashes bad attestations when checkpoint proposer slashing is disabled', async () => { + validatorClient.updateConfig({ slashBroadcastedInvalidCheckpointProposalPenalty: 0n }); + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const attesterSigner = Secp256k1Signer.random(); + p2pClient.getCheckpointAttestationsForSlot.mockResolvedValue([ + makeCheckpointAttestationFromProposal(checkpointProposal, attesterSigner), + ]); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + expect(p2pClient.getCheckpointAttestationsForSlot).toHaveBeenCalledWith( + checkpointProposal.slotNumber, + checkpointProposal.getPayloadHash(), + ); + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(1); + }); + + it('does not mark checkpoint proposal payloads when bad attestation slashing is disabled', async () => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; + validatorClient.updateConfig({ slashAttestInvalidCheckpointProposalPenalty: 0n }); + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + + attestationCallback(makeCheckpointAttestationFromProposal(checkpointProposal, Secp256k1Signer.random())); + + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + }); + it.each(['last_block_not_found', 'checkpoint_already_published'])( 'does not emit checkpoint proposal slash event for %s', async reason => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; const checkpointHandler = registerAllNodesCheckpointHandler(); const checkpointProposal = await makeCheckpointProposalForSlot(); jest.spyOn(validatorClient.getProposalHandler(), 'handleCheckpointProposal').mockResolvedValue({ @@ -996,7 +1139,15 @@ describe('ValidatorClient', () => { await checkpointHandler(checkpointProposal, sender); + attestationCallback( + makeCheckpointAttestation({ + header: makeCheckpointHeader(1, { slotNumber: checkpointProposal.slotNumber }), + attesterSigner: Secp256k1Signer.random(), + }), + ); + expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); }, ); @@ -1011,6 +1162,50 @@ describe('ValidatorClient', () => { expect(getBroadcastedInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(1); }); + it('clears bad attestation offenses emitted for invalid checkpoint proposals when equivocation is detected', async () => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; + const duplicateProposalCallback = p2pClient.registerDuplicateProposalCallback.mock.calls[0][0]; + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + attestationCallback(makeCheckpointAttestationFromProposal(checkpointProposal, Secp256k1Signer.random())); + duplicateProposalCallback({ + slot: checkpointProposal.slotNumber, + proposer: checkpointProposal.getSender()!, + type: 'checkpoint', + }); + + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(1); + expect(emitSpy).toHaveBeenCalledWith(WANT_TO_CLEAR_SLASH_EVENT, [ + { + offenseType: OffenseType.ATTESTED_TO_INVALID_CHECKPOINT_PROPOSAL, + epochOrSlot: BigInt(checkpointProposal.slotNumber), + }, + ]); + }); + + it('suppresses bad attestation offenses for invalid checkpoint proposal slots after equivocation is detected', async () => { + await validatorClient.registerHandlers(); + const attestationCallback = p2pClient.registerCheckpointAttestationCallback.mock.calls[0][0]; + const duplicateProposalCallback = p2pClient.registerDuplicateProposalCallback.mock.calls[0][0]; + const checkpointHandler = registerAllNodesCheckpointHandler(); + const { checkpointProposal } = await makeCheckpointProposalWithHeaderMismatch(); + const emitSpy = jest.spyOn(validatorClient, 'emit'); + + await checkpointHandler(checkpointProposal, sender); + duplicateProposalCallback({ + slot: checkpointProposal.slotNumber, + proposer: checkpointProposal.getSender()!, + type: 'checkpoint', + }); + attestationCallback(makeCheckpointAttestationFromProposal(checkpointProposal, Secp256k1Signer.random())); + + expect(getAttestedToInvalidCheckpointProposalSlashEvents(emitSpy)).toHaveLength(0); + }); + it('emits slash event even if validator is not in the current committee', async () => { epochCache.filterInCommittee.mockResolvedValue([]); const checkpointHandler = registerAllNodesCheckpointHandler(); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index c7721e1ea66f..55d17eacc504 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -1,7 +1,13 @@ import type { BlobClientInterface } from '@aztec/blob-client/client'; import { type Blob, getBlobsPerL1Block } from '@aztec/blob-lib'; import type { EpochCache } from '@aztec/epoch-cache'; -import { CheckpointNumber, EpochNumber, IndexWithinCheckpoint, SlotNumber } from '@aztec/foundation/branded-types'; +import { + CheckpointNumber, + type CheckpointProposalHash, + EpochNumber, + IndexWithinCheckpoint, + SlotNumber, +} from '@aztec/foundation/branded-types'; import { Fr } from '@aztec/foundation/curves/bn254'; import type { EthAddress } from '@aztec/foundation/eth-address'; import type { Signature } from '@aztec/foundation/eth-signature'; @@ -128,7 +134,9 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) private lastAttestedEpochByAttester: Map = new Map(); private proposersOfInvalidBlocks = FifoSet.withLimit(MAX_PROPOSERS_OF_INVALID_BLOCKS); - private slotsWithInvalidBlockProposals = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); + private invalidCheckpointProposalPayloadHashes = FifoSet.withLimit( + MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS, + ); private invalidCheckpointProposalOffenseKeys = FifoSet.withLimit(MAX_TRACKED_INVALID_CHECKPOINT_PROPOSALS); private slotsWithProposalEquivocation = FifoSet.withLimit(MAX_TRACKED_INVALID_PROPOSAL_SLOTS); private badAttestationOffenseKeys = FifoSet.withLimit(MAX_TRACKED_BAD_ATTESTATIONS); @@ -527,9 +535,6 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.log.warn(`Slashing proposer for invalid block proposal`, proposalInfo); this.slashInvalidBlock(proposal); } - if (slashAttestInvalidCheckpointProposalPenalty > 0n) { - this.markInvalidProposalSlot(proposal.slotNumber); - } } return false; } @@ -759,15 +764,23 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) ]); } - private handleInvalidCheckpointProposal( + private async handleInvalidCheckpointProposal( proposal: CheckpointProposalCore, result: CheckpointProposalValidationFailureResult, proposalInfo: LogData, - ): void { + ): Promise { if (!SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[result.reason]) { return; } + const proposalPayloadHash = proposal.getPayloadHash(); + if ( + this.config.slashAttestInvalidCheckpointProposalPenalty > 0n && + this.invalidCheckpointProposalPayloadHashes.addIfAbsent(proposalPayloadHash) + ) { + await this.processRetainedCheckpointAttestations(proposal.slotNumber, proposalPayloadHash); + } + if (this.slashInvalidCheckpointProposal(proposal)) { this.log.warn(`Slashing proposer for invalid checkpoint proposal`, { ...proposalInfo, @@ -807,15 +820,32 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return true; } - private markInvalidProposalSlot(slotNumber: SlotNumber): void { - const slotKey = this.getSlotKey(slotNumber); - this.slotsWithInvalidBlockProposals.add(slotKey); + private async processRetainedCheckpointAttestations( + slotNumber: SlotNumber, + proposalPayloadHash: CheckpointProposalHash, + ): Promise { + try { + const attestations = await this.p2pClient.getCheckpointAttestationsForSlot(slotNumber, proposalPayloadHash); + for (const attestation of attestations) { + this.handleCheckpointAttestation(attestation); + } + } catch (err) { + this.log.warn(`Failed to process retained checkpoint attestations for invalid checkpoint proposal payload`, { + slotNumber, + proposalPayloadHash, + err, + }); + } } private handleCheckpointAttestation(attestation: CheckpointAttestation): void { const slotNumber = attestation.slotNumber; const slotKey = this.getSlotKey(slotNumber); - if (!this.slotsWithInvalidBlockProposals.has(slotKey) || this.slotsWithProposalEquivocation.has(slotKey)) { + const proposalPayloadHash = attestation.getPayloadHash(); + if ( + !this.invalidCheckpointProposalPayloadHashes.has(proposalPayloadHash) || + this.slotsWithProposalEquivocation.has(slotKey) + ) { return; } @@ -824,14 +854,19 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.log.warn(`Cannot slash checkpoint attestation with invalid signature`, { slotNumber, archive: attestation.archive.toString(), + proposalPayloadHash, }); return; } - this.slashAttestedToInvalidCheckpointProposal(slotNumber, attester); + this.slashAttestedToInvalidCheckpointProposal(slotNumber, proposalPayloadHash, attester); } - private slashAttestedToInvalidCheckpointProposal(slotNumber: SlotNumber, attester: EthAddress): void { + private slashAttestedToInvalidCheckpointProposal( + slotNumber: SlotNumber, + proposalPayloadHash: CheckpointProposalHash, + attester: EthAddress, + ): void { if (this.config.slashAttestInvalidCheckpointProposalPenalty <= 0n) { return; } @@ -844,6 +879,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.log.warn(`Slashing attester for attesting to invalid checkpoint proposal`, { attester: attester.toString(), slotNumber, + proposalPayloadHash, }); this.emit(WANT_TO_SLASH_EVENT, [