diff --git a/spartan/aztec-node/templates/_pod-template.yaml b/spartan/aztec-node/templates/_pod-template.yaml index 2011f6a609a8..b6989b8543ba 100644 --- a/spartan/aztec-node/templates/_pod-template.yaml +++ b/spartan/aztec-node/templates/_pod-template.yaml @@ -248,6 +248,10 @@ spec: - name: SLASH_DUPLICATE_PROPOSAL_PENALTY value: {{ .Values.node.slash.duplicateProposalPenalty | quote }} {{- end }} + {{- if .Values.node.slash.duplicateAttestationPenalty }} + - name: SLASH_DUPLICATE_ATTESTATION_PENALTY + value: {{ .Values.node.slash.duplicateAttestationPenalty | quote }} + {{- end }} {{- if .Values.node.slash.attestDescendantOfInvalidPenalty }} - name: SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY value: {{ .Values.node.slash.attestDescendantOfInvalidPenalty | quote }} diff --git a/spartan/environments/network-defaults.yml b/spartan/environments/network-defaults.yml index 4a93c5aef734..9291bc82795c 100644 --- a/spartan/environments/network-defaults.yml +++ b/spartan/environments/network-defaults.yml @@ -139,6 +139,8 @@ slasher: &slasher SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY: 10e18 # Penalty for proposing two different block or checkpoint proposal for the same position. SLASH_DUPLICATE_PROPOSAL_PENALTY: 10e18 + # Penalty for signing attestations for different proposals at the same slot. + SLASH_DUPLICATE_ATTESTATION_PENALTY: 10e18 # Penalty for unknown offenses. SLASH_UNKNOWN_PENALTY: 10e18 # Penalty for broadcasting an invalid block. @@ -240,6 +242,7 @@ networks: SLASH_INACTIVITY_PENALTY: 10e18 SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY: 10e18 SLASH_DUPLICATE_PROPOSAL_PENALTY: 10e18 + SLASH_DUPLICATE_ATTESTATION_PENALTY: 10e18 SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY: 10e18 SLASH_UNKNOWN_PENALTY: 10e18 SLASH_INVALID_BLOCK_PENALTY: 10e18 @@ -278,6 +281,7 @@ networks: SLASH_INACTIVITY_PENALTY: 10e18 SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY: 10e18 SLASH_DUPLICATE_PROPOSAL_PENALTY: 10e18 + SLASH_DUPLICATE_ATTESTATION_PENALTY: 10e18 SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY: 10e18 SLASH_UNKNOWN_PENALTY: 10e18 SLASH_INVALID_BLOCK_PENALTY: 10e18 @@ -334,6 +338,7 @@ networks: SLASH_INACTIVITY_PENALTY: 2000e18 SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY: 2000e18 SLASH_DUPLICATE_PROPOSAL_PENALTY: 2000e18 + SLASH_DUPLICATE_ATTESTATION_PENALTY: 2000e18 SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY: 2000e18 SLASH_UNKNOWN_PENALTY: 2000e18 SLASH_INVALID_BLOCK_PENALTY: 2000e18 diff --git a/spartan/scripts/deploy_network.sh b/spartan/scripts/deploy_network.sh index cb7d3449dfb3..e6007c3fd3f4 100755 --- a/spartan/scripts/deploy_network.sh +++ b/spartan/scripts/deploy_network.sh @@ -484,6 +484,7 @@ SLASH_PRUNE_PENALTY = ${SLASH_PRUNE_PENALTY:-null} SLASH_DATA_WITHHOLDING_PENALTY = ${SLASH_DATA_WITHHOLDING_PENALTY:-null} SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY = ${SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY:-null} SLASH_DUPLICATE_PROPOSAL_PENALTY = ${SLASH_DUPLICATE_PROPOSAL_PENALTY:-null} +SLASH_DUPLICATE_ATTESTATION_PENALTY = ${SLASH_DUPLICATE_ATTESTATION_PENALTY:-null} SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY = ${SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY:-null} SLASH_UNKNOWN_PENALTY = ${SLASH_UNKNOWN_PENALTY:-null} SLASH_INVALID_BLOCK_PENALTY = ${SLASH_INVALID_BLOCK_PENALTY:-null} diff --git a/spartan/terraform/deploy-aztec-infra/main.tf b/spartan/terraform/deploy-aztec-infra/main.tf index 8f98ebd35d07..1222f613773f 100644 --- a/spartan/terraform/deploy-aztec-infra/main.tf +++ b/spartan/terraform/deploy-aztec-infra/main.tf @@ -180,6 +180,7 @@ locals { "validator.slash.dataWithholdingPenalty" = var.SLASH_DATA_WITHHOLDING_PENALTY "validator.slash.proposeInvalidAttestationsPenalty" = var.SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY "validator.slash.duplicateProposalPenalty" = var.SLASH_DUPLICATE_PROPOSAL_PENALTY + "validator.slash.duplicateAttestationPenalty" = var.SLASH_DUPLICATE_ATTESTATION_PENALTY "validator.slash.attestDescendantOfInvalidPenalty" = var.SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY "validator.slash.unknownPenalty" = var.SLASH_UNKNOWN_PENALTY "validator.slash.invalidBlockPenalty" = var.SLASH_INVALID_BLOCK_PENALTY diff --git a/spartan/terraform/deploy-aztec-infra/variables.tf b/spartan/terraform/deploy-aztec-infra/variables.tf index 3edf41f7bcbe..d5d412557818 100644 --- a/spartan/terraform/deploy-aztec-infra/variables.tf +++ b/spartan/terraform/deploy-aztec-infra/variables.tf @@ -405,6 +405,12 @@ variable "SLASH_DUPLICATE_PROPOSAL_PENALTY" { nullable = true } +variable "SLASH_DUPLICATE_ATTESTATION_PENALTY" { + description = "The slash duplicate attestation penalty" + type = string + nullable = true +} + variable "SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY" { description = "The slash attest descendant of invalid penalty" type = string diff --git a/yarn-project/end-to-end/src/e2e_p2p/duplicate_attestation_slash.test.ts b/yarn-project/end-to-end/src/e2e_p2p/duplicate_attestation_slash.test.ts new file mode 100644 index 000000000000..2f68d908d458 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_p2p/duplicate_attestation_slash.test.ts @@ -0,0 +1,249 @@ +import type { AztecNodeService } from '@aztec/aztec-node'; +import type { TestAztecNodeService } from '@aztec/aztec-node/test'; +import { EthAddress } from '@aztec/aztec.js/addresses'; +import { EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; +import { bufferToHex } from '@aztec/foundation/string'; +import { OffenseType } from '@aztec/slasher'; + +import { jest } from '@jest/globals'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { privateKeyToAccount } from 'viem/accounts'; + +import { shouldCollectMetrics } from '../fixtures/fixtures.js'; +import { ATTESTER_PRIVATE_KEYS_START_INDEX, createNode } from '../fixtures/setup_p2p_test.js'; +import { getPrivateKeyFromIndex } from '../fixtures/utils.js'; +import { P2PNetworkTest } from './p2p_network.js'; +import { awaitCommitteeExists, awaitOffenseDetected } from './shared.js'; + +const TEST_TIMEOUT = 600_000; // 10 minutes + +jest.setTimeout(TEST_TIMEOUT); + +const NUM_VALIDATORS = 4; +const BOOT_NODE_UDP_PORT = 4600; +const COMMITTEE_SIZE = NUM_VALIDATORS; +const ETHEREUM_SLOT_DURATION = 8; +const AZTEC_SLOT_DURATION = ETHEREUM_SLOT_DURATION * 3; +const BLOCK_DURATION = 4; + +const DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'duplicate-attestation-slash-')); + +/** + * Test that slashing occurs when a validator sends duplicate attestations (equivocation). + * + * The setup of the test is as follows: + * 1. Create 4 validator nodes total: + * - 2 honest validators with unique keys + * - 2 "malicious proposer" validators that share the SAME validator key but have DIFFERENT coinbase addresses + * (these will create duplicate proposals for the same slot) + * - The malicious proposer validators also have `attestToEquivocatedProposals: true` which makes them attest + * to BOTH proposals when they receive them - this is the attestation equivocation we want to detect + * 2. The two nodes with the same proposer key will both detect they are proposers for the same slot and race to propose + * 3. Since they have different coinbase addresses, their proposals will have different archives (different content) + * 4. The malicious attester nodes (with attestToEquivocatedProposals enabled) will attest to BOTH proposals + * 5. Honest validators will detect the duplicate attestations and emit a slash event + * + * NOTE: This test triggers BOTH duplicate proposal (from malicious proposers sharing a key) AND duplicate attestation + * (from the malicious proposers attesting to multiple proposals). We verify specifically that the duplicate + * attestation offense is recorded. + */ +describe('e2e_p2p_duplicate_attestation_slash', () => { + let t: P2PNetworkTest; + let nodes: AztecNodeService[]; + + // Small slashing unit so we don't kick anyone out + const slashingUnit = BigInt(1e14); + const slashingQuorum = 3; + const slashingRoundSize = 4; + const aztecEpochDuration = 2; + + beforeEach(async () => { + t = await P2PNetworkTest.create({ + testName: 'e2e_p2p_duplicate_attestation_slash', + numberOfNodes: 0, + numberOfValidators: NUM_VALIDATORS, + basePort: BOOT_NODE_UDP_PORT, + metricsPort: shouldCollectMetrics(), + initialConfig: { + listenAddress: '127.0.0.1', + aztecEpochDuration, + ethereumSlotDuration: ETHEREUM_SLOT_DURATION, + aztecSlotDuration: AZTEC_SLOT_DURATION, + aztecTargetCommitteeSize: COMMITTEE_SIZE, + aztecProofSubmissionEpochs: 1024, // effectively do not reorg + slashInactivityConsecutiveEpochThreshold: 32, // effectively do not slash for inactivity + minTxsPerBlock: 0, // always be building + mockGossipSubNetwork: true, // do not worry about p2p connectivity issues + slashingQuorum, + slashingRoundSizeInEpochs: slashingRoundSize / aztecEpochDuration, + slashAmountSmall: slashingUnit, + slashAmountMedium: slashingUnit * 2n, + slashAmountLarge: slashingUnit * 3n, + enforceTimeTable: true, + blockDurationMs: BLOCK_DURATION * 1000, + slashDuplicateProposalPenalty: slashingUnit, + slashDuplicateAttestationPenalty: slashingUnit, + slashingOffsetInRounds: 1, + }, + }); + + await t.setup(); + await t.applyBaseSetup(); + }); + + afterEach(async () => { + await t.stopNodes(nodes); + await t.teardown(); + for (let i = 0; i < NUM_VALIDATORS; i++) { + fs.rmSync(`${DATA_DIR}-${i}`, { recursive: true, force: true, maxRetries: 3 }); + } + }); + + const debugRollup = async () => { + await t.ctx.cheatCodes.rollup.debugRollup(); + }; + + it('slashes validator who sends duplicate attestations', async () => { + const { rollup } = await t.getContracts(); + + // Jump forward to an epoch in the future such that the validator set is not empty + await t.ctx.cheatCodes.rollup.advanceToEpoch(EpochNumber(4)); + await debugRollup(); + + t.logger.warn('Creating nodes'); + + // Get the attester private key that will be shared between two malicious proposer nodes + // We'll use validator index 0 for the "malicious" proposer validator key + const maliciousProposerIndex = 0; + const maliciousProposerPrivateKey = getPrivateKeyFromIndex( + ATTESTER_PRIVATE_KEYS_START_INDEX + maliciousProposerIndex, + )!; + const maliciousProposerAddress = EthAddress.fromString( + privateKeyToAccount(`0x${maliciousProposerPrivateKey.toString('hex')}`).address, + ); + + t.logger.warn(`Malicious proposer address: ${maliciousProposerAddress.toString()}`); + + // Create two nodes with the SAME validator key but DIFFERENT coinbase addresses + // This will cause them to create proposals with different content for the same slot + // Additionally, enable attestToEquivocatedProposals so they will attest to BOTH proposals + const maliciousProposerPrivateKeyHex = bufferToHex(maliciousProposerPrivateKey); + const coinbase1 = EthAddress.random(); + const coinbase2 = EthAddress.random(); + + t.logger.warn(`Creating malicious proposer node 1 with coinbase ${coinbase1.toString()}`); + const maliciousNode1 = await createNode( + { + ...t.ctx.aztecNodeConfig, + validatorPrivateKey: maliciousProposerPrivateKeyHex, + coinbase: coinbase1, + attestToEquivocatedProposals: true, // Attest to all proposals - creates duplicate attestations + broadcastEquivocatedProposals: true, // Don't abort checkpoint building on duplicate block proposals + }, + t.ctx.dateProvider!, + BOOT_NODE_UDP_PORT + 1, + t.bootstrapNodeEnr, + maliciousProposerIndex, + t.prefilledPublicData, + `${DATA_DIR}-0`, + shouldCollectMetrics(), + ); + + t.logger.warn(`Creating malicious proposer node 2 with coinbase ${coinbase2.toString()}`); + const maliciousNode2 = await createNode( + { + ...t.ctx.aztecNodeConfig, + validatorPrivateKey: maliciousProposerPrivateKeyHex, + coinbase: coinbase2, + attestToEquivocatedProposals: true, // Attest to all proposals - creates duplicate attestations + broadcastEquivocatedProposals: true, // Don't abort checkpoint building on duplicate block proposals + }, + t.ctx.dateProvider!, + BOOT_NODE_UDP_PORT + 2, + t.bootstrapNodeEnr, + maliciousProposerIndex, + t.prefilledPublicData, + `${DATA_DIR}-1`, + shouldCollectMetrics(), + ); + + // Create honest nodes with unique validator keys (indices 1 and 2) + t.logger.warn('Creating honest nodes'); + const honestNode1 = await createNode( + t.ctx.aztecNodeConfig, + t.ctx.dateProvider!, + BOOT_NODE_UDP_PORT + 3, + t.bootstrapNodeEnr, + 1, + t.prefilledPublicData, + `${DATA_DIR}-2`, + shouldCollectMetrics(), + ); + const honestNode2 = await createNode( + t.ctx.aztecNodeConfig, + t.ctx.dateProvider!, + BOOT_NODE_UDP_PORT + 4, + t.bootstrapNodeEnr, + 2, + t.prefilledPublicData, + `${DATA_DIR}-3`, + shouldCollectMetrics(), + ); + + nodes = [maliciousNode1, maliciousNode2, honestNode1, honestNode2]; + + // Wait for P2P mesh and the committee to be fully formed before proceeding + await t.waitForP2PMeshConnectivity(nodes, NUM_VALIDATORS); + await awaitCommitteeExists({ rollup, logger: t.logger }); + + // Wait for offenses to be detected + // We expect BOTH duplicate proposal AND duplicate attestation offenses + // The malicious proposer nodes create duplicate proposals (same key, different coinbase) + // The malicious proposer nodes also create duplicate attestations (attestToEquivocatedProposals enabled) + t.logger.warn('Waiting for duplicate attestation offense to be detected...'); + const offenses = await awaitOffenseDetected({ + epochDuration: t.ctx.aztecNodeConfig.aztecEpochDuration, + logger: t.logger, + nodeAdmin: honestNode1, // Use honest node to check for offenses + slashingRoundSize, + waitUntilOffenseCount: 2, // Wait for both duplicate proposal and duplicate attestation + timeoutSeconds: AZTEC_SLOT_DURATION * 16, + }); + + t.logger.warn(`Collected offenses`, { offenses }); + + // Verify we have detected the duplicate attestation offense + const duplicateAttestationOffenses = offenses.filter( + offense => offense.offenseType === OffenseType.DUPLICATE_ATTESTATION, + ); + const duplicateProposalOffenses = offenses.filter( + offense => offense.offenseType === OffenseType.DUPLICATE_PROPOSAL, + ); + + t.logger.info(`Found ${duplicateAttestationOffenses.length} duplicate attestation offenses`); + t.logger.info(`Found ${duplicateProposalOffenses.length} duplicate proposal offenses`); + + // We should have at least one duplicate attestation offense + expect(duplicateAttestationOffenses.length).toBeGreaterThan(0); + + // Verify the duplicate attestation offense is from the malicious proposer address + // (since they are the ones with attestToEquivocatedProposals enabled) + for (const offense of duplicateAttestationOffenses) { + expect(offense.offenseType).toEqual(OffenseType.DUPLICATE_ATTESTATION); + expect(offense.validator.toString()).toEqual(maliciousProposerAddress.toString()); + } + + // Verify that for each duplicate attestation offense, the attester for that slot is the malicious validator + const epochCache = (honestNode1 as TestAztecNodeService).epochCache; + for (const offense of duplicateAttestationOffenses) { + const offenseSlot = SlotNumber(Number(offense.epochOrSlot)); + const committeeInfo = await epochCache.getCommittee(offenseSlot); + t.logger.info(`Offense slot ${offenseSlot}: committee includes attester ${maliciousProposerAddress.toString()}`); + expect(committeeInfo.committee?.map(addr => addr.toString())).toContain(maliciousProposerAddress.toString()); + } + + t.logger.warn('Duplicate attestation offense correctly detected and recorded'); + }); +}); diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index fbc42a161bdb..075c1106ef27 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -221,6 +221,7 @@ export type EnvVar = | 'SLASH_INACTIVITY_CONSECUTIVE_EPOCH_THRESHOLD' | 'SLASH_INVALID_BLOCK_PENALTY' | 'SLASH_DUPLICATE_PROPOSAL_PENALTY' + | 'SLASH_DUPLICATE_ATTESTATION_PENALTY' | 'SLASH_OVERRIDE_PAYLOAD' | 'SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY' | 'SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY' diff --git a/yarn-project/p2p/src/client/interface.ts b/yarn-project/p2p/src/client/interface.ts index c0638acb650e..6489c62773f5 100644 --- a/yarn-project/p2p/src/client/interface.ts +++ b/yarn-project/p2p/src/client/interface.ts @@ -14,6 +14,7 @@ import type { ReqRespSubProtocolValidators, } from '../services/reqresp/interface.js'; import type { + DuplicateAttestationInfo, DuplicateProposalInfo, P2PBlockReceivedCallback, P2PCheckpointReceivedCallback, @@ -90,6 +91,15 @@ export type P2P = P2PApiFull & */ registerDuplicateProposalCallback(callback: (info: DuplicateProposalInfo) => void): void; + /** + * Registers a callback invoked when a duplicate attestation is detected (equivocation). + * A validator signing attestations for different proposals at the same slot. + * The callback is triggered on the first duplicate (when count goes from 1 to 2). + * + * @param callback - Function called with info about the duplicate attestation + */ + registerDuplicateAttestationCallback(callback: (info: DuplicateAttestationInfo) => void): void; + /** * Request a list of transactions from another peer by their tx hashes. * @param txHashes - Hashes of the txs to query. diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 0552af7ab4cd..3afbcf88dfe4 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -40,6 +40,7 @@ import { } from '../services/reqresp/interface.js'; import { chunkTxHashesRequest } from '../services/reqresp/protocols/tx.js'; import type { + DuplicateAttestationInfo, DuplicateProposalInfo, P2PBlockReceivedCallback, P2PCheckpointReceivedCallback, @@ -339,9 +340,17 @@ export class P2PClient public async broadcastProposal(proposal: BlockProposal): Promise { this.log.verbose(`Broadcasting proposal for slot ${proposal.slotNumber} to peers`); // Store our own proposal so we can respond to req/resp requests for it - const { totalForPosition } = await this.attestationPool.tryAddBlockProposal(proposal); - if (totalForPosition > 1) { - throw new Error(`Attempted to broadcast a duplicate block proposal for slot ${proposal.slotNumber}`); + const { count } = await this.attestationPool.tryAddBlockProposal(proposal); + if (count > 1) { + if (this.config.broadcastEquivocatedProposals) { + this.log.warn(`Broadcasting equivocated block proposal for slot ${proposal.slotNumber}`, { + slot: proposal.slotNumber, + archive: proposal.archive.toString(), + count, + }); + } else { + throw new Error(`Attempted to broadcast a duplicate block proposal for slot ${proposal.slotNumber}`); + } } return this.p2pService.propagate(proposal); } @@ -393,6 +402,10 @@ export class P2PClient this.p2pService.registerDuplicateProposalCallback(callback); } + public registerDuplicateAttestationCallback(callback: (info: DuplicateAttestationInfo) => void): void { + this.p2pService.registerDuplicateAttestationCallback(callback); + } + /** * Uses the batched Request Response protocol to request a set of transactions from the network. */ diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index e8124b6ad882..ba36ccb4b39e 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -177,6 +177,9 @@ export interface P2PConfig /** Whether to run in fisherman mode: validates all proposals and attestations but does not broadcast attestations or participate in consensus */ fishermanMode: boolean; + + /** Broadcast block proposals even when a conflicting proposal for the same slot already exists in the pool (for testing purposes only). */ + broadcastEquivocatedProposals?: boolean; } export const DEFAULT_P2P_PORT = 40400; @@ -441,6 +444,11 @@ export const p2pConfigMappings: ConfigMappingsType = { 'Whether to run in fisherman mode: validates all proposals and attestations but does not broadcast attestations or participate in consensus.', ...booleanConfigHelper(false), }, + broadcastEquivocatedProposals: { + description: + 'Broadcast block proposals even when a conflicting proposal for the same slot already exists in the pool (for testing purposes only).', + ...booleanConfigHelper(false), + }, ...p2pReqRespConfigMappings, ...batchTxRequesterConfigMappings, ...chainConfigMappings, diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.test.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.test.ts index 066e7dd6b9ff..cc7850c1e902 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.test.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.test.ts @@ -5,7 +5,7 @@ import type { AztecAsyncKVStore } from '@aztec/kv-store'; import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; import { makeBlockHeader, makeBlockProposal } from '@aztec/stdlib/testing'; -import { ATTESTATION_CAP_BUFFER, AttestationPool } from './attestation_pool.js'; +import { AttestationPool, MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER } from './attestation_pool.js'; import { describeAttestationPool } from './attestation_pool_test_suite.js'; import { mockCheckpointAttestation } from './mocks.js'; @@ -38,38 +38,144 @@ describe('Attestation Pool', () => { }); }); - describe('Checkpoint Attestation cap exceeded', () => { - it('should cap unique checkpoint attestations per (slot, proposalId) at committeeSize + buffer', async () => { + describe('Checkpoint Attestation behavior', () => { + it('should add attestations from multiple signers for the same proposal', async () => { const slotNumber = 100; const archive = Fr.random(); - // Committee size and buffer (buffer is enforced inside the pool; here we pass only committeeSize) - const committeeSize = 5; - const buffer = ATTESTATION_CAP_BUFFER; - const limit = committeeSize + buffer; - - // Create 'limit' distinct checkpoint attestations for the same (slot, proposalId) - const signers = Array.from({ length: limit }, () => Secp256k1Signer.random()); + // Create distinct checkpoint attestations for the same (slot, proposalId) from different signers + const numSigners = 10; + const signers = Array.from({ length: numSigners }, () => Secp256k1Signer.random()); const attestations = signers.map(s => mockCheckpointAttestation(s, slotNumber, archive)); // Add each attestation using tryAddCheckpointAttestation + // count is the number of attestations by this signer for this slot for (let i = 0; i < attestations.length; i++) { - const result = await attestationPool.tryAddCheckpointAttestation(attestations[i], committeeSize); + const result = await attestationPool.tryAddCheckpointAttestation(attestations[i]); expect(result.added).toBe(true); - expect(result.totalForPosition).toBe(i + 1); + expect(result.count).toBe(1); // First attestation from this signer for this slot } - // A new attestation from a new signer should not be added (cap reached) - const extra = mockCheckpointAttestation(Secp256k1Signer.random(), slotNumber, archive); - const extraResult = await attestationPool.tryAddCheckpointAttestation(extra, committeeSize); - expect(extraResult.added).toBe(false); - expect(extraResult.alreadyExists).toBe(false); - expect(extraResult.totalForPosition).toBe(limit); - // Re-adding an existing attestation should return alreadyExists - const existingResult = await attestationPool.tryAddCheckpointAttestation(attestations[0], committeeSize); + const existingResult = await attestationPool.tryAddCheckpointAttestation(attestations[0]); expect(existingResult.added).toBe(false); expect(existingResult.alreadyExists).toBe(true); + expect(existingResult.count).toBe(1); // This signer has 1 attestation for this slot + }); + }); + + describe('Duplicate attestation detection (equivocation)', () => { + it('should detect duplicate attestations from same signer for same slot but different proposals', async () => { + const slotNumber = 100; + const signer = Secp256k1Signer.random(); + + // First attestation - should succeed with count=1 + const archive1 = Fr.random(); + const attestation1 = mockCheckpointAttestation(signer, slotNumber, archive1); + const result1 = await attestationPool.tryAddCheckpointAttestation(attestation1); + expect(result1.added).toBe(true); + expect(result1.count).toBe(1); // Attestations from this signer + + // Second attestation from same signer for same slot but different proposal (equivocation!) + const archive2 = Fr.random(); + const attestation2 = mockCheckpointAttestation(signer, slotNumber, archive2); + const result2 = await attestationPool.tryAddCheckpointAttestation(attestation2); + expect(result2.added).toBe(true); + expect(result2.count).toBe(2); // This is the first duplicate - triggers slashing + + // Third attestation from same signer (if we want to track more) + const archive3 = Fr.random(); + const attestation3 = mockCheckpointAttestation(signer, slotNumber, archive3); + const result3 = await attestationPool.tryAddCheckpointAttestation(attestation3); + expect(result3.added).toBe(true); + expect(result3.count).toBe(3); // Attestations from this signer + }); + + it('should reject attestations when signer exceeds per-slot cap', async () => { + const slotNumber = 100; + const signer = Secp256k1Signer.random(); + + // Add attestations up to the per-signer-per-slot cap + for (let i = 0; i < MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER; i++) { + const archive = Fr.random(); + const attestation = mockCheckpointAttestation(signer, slotNumber, archive); + const result = await attestationPool.tryAddCheckpointAttestation(attestation); + expect(result.added).toBe(true); + expect(result.count).toBe(i + 1); // Attestations from this signer + } + + // One more attestation from the same signer should be rejected + const extraArchive = Fr.random(); + const extraAttestation = mockCheckpointAttestation(signer, slotNumber, extraArchive); + const extraResult = await attestationPool.tryAddCheckpointAttestation(extraAttestation); + expect(extraResult.added).toBe(false); + expect(extraResult.alreadyExists).toBe(false); + expect(extraResult.count).toBe(MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER); // Attestations from this signer + }); + + it('should not detect duplicates for attestations from different signers', async () => { + const slotNumber = 100; + const archive = Fr.random(); + + // First signer + const signer1 = Secp256k1Signer.random(); + const attestation1 = mockCheckpointAttestation(signer1, slotNumber, archive); + const result1 = await attestationPool.tryAddCheckpointAttestation(attestation1); + expect(result1.added).toBe(true); + expect(result1.count).toBe(1); // Attestations from this signer + + // Second signer for same slot and proposal - not a duplicate, just another attestation + const signer2 = Secp256k1Signer.random(); + const attestation2 = mockCheckpointAttestation(signer2, slotNumber, archive); + const result2 = await attestationPool.tryAddCheckpointAttestation(attestation2); + expect(result2.added).toBe(true); + expect(result2.count).toBe(1); // Different signer, so count is 1 + }); + + it('should not detect duplicates for attestations from same signer but different slots', async () => { + const signer = Secp256k1Signer.random(); + const archive = Fr.random(); + + // Attestation for slot 100 + const attestation1 = mockCheckpointAttestation(signer, 100, archive); + const result1 = await attestationPool.tryAddCheckpointAttestation(attestation1); + expect(result1.added).toBe(true); + expect(result1.count).toBe(1); // Attestations from this signer for slot 100 + + // Attestation for slot 101 - different slot, not a duplicate + const attestation2 = mockCheckpointAttestation(signer, 101, archive); + const result2 = await attestationPool.tryAddCheckpointAttestation(attestation2); + expect(result2.added).toBe(true); + expect(result2.count).toBe(1); // Different slot, so count is 1 + }); + + it('should clean up per-slot-signer index when deleting old data', async () => { + const signer = Secp256k1Signer.random(); + + // Add attestations for slot 100 (to be deleted) + const attestation1 = mockCheckpointAttestation(signer, 100, Fr.random()); + await attestationPool.tryAddCheckpointAttestation(attestation1); + const attestation2 = mockCheckpointAttestation(signer, 100, Fr.random()); + await attestationPool.tryAddCheckpointAttestation(attestation2); + + // Add attestation for slot 200 (to be kept) + const attestation3 = mockCheckpointAttestation(signer, 200, Fr.random()); + await attestationPool.tryAddCheckpointAttestation(attestation3); + + // Delete data older than slot 150 + await attestationPool.deleteOlderThan(SlotNumber(150)); + + // Now adding attestations for slot 100 should start fresh + const newAttestation = mockCheckpointAttestation(signer, 100, Fr.random()); + const result = await attestationPool.tryAddCheckpointAttestation(newAttestation); + expect(result.added).toBe(true); + expect(result.count).toBe(1); // Attestations from this signer for this slot (index was cleaned up) + + // Slot 200 should still have 1 attestation from this signer + const slotNumber200Attestation = mockCheckpointAttestation(signer, 200, Fr.random()); + const result200 = await attestationPool.tryAddCheckpointAttestation(slotNumber200Attestation); + expect(result200.added).toBe(true); + expect(result200.count).toBe(2); // Original + new from same signer }); }); }); diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts index f1389e5c1be3..7f4626a035c7 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool.ts @@ -19,13 +19,17 @@ export type TryAddResult = { added: boolean; /** Whether the exact item already existed */ alreadyExists: boolean; - /** Total items for this position - used for duplicate detection */ - totalForPosition: number; + /** Count of items for the position. Meaning varies by method: + * - tryAddBlockProposal: proposals at (slot, indexWithinCheckpoint) + * - tryAddCheckpointProposal: proposals at slot + * - tryAddCheckpointAttestation: attestations by this signer for this slot */ + count: number; }; -export const MAX_PROPOSALS_PER_SLOT = 5; -export const MAX_PROPOSALS_PER_POSITION = 3; -export const ATTESTATION_CAP_BUFFER = 10; +export const MAX_CHECKPOINT_PROPOSALS_PER_SLOT = 5; +export const MAX_BLOCK_PROPOSALS_PER_POSITION = 3; +/** Maximum attestations a single signer can make per slot before being rejected. */ +export const MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER = 3; /** Public API interface for attestation pools. Used for typing mocks and test implementations. */ export type AttestationPoolApi = Pick< @@ -69,6 +73,10 @@ export class AttestationPool { // Key: (slot << 10) | indexWithinCheckpoint, Value: archive string private blockProposalsForSlotAndIndex: AztecAsyncMultiMap; + // Checkpoint attestations indexed by (slot, signer) for tracking attestations per (slot, signer) for duplicate detection + // Key: `${Fr(slot).toString()}-${signerAddress}` string (padded for lexicographic ordering), Value: `proposalId` strings + private checkpointAttestationsPerSlotAndSigner: AztecAsyncMultiMap; + constructor( private store: AztecAsyncKVStore, telemetry: TelemetryClient = getTelemetryClient(), @@ -80,6 +88,7 @@ export class AttestationPool { // Initialize checkpoint attestations storage this.checkpointAttestations = store.openMap('checkpoint_attestations'); + this.checkpointAttestationsPerSlotAndSigner = store.openMultiMap('checkpoint_attestations_per_slot_and_signer'); // Initialize checkpoint proposal storage this.checkpointProposals = store.openMap('checkpoint_proposals'); @@ -133,6 +142,12 @@ export class AttestationPool { return { start: `${proposalKey}-`, end: `${proposalKey}-Z` }; } + /** Creates a key for the per-signer-per-slot attestation index. Uses padded slot for lexicographic ordering. */ + private getSlotSignerKey(slot: SlotNumber, signerAddress: string): string { + const slotStr = new Fr(slot).toString(); + return `${slotStr}-${signerAddress}`; + } + /** Number of bits reserved for indexWithinCheckpoint in position keys. */ private static readonly INDEX_BITS = 10; /** Maximum indexWithinCheckpoint value (2^10 - 1 = 1023). */ @@ -166,21 +181,21 @@ export class AttestationPool { // Check if already exists const alreadyExists = await this.blockProposals.hasAsync(proposalId); if (alreadyExists) { - const totalForPosition = await this.getBlockProposalCountForPosition( + const count = await this.getBlockProposalCountForPosition( blockProposal.slotNumber, blockProposal.indexWithinCheckpoint, ); - return { added: false, alreadyExists: true, totalForPosition }; + return { added: false, alreadyExists: true, count }; } // Get current count for position and check cap, do not add if exceeded - const totalForPosition = await this.getBlockProposalCountForPosition( + const count = await this.getBlockProposalCountForPosition( blockProposal.slotNumber, blockProposal.indexWithinCheckpoint, ); - if (totalForPosition >= MAX_PROPOSALS_PER_POSITION) { - return { added: false, alreadyExists: false, totalForPosition }; + if (count >= MAX_BLOCK_PROPOSALS_PER_POSITION) { + return { added: false, alreadyExists: false, count }; } // Add the proposal @@ -195,7 +210,7 @@ export class AttestationPool { }, ); - return { added: true, alreadyExists: false, totalForPosition: totalForPosition + 1 }; + return { added: true, alreadyExists: false, count: count + 1 }; }); } @@ -261,14 +276,14 @@ export class AttestationPool { // Check if already exists const alreadyExists = await this.checkpointProposals.hasAsync(proposalId); if (alreadyExists) { - const totalForPosition = await this.checkpointProposalsForSlot.getValueCountAsync(proposal.slotNumber); - return { added: false, alreadyExists: true, totalForPosition }; + const count = await this.checkpointProposalsForSlot.getValueCountAsync(proposal.slotNumber); + return { added: false, alreadyExists: true, count }; } // Get current count for slot and check cap - const totalForPosition = await this.checkpointProposalsForSlot.getValueCountAsync(proposal.slotNumber); - if (totalForPosition >= MAX_PROPOSALS_PER_SLOT) { - return { added: false, alreadyExists: false, totalForPosition }; + const count = await this.checkpointProposalsForSlot.getValueCountAsync(proposal.slotNumber); + if (count >= MAX_CHECKPOINT_PROPOSALS_PER_SLOT) { + return { added: false, alreadyExists: false, count }; } // Add the proposal if cap not exceeded @@ -279,7 +294,7 @@ export class AttestationPool { slotNumber: proposal.slotNumber, }); - return { added: true, alreadyExists: false, totalForPosition: totalForPosition + 1 }; + return { added: true, alreadyExists: false, count: count + 1 }; }); } @@ -409,6 +424,14 @@ export class AttestationPool { numberOfAttestations++; } + // Clean up per-signer-per-slot index. Keys are formatted as `${Fr(slot).toString()}-${signerAddress}`. + // Since Fr pads to fixed-width hex, Fr(oldestSlot) is lexicographically greater than any key with + // a smaller slot (even with the signer suffix), so using it as the exclusive end bound is correct. + const slotSignerEndKey = new Fr(oldestSlot).toString(); + for await (const key of this.checkpointAttestationsPerSlotAndSigner.keysAsync({ end: slotSignerEndKey })) { + await this.checkpointAttestationsPerSlotAndSigner.delete(key); + } + // Delete checkpoint proposals for slots < oldestSlot, using checkpointProposalsForSlot as index for await (const slot of this.checkpointProposalsForSlot.keysAsync({ end: oldestSlot })) { const proposalIds = await toArray(this.checkpointProposalsForSlot.getValuesAsync(slot)); @@ -445,61 +468,81 @@ export class AttestationPool { * * This method performs validation and addition in a single call: * - Checks if the attestation already exists (returns alreadyExists: true if so) - * - Checks if the (slot, proposalId) has reached the attestation cap (returns added: false if so) + * - Checks if this signer has reached the per-signer attestation cap for this slot * - Adds the attestation if validation passes * * @param attestation - The checkpoint attestation to add - * @param committeeSize - Committee size for the attestation's slot - * @returns Result indicating whether the attestation was added and existence info + * @returns Result indicating whether the attestation was added, existence info, and count of + * attestations by this signer for this slot (for equivocation detection) */ - public async tryAddCheckpointAttestation( - attestation: CheckpointAttestation, - committeeSize: number, - ): Promise { + public async tryAddCheckpointAttestation(attestation: CheckpointAttestation): Promise { const slotNumber = attestation.payload.header.slotNumber; const proposalId = attestation.archive.toString(); const sender = attestation.getSender(); if (!sender) { - return { added: false, alreadyExists: false, totalForPosition: 0 }; + return { added: false, alreadyExists: false, count: 0 }; } + const signerAddress = sender.toString(); + return await this.store.transactionAsync(async () => { - const key = this.getAttestationKey(slotNumber, proposalId, sender.toString()); + const key = this.getAttestationKey(slotNumber, proposalId, signerAddress); const alreadyExists = await this.checkpointAttestations.hasAsync(key); + // Get count of attestations by this signer for this slot (for duplicate detection) + const signerAttestationCount = await this.getSignerAttestationCountForSlot(slotNumber, signerAddress); + if (alreadyExists) { - const total = await this.getAttestationCount(slotNumber, proposalId); - return { added: false, alreadyExists: true, totalForPosition: total }; + return { + added: false, + alreadyExists: true, + count: signerAttestationCount, + }; } - const limit = committeeSize + ATTESTATION_CAP_BUFFER; - const currentCount = await this.getAttestationCount(slotNumber, proposalId); - - if (currentCount >= limit) { - return { added: false, alreadyExists: false, totalForPosition: currentCount }; + // Check if this signer has exceeded the per-signer cap for this slot + if (signerAttestationCount >= MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER) { + this.log.debug(`Rejecting attestation: signer ${signerAddress} exceeded per-slot cap for slot ${slotNumber}`, { + slotNumber, + signerAddress, + proposalId, + signerAttestationCount, + }); + return { + added: false, + alreadyExists: false, + count: signerAttestationCount, + }; } + // Add the attestation await this.checkpointAttestations.set(key, attestation.toBuffer()); - this.log.debug(`Added checkpoint attestation for slot ${slotNumber} from ${sender.toString()}`, { + // Track this attestation in the per-signer-per-slot index for duplicate detection + const slotSignerKey = this.getSlotSignerKey(slotNumber, signerAddress); + await this.checkpointAttestationsPerSlotAndSigner.set(slotSignerKey, proposalId); + + this.log.debug(`Added checkpoint attestation for slot ${slotNumber} from ${signerAddress}`, { signature: attestation.signature.toString(), slotNumber, - address: sender.toString(), + address: signerAddress, proposalId, }); - return { added: true, alreadyExists: false, totalForPosition: currentCount + 1 }; + + // Return the new count + return { + added: true, + alreadyExists: false, + count: signerAttestationCount + 1, + }; }); } - /** Gets the count of attestations for a given (slot, proposalId). */ - private async getAttestationCount(slot: SlotNumber, proposalId: string): Promise { - const range = this.getAttestationKeyRangeForProposal(slot, proposalId); - let count = 0; - for await (const _ of this.checkpointAttestations.keysAsync(range)) { - count++; - } - return count; + /** Gets the count of attestations by a specific signer for a given slot. */ + private async getSignerAttestationCountForSlot(slot: SlotNumber, signerAddress: string): Promise { + const slotSignerKey = this.getSlotSignerKey(slot, signerAddress); + return await this.checkpointAttestationsPerSlotAndSigner.getValueCountAsync(slotSignerKey); } } diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool_test_suite.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool_test_suite.ts index 31b6f38fe353..20a198da71a0 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool_test_suite.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/attestation_pool_test_suite.ts @@ -10,7 +10,11 @@ import { makeCheckpointProposal, } from '@aztec/stdlib/testing'; -import { type AttestationPool, MAX_PROPOSALS_PER_POSITION, MAX_PROPOSALS_PER_SLOT } from './attestation_pool.js'; +import { + type AttestationPool, + MAX_BLOCK_PROPOSALS_PER_POSITION, + MAX_CHECKPOINT_PROPOSALS_PER_SLOT, +} from './attestation_pool.js'; import { mockCheckpointAttestation } from './mocks.js'; const NUMBER_OF_SIGNERS_PER_TEST = 4; @@ -191,7 +195,7 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result.added).toBe(true); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); const retrievedProposal = await ap.getBlockProposal(proposalId); @@ -258,7 +262,7 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result.added).toBe(true); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); const retrievedProposal = await ap.getCheckpointProposal(proposalId); @@ -324,12 +328,12 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo it('should return added=false when exceeding capacity', async () => { const slotNumber = 420; - // Add MAX_PROPOSALS_PER_SLOT proposals - for (let i = 0; i < MAX_PROPOSALS_PER_SLOT; i++) { + // Add MAX_CHECKPOINT_PROPOSALS_PER_SLOT proposals + for (let i = 0; i < MAX_CHECKPOINT_PROPOSALS_PER_SLOT; i++) { const proposal = await mockCheckpointProposalForPool(signers[i % NUMBER_OF_SIGNERS_PER_TEST], slotNumber); const result = await ap.tryAddCheckpointProposal(proposal); expect(result.added).toBe(true); - expect(result.totalForPosition).toBe(i + 1); + expect(result.count).toBe(i + 1); } // The next proposal should not be added @@ -337,7 +341,7 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result = await ap.tryAddCheckpointProposal(extraProposal); expect(result.added).toBe(false); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(MAX_PROPOSALS_PER_SLOT); + expect(result.count).toBe(MAX_CHECKPOINT_PROPOSALS_PER_SLOT); }); }); @@ -358,13 +362,13 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo }; describe('tryAddBlockProposal duplicate detection', () => { - it('should return totalForPosition=1 when pool is empty', async () => { + it('should return count=1 when pool is empty', async () => { const proposal = await mockBlockProposalWithIndex(signers[0], 100, 0); const result = await ap.tryAddBlockProposal(proposal); expect(result.added).toBe(true); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); }); it('should return alreadyExists when same proposal exists', async () => { @@ -375,17 +379,17 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result.added).toBe(false); expect(result.alreadyExists).toBe(true); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); }); - it('should detect duplicate via totalForPosition when different proposal exists at same position', async () => { + it('should detect duplicate via count when different proposal exists at same position', async () => { const slotNumber = 100; const indexWithinCheckpoint = 2; // Add first proposal const proposal1 = await mockBlockProposalWithIndex(signers[0], slotNumber, indexWithinCheckpoint); const result1 = await ap.tryAddBlockProposal(proposal1); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); // Add a different proposal at same position - this is a duplicate (equivocation) const proposal2 = await mockBlockProposalWithIndex(signers[1], slotNumber, indexWithinCheckpoint); @@ -393,8 +397,8 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result2.added).toBe(true); expect(result2.alreadyExists).toBe(false); - // totalForPosition >= 2 indicates duplicate detection - expect(result2.totalForPosition).toBe(2); + // count >= 2 indicates duplicate detection + expect(result2.count).toBe(2); }); it('should not detect duplicate for different positions in same slot', async () => { @@ -409,8 +413,8 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result = await ap.tryAddBlockProposal(proposal2); expect(result.added).toBe(true); - // totalForPosition = 1 means no duplicate for this position - expect(result.totalForPosition).toBe(1); + // count = 1 means no duplicate for this position + expect(result.count).toBe(1); }); it('should not detect duplicate for same position in different slots', async () => { @@ -425,37 +429,37 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result = await ap.tryAddBlockProposal(proposal2); expect(result.added).toBe(true); - // totalForPosition = 1 means no duplicate for this position - expect(result.totalForPosition).toBe(1); + // count = 1 means no duplicate for this position + expect(result.count).toBe(1); }); - it('should track multiple duplicates correctly via totalForPosition', async () => { + it('should track multiple duplicates correctly via count', async () => { const slotNumber = 100; const indexWithinCheckpoint = 0; // Add multiple proposals for same position const proposal1 = await mockBlockProposalWithIndex(signers[0], slotNumber, indexWithinCheckpoint); const result1 = await ap.tryAddBlockProposal(proposal1); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); const proposal2 = await mockBlockProposalWithIndex(signers[1], slotNumber, indexWithinCheckpoint); const result2 = await ap.tryAddBlockProposal(proposal2); - expect(result2.totalForPosition).toBe(2); + expect(result2.count).toBe(2); // Add a third proposal for same position const proposal3 = await mockBlockProposalWithIndex(signers[2], slotNumber, indexWithinCheckpoint); const result3 = await ap.tryAddBlockProposal(proposal3); expect(result3.added).toBe(true); - expect(result3.totalForPosition).toBe(3); + expect(result3.count).toBe(3); }); it('should return added=false when exceeding capacity', async () => { const slotNumber = 100; const indexWithinCheckpoint = 0; - // Add MAX_PROPOSALS_PER_POSITION proposals - for (let i = 0; i < MAX_PROPOSALS_PER_POSITION; i++) { + // Add MAX_BLOCK_PROPOSALS_PER_POSITION proposals + for (let i = 0; i < MAX_BLOCK_PROPOSALS_PER_POSITION; i++) { const proposal = await mockBlockProposalWithIndex( signers[i % NUMBER_OF_SIGNERS_PER_TEST], slotNumber, @@ -463,7 +467,7 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo ); const result = await ap.tryAddBlockProposal(proposal); expect(result.added).toBe(true); - expect(result.totalForPosition).toBe(i + 1); + expect(result.count).toBe(i + 1); } // The next proposal should not be added @@ -471,7 +475,7 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result = await ap.tryAddBlockProposal(extraProposal); expect(result.added).toBe(false); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(MAX_PROPOSALS_PER_POSITION); + expect(result.count).toBe(MAX_BLOCK_PROPOSALS_PER_POSITION); }); it('should clean up block position index when deleting old data', async () => { @@ -482,18 +486,18 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const proposal1 = await mockBlockProposalWithIndex(signers[0], slotNumber, indexWithinCheckpoint); await ap.tryAddBlockProposal(proposal1); - // Verify it's tracked (adding another should show totalForPosition = 2) + // Verify it's tracked (adding another should show count = 2) const proposal2 = await mockBlockProposalWithIndex(signers[1], slotNumber, indexWithinCheckpoint); let result = await ap.tryAddBlockProposal(proposal2); - expect(result.totalForPosition).toBe(2); + expect(result.count).toBe(2); // Delete old data await ap.deleteOlderThan(SlotNumber(slotNumber + 1)); - // Verify position index is cleaned up (totalForPosition should be 1 now) + // Verify position index is cleaned up (count should be 1 now) const proposal3 = await mockBlockProposalWithIndex(signers[2], slotNumber, indexWithinCheckpoint); result = await ap.tryAddBlockProposal(proposal3); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); }); it('should correctly delete block proposals at slot boundary', async () => { @@ -514,16 +518,16 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo // Slot 99 proposals should have their index cleaned up const newProposal99 = await mockBlockProposalWithIndex(signers[0], 99, 0); const result99 = await ap.tryAddBlockProposal(newProposal99); - expect(result99.totalForPosition).toBe(1); // Index was cleaned up + expect(result99.count).toBe(1); // Index was cleaned up // Slot 100 and 101 should still be tracked const newProposal100 = await mockBlockProposalWithIndex(signers[1], 100, 0); const result100 = await ap.tryAddBlockProposal(newProposal100); - expect(result100.totalForPosition).toBe(2); // Still has the original + expect(result100.count).toBe(2); // Still has the original const newProposal101 = await mockBlockProposalWithIndex(signers[2], 101, 0); const result101 = await ap.tryAddBlockProposal(newProposal101); - expect(result101.totalForPosition).toBe(2); // Still has the original + expect(result101.count).toBe(2); // Still has the original }); it('should delete all indices for a given slot', async () => { @@ -544,15 +548,15 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo // All indices should be cleaned up const newProposal0 = await mockBlockProposalWithIndex(signers[0], slotNumber, 0); const result0 = await ap.tryAddBlockProposal(newProposal0); - expect(result0.totalForPosition).toBe(1); + expect(result0.count).toBe(1); const newProposal1 = await mockBlockProposalWithIndex(signers[1], slotNumber, 1); const result1 = await ap.tryAddBlockProposal(newProposal1); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); const newProposal2 = await mockBlockProposalWithIndex(signers[2], slotNumber, 2); const result2 = await ap.tryAddBlockProposal(newProposal2); - expect(result2.totalForPosition).toBe(1); + expect(result2.count).toBe(1); }); it('should delete block proposals from storage when deleting old data', async () => { @@ -598,13 +602,13 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo return proposal.toCore(); }; - it('should return totalForPosition=1 when pool is empty', async () => { + it('should return count=1 when pool is empty', async () => { const proposal = await mockCheckpointProposalCoreForPool(signers[0], 100); const result = await ap.tryAddCheckpointProposal(proposal); expect(result.added).toBe(true); expect(result.alreadyExists).toBe(false); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); }); it('should return alreadyExists when same proposal exists', async () => { @@ -615,16 +619,16 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result.added).toBe(false); expect(result.alreadyExists).toBe(true); - expect(result.totalForPosition).toBe(1); + expect(result.count).toBe(1); }); - it('should detect duplicate via totalForPosition when different proposal exists for same slot', async () => { + it('should detect duplicate via count when different proposal exists for same slot', async () => { const slotNumber = 100; // Add first proposal const proposal1 = await mockCheckpointProposalCoreForPool(signers[0], slotNumber); const result1 = await ap.tryAddCheckpointProposal(proposal1); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); // Add a different proposal for same slot - this is a duplicate (equivocation) const proposal2 = await mockCheckpointProposalCoreForPool(signers[1], slotNumber); @@ -632,8 +636,8 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result2.added).toBe(true); expect(result2.alreadyExists).toBe(false); - // totalForPosition >= 2 indicates duplicate detection - expect(result2.totalForPosition).toBe(2); + // count >= 2 indicates duplicate detection + expect(result2.count).toBe(2); }); it('should not detect duplicate for different slots', async () => { @@ -646,28 +650,28 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result = await ap.tryAddCheckpointProposal(proposal2); expect(result.added).toBe(true); - // totalForPosition = 1 means no duplicate for this slot - expect(result.totalForPosition).toBe(1); + // count = 1 means no duplicate for this slot + expect(result.count).toBe(1); }); - it('should track multiple duplicates correctly via totalForPosition', async () => { + it('should track multiple duplicates correctly via count', async () => { const slotNumber = 100; // Add multiple proposals for same slot const proposal1 = await mockCheckpointProposalCoreForPool(signers[0], slotNumber); const result1 = await ap.tryAddCheckpointProposal(proposal1); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); const proposal2 = await mockCheckpointProposalCoreForPool(signers[1], slotNumber); const result2 = await ap.tryAddCheckpointProposal(proposal2); - expect(result2.totalForPosition).toBe(2); + expect(result2.count).toBe(2); // Add a third proposal for same slot const proposal3 = await mockCheckpointProposalCoreForPool(signers[2], slotNumber); const result3 = await ap.tryAddCheckpointProposal(proposal3); expect(result3.added).toBe(true); - expect(result3.totalForPosition).toBe(3); + expect(result3.count).toBe(3); }); it('should not count attestations as proposals for duplicate detection', async () => { @@ -684,8 +688,8 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo expect(result.added).toBe(true); expect(result.alreadyExists).toBe(false); - // totalForPosition should be 1, NOT 2 - attestations should not count as proposals - expect(result.totalForPosition).toBe(1); + // count should be 1, NOT 2 - attestations should not count as proposals + expect(result.count).toBe(1); }); it('should not count attestations for different proposals as duplicates', async () => { @@ -703,14 +707,14 @@ export function describeAttestationPool(getAttestationPool: () => AttestationPoo const result1 = await ap.tryAddCheckpointProposal(proposal1); expect(result1.added).toBe(true); - expect(result1.totalForPosition).toBe(1); + expect(result1.count).toBe(1); // Add the second checkpoint proposal - this IS a duplicate (different archive, same slot) const proposal2 = await mockCheckpointProposalCoreForPool(signers[3], slotNumber, archive2); const result2 = await ap.tryAddCheckpointProposal(proposal2); expect(result2.added).toBe(true); - expect(result2.totalForPosition).toBe(2); + expect(result2.count).toBe(2); }); }); }); diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/index.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/index.ts index a9d9b3bf773f..25588c9616b1 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/index.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/index.ts @@ -3,7 +3,7 @@ export { type AttestationPoolApi, type TryAddResult, createTestAttestationPool, - MAX_PROPOSALS_PER_SLOT, - MAX_PROPOSALS_PER_POSITION, - ATTESTATION_CAP_BUFFER, + MAX_CHECKPOINT_PROPOSALS_PER_SLOT, + MAX_BLOCK_PROPOSALS_PER_POSITION, + MAX_ATTESTATIONS_PER_SLOT_AND_SIGNER, } from './attestation_pool.js'; diff --git a/yarn-project/p2p/src/services/dummy_service.ts b/yarn-project/p2p/src/services/dummy_service.ts index 0a6f382ac3b8..44e6c4367512 100644 --- a/yarn-project/p2p/src/services/dummy_service.ts +++ b/yarn-project/p2p/src/services/dummy_service.ts @@ -26,6 +26,7 @@ import { ReqRespStatus } from './reqresp/status.js'; import { type P2PBlockReceivedCallback, type P2PCheckpointReceivedCallback, + type P2PDuplicateAttestationCallback, type P2PDuplicateProposalCallback, type P2PService, type PeerDiscoveryService, @@ -88,6 +89,11 @@ export class DummyP2PService implements P2PService { */ public registerDuplicateProposalCallback(_callback: P2PDuplicateProposalCallback): void {} + /** + * Register a callback for when a duplicate attestation is detected + */ + public registerDuplicateAttestationCallback(_callback: P2PDuplicateAttestationCallback): void {} + /** * Sends a request to a peer. * @param _protocol - The protocol to send the request on. diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.test.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.test.ts index d5933de628af..34603f63d97e 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.test.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.test.ts @@ -28,8 +28,8 @@ import { type MockProxy, mock } from 'jest-mock-extended'; import { type P2PConfig, p2pConfigMappings } from '../../config.js'; import { AttestationPool, - MAX_PROPOSALS_PER_POSITION, - MAX_PROPOSALS_PER_SLOT, + MAX_BLOCK_PROPOSALS_PER_POSITION, + MAX_CHECKPOINT_PROPOSALS_PER_SLOT, } from '../../mem_pools/attestation_pool/attestation_pool.js'; import type { MemPools } from '../../mem_pools/interface.js'; import type { TxPool } from '../../mem_pools/tx_pool/tx_pool.js'; @@ -510,8 +510,8 @@ describe('LibP2PService', () => { const header = makeBlockHeader(1, { slotNumber: currentSlot }); const indexWithinCheckpoint = IndexWithinCheckpoint(0); - // Add MAX_PROPOSALS_PER_POSITION proposals - for (let i = 0; i < MAX_PROPOSALS_PER_POSITION; i++) { + // Add MAX_BLOCK_PROPOSALS_PER_POSITION proposals + for (let i = 0; i < MAX_BLOCK_PROPOSALS_PER_POSITION; i++) { const individualSigner = Secp256k1Signer.random(); mockEpochCache.getProposerAttesterAddressInSlot.mockResolvedValue(individualSigner.address); const proposal = await makeBlockProposal({ @@ -745,8 +745,8 @@ describe('LibP2PService', () => { const checkpointHeader = makeCheckpointHeader(1, { slotNumber: currentSlot }); const blockHeader = makeBlockHeader(1, { slotNumber: currentSlot }); - // Fill checkpoint slot to MAX_PROPOSALS_PER_SLOT - for (let i = 0; i < MAX_PROPOSALS_PER_SLOT; i++) { + // Fill checkpoint slot to MAX_CHECKPOINT_PROPOSALS_PER_SLOT + for (let i = 0; i < MAX_CHECKPOINT_PROPOSALS_PER_SLOT; i++) { const individualSigner = Secp256k1Signer.random(); mockEpochCache.getProposerAttesterAddressInSlot.mockResolvedValue(individualSigner.address); const proposal = await makeCheckpointProposal({ diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index 92b368c645a7..765c71b6d90b 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -113,6 +113,7 @@ import { ReqResp } from '../reqresp/reqresp.js'; import type { P2PBlockReceivedCallback, P2PCheckpointReceivedCallback, + P2PDuplicateAttestationCallback, P2PService, PeerDiscoveryService, } from '../service.js'; @@ -155,6 +156,9 @@ export class LibP2PService extends type: 'checkpoint' | 'block'; }) => void; + /** Callback invoked when a duplicate attestation is detected (triggers slashing). */ + private duplicateAttestationCallback?: P2PDuplicateAttestationCallback; + /** * Callback for when a block is received from a peer. * @param block - The block received from the peer. @@ -685,6 +689,15 @@ export class LibP2PService extends this.duplicateProposalCallback = callback; } + /** + * Registers a callback to be invoked when a duplicate attestation is detected. + * A validator signing attestations for different proposals at the same slot. + * This callback is triggered on the first duplicate (when count goes from 1 to 2). + */ + public registerDuplicateAttestationCallback(callback: P2PDuplicateAttestationCallback): void { + this.duplicateAttestationCallback = callback; + } + /** * Subscribes to a topic. * @param topic - The topic to subscribe to. @@ -991,40 +1004,53 @@ export class LibP2PService extends return { result: TopicValidatorResult.Ignore, obj: attestation }; } - // Get committee size for the attestation's slot - const slot = attestation.payload.header.slotNumber; - const { committee } = await this.epochCache.getCommittee(slot); - const committeeSize = committee?.length ?? 0; - // Try to add the attestation: this handles existence check, cap check, and adding in one call - const { added, alreadyExists } = await this.mempools.attestationPool.tryAddCheckpointAttestation( - attestation, - committeeSize, - ); + // count is the number of attestations by this signer for this slot (for duplicate detection) + const slot = attestation.payload.header.slotNumber; + const { added, alreadyExists, count } = + await this.mempools.attestationPool.tryAddCheckpointAttestation(attestation); this.logger.trace(`Validate propagated checkpoint attestation`, { added, alreadyExists, + count, [Attributes.SLOT_NUMBER]: slot.toString(), [Attributes.P2P_ID]: peerId.toString(), }); - // Duplicate attestation received, no need to re-broadcast + // Exact same attestation received, no need to re-broadcast if (alreadyExists) { return { result: TopicValidatorResult.Ignore, obj: attestation }; } - // Could not add (cap reached), no need to re-broadcast + // Could not add (cap reached for signer), no need to re-broadcast if (!added) { - this.logger.warn(`Dropping checkpoint attestation due to per-(slot, proposalId) attestation cap`, { + this.logger.warn(`Dropping checkpoint attestation due to cap`, { slot: slot.toString(), archive: attestation.archive.toString(), source: peerId.toString(), + attester: attestation.getSender()?.toString(), + count, }); return { result: TopicValidatorResult.Ignore, obj: attestation }; } - // Attestation was added successfully + // Check if this is a duplicate attestation (signer attested to a different proposal at the same slot) + // count is the number of attestations by this signer for this slot + if (count === 2) { + const attester = attestation.getSender(); + if (attester) { + this.logger.warn(`Detected duplicate attestation (equivocation) at slot ${slot}`, { + slot: slot.toString(), + archive: attestation.archive.toString(), + source: peerId.toString(), + attester: attester.toString(), + }); + this.duplicateAttestationCallback?.({ slot, attester }); + } + } + + // Attestation was added successfully - accept it so other nodes can also detect the equivocation return { result: TopicValidatorResult.Accept, obj: attestation }; } @@ -1070,8 +1096,8 @@ export class LibP2PService extends } // Try to add the proposal: this handles existence check, cap check, and adding in one call - const { added, alreadyExists, totalForPosition } = await this.mempools.attestationPool.tryAddBlockProposal(block); - const isEquivocated = totalForPosition !== undefined && totalForPosition > 1; + const { added, alreadyExists, count } = await this.mempools.attestationPool.tryAddBlockProposal(block); + const isEquivocated = count !== undefined && count > 1; // Duplicate proposal received, no need to re-broadcast if (alreadyExists) { @@ -1090,7 +1116,7 @@ export class LibP2PService extends this.logger.warn(`Penalizing peer for block proposal exceeding per-position cap`, { ...block.toBlockInfo(), indexWithinCheckpoint: block.indexWithinCheckpoint, - totalForPosition, + count, proposer: block.getSender()?.toString(), source: peerId.toString(), }); @@ -1107,7 +1133,7 @@ export class LibP2PService extends proposer: proposer?.toString(), }); // Invoke the duplicate callback on the first duplicate spotted only - if (proposer && totalForPosition === 2) { + if (proposer && count === 2) { this.duplicateProposalCallback?.({ slot: block.slotNumber, proposer, type: 'block' }); } return { result: TopicValidatorResult.Accept, obj: block, metadata: { isEquivocated } }; @@ -1224,8 +1250,8 @@ export class LibP2PService extends // Try to add the checkpoint proposal core: this handles existence check, cap check, and adding in one call const checkpointCore = checkpoint.toCore(); const tryAddResult = await this.mempools.attestationPool.tryAddCheckpointProposal(checkpointCore); - const { added, alreadyExists, totalForPosition } = tryAddResult; - const isEquivocated = totalForPosition !== undefined && totalForPosition > 1; + const { added, alreadyExists, count } = tryAddResult; + const isEquivocated = count !== undefined && count > 1; // Duplicate proposal received, do not re-broadcast if (alreadyExists) { @@ -1246,7 +1272,7 @@ export class LibP2PService extends this.peerManager.penalizePeer(peerId, PeerErrorSeverity.HighToleranceError); this.logger.warn(`Penalizing peer for checkpoint proposal exceeding per-slot cap`, { ...checkpoint.toCheckpointInfo(), - totalForPosition, + count, source: peerId.toString(), }); return { result: TopicValidatorResult.Reject, obj: checkpoint, metadata: { isEquivocated, processBlock } }; @@ -1262,7 +1288,7 @@ export class LibP2PService extends proposer: proposer?.toString(), }); // Invoke the duplicate callback on the first duplicate spotted only - if (proposer && totalForPosition === 2) { + if (proposer && count === 2) { this.duplicateProposalCallback?.({ slot: checkpoint.slotNumber, proposer, type: 'checkpoint' }); } return { diff --git a/yarn-project/p2p/src/services/service.ts b/yarn-project/p2p/src/services/service.ts index 14ebff2227ee..b7b3b6fa42d0 100644 --- a/yarn-project/p2p/src/services/service.ts +++ b/yarn-project/p2p/src/services/service.ts @@ -57,6 +57,19 @@ export type DuplicateProposalInfo = { */ export type P2PDuplicateProposalCallback = (info: DuplicateProposalInfo) => void; +/** Minimal info passed to the duplicate attestation callback. */ +export type DuplicateAttestationInfo = { + slot: SlotNumber; + attester: EthAddress; +}; + +/** + * Callback for when a duplicate attestation is detected (equivocation). + * A validator signing attestations for different proposals at the same slot. + * Invoked on the first duplicate (when count goes from 1 to 2). + */ +export type P2PDuplicateAttestationCallback = (info: DuplicateAttestationInfo) => void; + /** * The interface for a P2P service implementation. */ @@ -106,6 +119,13 @@ export interface P2PService { */ registerDuplicateProposalCallback(callback: P2PDuplicateProposalCallback): void; + /** + * Registers a callback invoked when a duplicate attestation is detected (equivocation). + * A validator signing attestations for different proposals at the same slot. + * The callback is triggered on the first duplicate (when count goes from 1 to 2). + */ + registerDuplicateAttestationCallback(callback: P2PDuplicateAttestationCallback): void; + getEnr(): ENR | undefined; getPeers(includePending?: boolean): PeerInfo[]; diff --git a/yarn-project/p2p/src/test-helpers/testbench-utils.ts b/yarn-project/p2p/src/test-helpers/testbench-utils.ts index 0ae3ece65537..041349a748a2 100644 --- a/yarn-project/p2p/src/test-helpers/testbench-utils.ts +++ b/yarn-project/p2p/src/test-helpers/testbench-utils.ts @@ -155,10 +155,10 @@ export class InMemoryAttestationPool { const id = blockProposal.archive.toString(); const alreadyExists = this.proposals.has(id); if (alreadyExists) { - return Promise.resolve({ added: false, alreadyExists: true, totalForPosition: 1 }); + return Promise.resolve({ added: false, alreadyExists: true, count: 1 }); } this.proposals.set(id, blockProposal); - return Promise.resolve({ added: true, alreadyExists: false, totalForPosition: 1 }); + return Promise.resolve({ added: true, alreadyExists: false, count: 1 }); } getBlockProposal(id: string): Promise { @@ -166,7 +166,7 @@ export class InMemoryAttestationPool { } tryAddCheckpointProposal(_proposal: CheckpointProposal): Promise { - return Promise.resolve({ added: true, alreadyExists: false, totalForPosition: 1 }); + return Promise.resolve({ added: true, alreadyExists: false, count: 1 }); } getCheckpointProposal(_id: string): Promise { @@ -188,8 +188,8 @@ export class InMemoryAttestationPool { return Promise.resolve([]); } - tryAddCheckpointAttestation(_attestation: CheckpointAttestation, _committeeSize: number): Promise { - return Promise.resolve({ added: true, alreadyExists: false, totalForPosition: 1 }); + tryAddCheckpointAttestation(_attestation: CheckpointAttestation): Promise { + return Promise.resolve({ added: true, alreadyExists: false, count: 1 }); } isEmpty(): Promise { diff --git a/yarn-project/slasher/src/config.ts b/yarn-project/slasher/src/config.ts index ddda18385ed4..79cef1e58b1a 100644 --- a/yarn-project/slasher/src/config.ts +++ b/yarn-project/slasher/src/config.ts @@ -24,6 +24,7 @@ export const DefaultSlasherConfig: SlasherConfig = { slashInactivityConsecutiveEpochThreshold: slasherDefaultEnv.SLASH_INACTIVITY_CONSECUTIVE_EPOCH_THRESHOLD, slashBroadcastedInvalidBlockPenalty: BigInt(slasherDefaultEnv.SLASH_INVALID_BLOCK_PENALTY), slashDuplicateProposalPenalty: BigInt(slasherDefaultEnv.SLASH_DUPLICATE_PROPOSAL_PENALTY), + slashDuplicateAttestationPenalty: BigInt(slasherDefaultEnv.SLASH_DUPLICATE_ATTESTATION_PENALTY), slashInactivityPenalty: BigInt(slasherDefaultEnv.SLASH_INACTIVITY_PENALTY), slashProposeInvalidAttestationsPenalty: BigInt(slasherDefaultEnv.SLASH_PROPOSE_INVALID_ATTESTATIONS_PENALTY), slashAttestDescendantOfInvalidPenalty: BigInt(slasherDefaultEnv.SLASH_ATTEST_DESCENDANT_OF_INVALID_PENALTY), @@ -94,6 +95,12 @@ export const slasherConfigMappings: ConfigMappingsType = { description: 'Penalty amount for slashing a validator for sending duplicate proposals.', ...bigintConfigHelper(DefaultSlasherConfig.slashDuplicateProposalPenalty), }, + slashDuplicateAttestationPenalty: { + env: 'SLASH_DUPLICATE_ATTESTATION_PENALTY', + description: + 'Penalty amount for slashing a validator for signing attestations for different proposals at the same slot.', + ...bigintConfigHelper(DefaultSlasherConfig.slashDuplicateAttestationPenalty), + }, slashInactivityTargetPercentage: { env: 'SLASH_INACTIVITY_TARGET_PERCENTAGE', description: diff --git a/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts b/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts index e0c65d40489e..01ad4f83e43a 100644 --- a/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts +++ b/yarn-project/stdlib/src/interfaces/aztec-node-admin.test.ts @@ -151,6 +151,7 @@ class MockAztecNodeAdmin implements AztecNodeAdmin { slashInactivityPenalty: 1000n, slashBroadcastedInvalidBlockPenalty: 1n, slashDuplicateProposalPenalty: 1n, + slashDuplicateAttestationPenalty: 1n, secondsBeforeInvalidatingBlockAsCommitteeMember: 0, secondsBeforeInvalidatingBlockAsNonCommitteeMember: 0, slashProposeInvalidAttestationsPenalty: 1000n, diff --git a/yarn-project/stdlib/src/interfaces/slasher.ts b/yarn-project/stdlib/src/interfaces/slasher.ts index 4e241021a475..44ebbd97f790 100644 --- a/yarn-project/stdlib/src/interfaces/slasher.ts +++ b/yarn-project/stdlib/src/interfaces/slasher.ts @@ -19,6 +19,7 @@ export interface SlasherConfig { slashInactivityPenalty: bigint; slashBroadcastedInvalidBlockPenalty: bigint; slashDuplicateProposalPenalty: bigint; + slashDuplicateAttestationPenalty: bigint; slashProposeInvalidAttestationsPenalty: bigint; slashAttestDescendantOfInvalidPenalty: bigint; slashUnknownPenalty: bigint; @@ -42,6 +43,7 @@ export const SlasherConfigSchema = zodFor()( slashInactivityPenalty: schemas.BigInt, slashProposeInvalidAttestationsPenalty: schemas.BigInt, slashDuplicateProposalPenalty: schemas.BigInt, + slashDuplicateAttestationPenalty: schemas.BigInt, slashAttestDescendantOfInvalidPenalty: schemas.BigInt, slashUnknownPenalty: schemas.BigInt, slashOffenseExpirationRounds: z.number(), diff --git a/yarn-project/stdlib/src/interfaces/validator.ts b/yarn-project/stdlib/src/interfaces/validator.ts index 916db68d0255..48cb4a1b8dd4 100644 --- a/yarn-project/stdlib/src/interfaces/validator.ts +++ b/yarn-project/stdlib/src/interfaces/validator.ts @@ -56,11 +56,17 @@ export type ValidatorClientConfig = ValidatorHASignerConfig & { /** Skip pushing re-executed blocks to archiver (default: false) */ skipPushProposedBlocksToArchiver?: boolean; + + /** Agree to attest to equivocated checkpoint proposals (for testing purposes only) */ + attestToEquivocatedProposals?: boolean; }; export type ValidatorClientFullConfig = ValidatorClientConfig & Pick & - Pick & { + Pick< + SlasherConfig, + 'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty' + > & { /** * Whether transactions are disabled for this node * @remarks This should match the property in P2PConfig. It's not picked from there to avoid circular dependencies. @@ -79,6 +85,7 @@ export const ValidatorClientConfigSchema = zodFor = { [OffenseType.PROPOSED_INCORRECT_ATTESTATIONS]: 6n, [OffenseType.ATTESTED_DESCENDANT_OF_INVALID]: 7n, [OffenseType.DUPLICATE_PROPOSAL]: 8n, + [OffenseType.DUPLICATE_ATTESTATION]: 9n, }; export function bigIntToOffense(offense: bigint): OffenseType { @@ -83,6 +88,8 @@ export function bigIntToOffense(offense: bigint): OffenseType { return OffenseType.ATTESTED_DESCENDANT_OF_INVALID; case 8n: return OffenseType.DUPLICATE_PROPOSAL; + case 9n: + return OffenseType.DUPLICATE_ATTESTATION; default: throw new Error(`Unknown offense: ${offense}`); } diff --git a/yarn-project/txe/src/state_machine/dummy_p2p_client.ts b/yarn-project/txe/src/state_machine/dummy_p2p_client.ts index b3ff0ecec717..452b5d1d8c5a 100644 --- a/yarn-project/txe/src/state_machine/dummy_p2p_client.ts +++ b/yarn-project/txe/src/state_machine/dummy_p2p_client.ts @@ -6,6 +6,7 @@ import type { P2PBlockReceivedCallback, P2PCheckpointReceivedCallback, P2PConfig, + P2PDuplicateAttestationCallback, P2PDuplicateProposalCallback, P2PSyncState, PeerId, @@ -211,4 +212,8 @@ export class DummyP2P implements P2P { public registerDuplicateProposalCallback(_callback: P2PDuplicateProposalCallback): void { throw new Error('DummyP2P does not implement "registerDuplicateProposalCallback"'); } + + public registerDuplicateAttestationCallback(_callback: P2PDuplicateAttestationCallback): void { + throw new Error('DummyP2P does not implement "registerDuplicateAttestationCallback"'); + } } diff --git a/yarn-project/validator-client/README.md b/yarn-project/validator-client/README.md index 118b313072d5..bb232bc28184 100644 --- a/yarn-project/validator-client/README.md +++ b/yarn-project/validator-client/README.md @@ -78,6 +78,7 @@ These rules must always hold: 3. **inHash is constant**: All blocks in a checkpoint share the same L1-to-L2 messages hash 4. **Sequential indexWithinCheckpoint**: Block N must have `indexWithinCheckpoint = parent.indexWithinCheckpoint + 1` 5. **One proposer per slot**: Each slot has exactly one designated proposer. Sending multiple proposals for the same position (slot, indexWithinCheckpoint) with different content is equivocation and slashable +6. **One attestation per slot**: Validators should only attest to one checkpoint per slot. Attesting to different proposals (different archives) for the same slot is equivocation and slashable ## Validation Flow @@ -155,16 +156,17 @@ Time | Proposer | Validator ## Configuration -| Flag | Purpose | -| ------------------------------------- | --------------------------------------------------------------------- | -| `validatorReexecute` | Re-execute transactions to verify proposals | -| `fishermanMode` | Validate proposals but don't broadcast attestations (monitoring only) | -| `alwaysReexecuteBlockProposals` | Force re-execution even when not in committee | -| `slashBroadcastedInvalidBlockPenalty` | Penalty amount for invalid proposals (0 = disabled) | -| `slashDuplicateProposalPenalty` | Penalty amount for duplicate proposals (0 = disabled) | -| `validatorReexecuteDeadlineMs` | Time reserved at end of slot for propagation/publishing | -| `attestationPollingIntervalMs` | How often to poll for attestations when collecting | -| `disabledValidators` | Validator addresses to exclude from duties | +| Flag | Purpose | +| ------------------------------------- | -------------------------------------------------------------------------------------- | +| `validatorReexecute` | Re-execute transactions to verify proposals | +| `fishermanMode` | Validate proposals but don't broadcast attestations (monitoring only) | +| `alwaysReexecuteBlockProposals` | Force re-execution even when not in committee | +| `slashBroadcastedInvalidBlockPenalty` | Penalty amount for invalid proposals (0 = disabled) | +| `slashDuplicateProposalPenalty` | Penalty amount for duplicate proposals (0 = disabled) | +| `slashDuplicateAttestationPenalty` | Penalty amount for duplicate attestations (0 = disabled) | +| `validatorReexecuteDeadlineMs` | Time reserved at end of slot for propagation/publishing | +| `attestationPollingIntervalMs` | How often to poll for attestations when collecting | +| `disabledValidators` | Validator addresses to exclude from duties | ### High Availability (HA) Keystore diff --git a/yarn-project/validator-client/src/config.ts b/yarn-project/validator-client/src/config.ts index 9a002c3843c1..6b367c214248 100644 --- a/yarn-project/validator-client/src/config.ts +++ b/yarn-project/validator-client/src/config.ts @@ -73,6 +73,10 @@ export const validatorClientConfigMappings: ConfigMappingsType { // Create 5 HA validator instances for use across all tests const baseConfig: ValidatorClientConfig & - Pick = { + Pick< + SlasherConfig, + 'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty' + > = { validatorPrivateKeys: new SecretValue(validatorPrivateKeys), attestationPollingIntervalMs: 1000, disableValidator: false, @@ -129,6 +132,7 @@ describe('ValidatorClient HA Integration', () => { slashBroadcastedInvalidBlockPenalty: 1n, l1Contracts: { rollupAddress }, slashDuplicateProposalPenalty: 1n, + slashDuplicateAttestationPenalty: 1n, haSigningEnabled: true, nodeId: 'ha-node-1', // temporary pollingIntervalMs: 100, @@ -168,7 +172,10 @@ describe('ValidatorClient HA Integration', () => { async function createHAValidator( pool: Pool, config: ValidatorClientConfig & - Pick, + Pick< + SlasherConfig, + 'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty' + >, ): Promise { // Track pool for cleanup pools.push(pool); diff --git a/yarn-project/validator-client/src/validator.integration.test.ts b/yarn-project/validator-client/src/validator.integration.test.ts index cf2e6a04844a..37f6252475eb 100644 --- a/yarn-project/validator-client/src/validator.integration.test.ts +++ b/yarn-project/validator-client/src/validator.integration.test.ts @@ -169,6 +169,7 @@ describe('ValidatorClient Integration', () => { validatorReexecute: true, slashBroadcastedInvalidBlockPenalty: 10n, slashDuplicateProposalPenalty: 10n, + slashDuplicateAttestationPenalty: 10n, haSigningEnabled: false, skipCheckpointProposalValidation: false, skipPushProposedBlocksToArchiver: false, diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 62fa685ed8a8..ac288b1a58a1 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -55,7 +55,10 @@ import { ValidatorClient } from './validator.js'; describe('ValidatorClient', () => { let config: ValidatorClientConfig & - Pick & { + Pick< + SlasherConfig, + 'slashBroadcastedInvalidBlockPenalty' | 'slashDuplicateProposalPenalty' | 'slashDuplicateAttestationPenalty' + > & { disableTransactions: boolean; }; let validatorClient: ValidatorClient; @@ -118,6 +121,7 @@ describe('ValidatorClient', () => { validatorReexecute: false, slashBroadcastedInvalidBlockPenalty: 1n, slashDuplicateProposalPenalty: 1n, + slashDuplicateAttestationPenalty: 1n, disableTransactions: false, haSigningEnabled: false, l1Contracts: { rollupAddress: EthAddress.random() }, diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 65fd888baba4..5cedbd0370e1 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -18,7 +18,7 @@ import { RunningPromise } from '@aztec/foundation/running-promise'; import { sleep } from '@aztec/foundation/sleep'; import { DateProvider } from '@aztec/foundation/timer'; import type { KeystoreManager } from '@aztec/node-keystore'; -import type { DuplicateProposalInfo, P2P, PeerId } from '@aztec/p2p'; +import type { DuplicateAttestationInfo, DuplicateProposalInfo, P2P, PeerId } from '@aztec/p2p'; import { AuthRequest, AuthResponse, BlockProposalValidator, ReqRespSubProtocol } from '@aztec/p2p'; import { OffenseType, WANT_TO_SLASH_EVENT, type Watcher, type WatcherEmitter } from '@aztec/slasher'; import type { AztecAddress } from '@aztec/stdlib/aztec-address'; @@ -32,14 +32,14 @@ import type { WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; import { type L1ToL2MessageSource, accumulateCheckpointOutHashes } from '@aztec/stdlib/messaging'; -import type { - BlockProposal, - BlockProposalOptions, - CheckpointAttestation, - CheckpointProposalCore, - CheckpointProposalOptions, +import { + type BlockProposal, + type BlockProposalOptions, + type CheckpointAttestation, + CheckpointProposal, + type CheckpointProposalCore, + type CheckpointProposalOptions, } from '@aztec/stdlib/p2p'; -import { CheckpointProposal } from '@aztec/stdlib/p2p'; import type { CheckpointHeader } from '@aztec/stdlib/rollup'; import type { BlockHeader, CheckpointGlobalVariables, Tx } from '@aztec/stdlib/tx'; import { AttestationTimeoutError } from '@aztec/stdlib/validators'; @@ -80,14 +80,20 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) // Whether it has already registered handlers on the p2p client private hasRegisteredHandlers = false; - // Used to check if we are sending the same proposal twice - private previousProposal?: BlockProposal; + /** Tracks the last block proposal we created, to detect duplicate proposal attempts. */ + private lastProposedBlock?: BlockProposal; + + /** Tracks the last checkpoint proposal we created. */ + private lastProposedCheckpoint?: CheckpointProposal; private lastEpochForCommitteeUpdateLoop: EpochNumber | undefined; private epochCacheUpdateLoop: RunningPromise; private proposersOfInvalidBlocks: Set = new Set(); + /** Tracks the last checkpoint proposal we attested to, to prevent equivocation. */ + private lastAttestedProposal?: CheckpointProposalCore; + protected constructor( private keyStore: ExtendedValidatorKeyStore, private epochCache: EpochCache, @@ -314,6 +320,11 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.handleDuplicateProposal(info); }); + // Duplicate attestation handler - triggers slashing for attestation equivocation + this.p2pClient.registerDuplicateAttestationCallback((info: DuplicateAttestationInfo) => { + this.handleDuplicateAttestation(info); + }); + const myAddresses = this.getValidatorAddresses(); this.p2pClient.registerThisValidatorAddresses(myAddresses); @@ -515,14 +526,44 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return undefined; } - return this.createCheckpointAttestationsFromProposal(proposal, attestors); + return await this.createCheckpointAttestationsFromProposal(proposal, attestors); + } + + /** + * Checks if we should attest to a slot based on equivocation prevention rules. + * @returns true if we should attest, false if we should skip + */ + private shouldAttestToSlot(slotNumber: SlotNumber): boolean { + // If attestToEquivocatedProposals is true, always allow + if (this.config.attestToEquivocatedProposals) { + return true; + } + + // Check if incoming slot is strictly greater than last attested + if (this.lastAttestedProposal && slotNumber <= this.lastAttestedProposal.slotNumber) { + this.log.warn( + `Refusing to process a proposal for slot ${slotNumber} given we already attested to a proposal for slot ${this.lastAttestedProposal.slotNumber}`, + ); + return false; + } + + return true; } private async createCheckpointAttestationsFromProposal( proposal: CheckpointProposalCore, attestors: EthAddress[] = [], - ): Promise { + ): Promise { + // Equivocation check: must happen right before signing to minimize the race window + if (!this.shouldAttestToSlot(proposal.slotNumber)) { + return undefined; + } + const attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors); + + // Track the proposal we attested to (to prevent equivocation) + this.lastAttestedProposal = proposal; + await this.p2pClient.addOwnCheckpointAttestations(attestations); return attestations; } @@ -750,6 +791,28 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) ]); } + /** + * Handle detection of a duplicate attestation (equivocation). + * Emits a slash event when an attester signs attestations for different proposals at the same slot. + */ + private handleDuplicateAttestation(info: DuplicateAttestationInfo): void { + const { slot, attester } = info; + + this.log.warn(`Triggering slash event for duplicate attestation from ${attester.toString()} at slot ${slot}`, { + attester: attester.toString(), + slot, + }); + + this.emit(WANT_TO_SLASH_EVENT, [ + { + validator: attester, + amount: this.config.slashDuplicateAttestationPenalty, + offenseType: OffenseType.DUPLICATE_ATTESTATION, + epochOrSlot: BigInt(slot), + }, + ]); + } + async createBlockProposal( blockHeader: BlockHeader, indexWithinCheckpoint: IndexWithinCheckpoint, @@ -759,11 +822,19 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) proposerAddress: EthAddress | undefined, options: BlockProposalOptions = {}, ): Promise { - // TODO(palla/mbps): Prevent double proposals properly - // if (this.previousProposal?.slotNumber === blockHeader.globalVariables.slotNumber) { - // this.log.verbose(`Already made a proposal for the same slot, skipping proposal`); - // return Promise.resolve(undefined); - // } + // Validate that we're not creating a proposal for an older or equal position + if (this.lastProposedBlock) { + const lastSlot = this.lastProposedBlock.slotNumber; + const lastIndex = this.lastProposedBlock.indexWithinCheckpoint; + const newSlot = blockHeader.globalVariables.slotNumber; + + if (newSlot < lastSlot || (newSlot === lastSlot && indexWithinCheckpoint <= lastIndex)) { + throw new Error( + `Cannot create block proposal for slot ${newSlot} index ${indexWithinCheckpoint}: ` + + `already proposed block for slot ${lastSlot} index ${lastIndex}`, + ); + } + } this.log.info( `Assembling block proposal for block ${blockHeader.globalVariables.blockNumber} slot ${blockHeader.globalVariables.slotNumber}`, @@ -780,7 +851,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) broadcastInvalidBlockProposal: this.config.broadcastInvalidBlockProposal, }, ); - this.previousProposal = newProposal; + this.lastProposedBlock = newProposal; return newProposal; } @@ -791,14 +862,29 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) proposerAddress: EthAddress | undefined, options: CheckpointProposalOptions = {}, ): Promise { + // Validate that we're not creating a proposal for an older or equal slot + if (this.lastProposedCheckpoint) { + const lastSlot = this.lastProposedCheckpoint.slotNumber; + const newSlot = checkpointHeader.slotNumber; + + if (newSlot <= lastSlot) { + throw new Error( + `Cannot create checkpoint proposal for slot ${newSlot}: ` + + `already proposed checkpoint for slot ${lastSlot}`, + ); + } + } + this.log.info(`Assembling checkpoint proposal for slot ${checkpointHeader.slotNumber}`); - return await this.validationService.createCheckpointProposal( + const newProposal = await this.validationService.createCheckpointProposal( checkpointHeader, archive, lastBlockInfo, proposerAddress, options, ); + this.lastProposedCheckpoint = newProposal; + return newProposal; } async broadcastBlockProposal(proposal: BlockProposal): Promise { @@ -820,6 +906,10 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.log.debug(`Collecting ${inCommittee.length} self-attestations for slot ${slot}`, { inCommittee }); const attestations = await this.createCheckpointAttestationsFromProposal(proposal, inCommittee); + if (!attestations) { + return []; + } + // We broadcast our own attestations to our peers so, in case our block does not get mined on L1, // other nodes can see that our validators did attest to this block proposal, and do not slash us // due to inactivity for missed attestations.