From e2d78293d95cec0bc80216058df3712268d9af6f Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Sat, 30 Nov 2024 15:32:12 +0000 Subject: [PATCH 1/4] feat: delete attestations older than a slot --- yarn-project/foundation/src/config/env_var.ts | 1 + .../p2p/src/client/p2p_client.test.ts | 1 + yarn-project/p2p/src/client/p2p_client.ts | 16 +++++++++- yarn-project/p2p/src/config.ts | 9 ++++++ .../attestation_pool/attestation_pool.ts | 10 ++++++ .../memory_attestation_pool.test.ts | 31 +++++++++++++++++++ .../memory_attestation_pool.ts | 21 +++++++++++++ .../reqresp/reqresp.integration.test.ts | 1 + 8 files changed, 89 insertions(+), 1 deletion(-) diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index ce7c17fb3ef..9dc10da7611 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -91,6 +91,7 @@ export type EnvVar = | 'P2P_TCP_LISTEN_ADDR' | 'P2P_TCP_ANNOUNCE_ADDR' | 'P2P_TX_POOL_KEEP_PROVEN_FOR' + | 'P2P_ATTESTATION_POOL_KEEP_FOR' | 'P2P_TX_PROTOCOL' | 'P2P_UDP_ANNOUNCE_ADDR' | 'P2P_UDP_LISTEN_ADDR' diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index b81929903a2..b277a21509a 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -58,6 +58,7 @@ describe('In-Memory P2P Client', () => { addAttestations: jest.fn(), deleteAttestations: jest.fn(), deleteAttestationsForSlot: jest.fn(), + deleteAttestationsOlderThan: jest.fn(), getAttestationsForSlot: jest.fn().mockReturnValue(undefined), }; diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index dfc74254bd9..9fcb8dd1f69 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -202,6 +202,9 @@ export class P2PClient extends WithTracer implements P2P { private attestationPool: AttestationPool; private epochProofQuotePool: EpochProofQuotePool; + /** How many slots to keep attestations for. */ + private keepAttestationsInPoolFor: number; + private blockStream; /** @@ -224,7 +227,10 @@ export class P2PClient extends WithTracer implements P2P { ) { super(telemetry, 'P2PClient'); - const { blockCheckIntervalMS, blockRequestBatchSize } = getP2PConfigFromEnv(); + const { blockCheckIntervalMS, blockRequestBatchSize, keepAttestationsInPoolFor } = + getP2PConfigFromEnv(); + + this.keepAttestationsInPoolFor = keepAttestationsInPoolFor; this.blockStream = new L2BlockStream(l2BlockSource, this, this, { batchSize: blockRequestBatchSize, @@ -615,6 +621,7 @@ export class P2PClient extends WithTracer implements P2P { const firstBlockNum = blocks[0].number; const lastBlockNum = blocks[blocks.length - 1].number; + const lastBlockSlot = blocks[blocks.length - 1].header.globalVariables.slotNumber.toBigInt(); if (this.keepProvenTxsFor === 0) { await this.deleteTxsFromBlocks(blocks); @@ -626,12 +633,19 @@ export class P2PClient extends WithTracer implements P2P { await this.deleteTxsFromBlocks(blocksToDeleteTxsFrom); } + // We delete attestations older than the last block slot minus the number of slots we want to keep in the pool. + if (lastBlockSlot - BigInt(this.keepAttestationsInPoolFor) >= BigInt(INITIAL_L2_BLOCK_NUM)) { + await this.attestationPool.deleteAttestationsOlderThan(lastBlockSlot - BigInt(this.keepAttestationsInPoolFor)); + } + await this.synchedProvenBlockNumber.set(lastBlockNum); this.log.debug(`Synched to proven block ${lastBlockNum}`); const provenEpochNumber = await this.l2BlockSource.getProvenL2EpochNumber(); if (provenEpochNumber !== undefined) { this.epochProofQuotePool.deleteQuotesToEpoch(BigInt(provenEpochNumber)); } + + await this.startServiceIfSynched(); } diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index 7cff1711b48..b16f0f46543 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -91,6 +91,10 @@ export interface P2PConfig extends P2PReqRespConfig { /** How many blocks have to pass after a block is proven before its txs are deleted (zero to delete immediately once proven) */ keepProvenTxsInPoolFor: number; + /** How many slots to keep attestations for. */ + keepAttestationsInPoolFor: number; + + /** * The interval of the gossipsub heartbeat to perform maintenance tasks. */ @@ -229,6 +233,11 @@ export const p2pConfigMappings: ConfigMappingsType = { 'How many blocks have to pass after a block is proven before its txs are deleted (zero to delete immediately once proven)', ...numberConfigHelper(0), }, + keepAttestationsInPoolFor: { + env: 'P2P_ATTESTATION_POOL_KEEP_FOR', + description: 'How many slots to keep attestations for.', + ...numberConfigHelper(96), + }, gossipsubInterval: { env: 'P2P_GOSSIPSUB_INTERVAL_MS', description: 'The interval of the gossipsub heartbeat to perform maintenance tasks.', 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 cdfe8729911..5c57e85b87b 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 @@ -21,6 +21,16 @@ export interface AttestationPool { */ deleteAttestations(attestations: BlockAttestation[]): Promise; + /** + * Delete Attestations with a slot number smaller than the given slot + * + * Removes all attestations associated with a slot + * + * @param slot - The oldest slot to keep. + */ + deleteAttestationsOlderThan(slot: bigint): Promise; + + /** * Delete Attestations for slot * diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts index b8bb71f30ce..0840f5e7d94 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts @@ -5,6 +5,7 @@ import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { type MockProxy, mock } from 'jest-mock-extended'; +import { jest } from '@jest/globals'; import { type PoolInstrumentation } from '../instrumentation.js'; import { InMemoryAttestationPool } from './memory_attestation_pool.js'; import { mockAttestation } from './mocks.js'; @@ -30,6 +31,11 @@ describe('MemoryAttestationPool', () => { (ap as any).metrics = metricsMock; }); + const createAttestationsForSlot = (slotNumber: number) => { + const archive = Fr.random(); + return signers.map(signer => mockAttestation(signer, slotNumber, archive)); + }; + it('should add attestations to pool', async () => { const slotNumber = 420; const archive = Fr.random(); @@ -171,4 +177,29 @@ describe('MemoryAttestationPool', () => { const retreivedAttestationsAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestationsAfterDelete.length).toBe(0); }); + + it('Should delete attestations older than a given slot', async () => { + const slotNumbers = [1, 2, 3, 69, 72, 74, 88, 420]; + const attestations = slotNumbers.map(slotNumber => createAttestationsForSlot(slotNumber)).flat(); + const proposalId = attestations[0].archive.toString(); + + await ap.addAttestations(attestations); + + const attestationsForSlot1 = await ap.getAttestationsForSlot(BigInt(1), proposalId); + expect(attestationsForSlot1.length).toBe(signers.length); + + const deleteAttestationsSpy = jest.spyOn(ap, 'deleteAttestationsForSlot'); + + await ap.deleteAttestationsOlderThan(BigInt(73)); + + const attestationsForSlot1AfterDelete = await ap.getAttestationsForSlot(BigInt(1), proposalId); + expect(attestationsForSlot1AfterDelete.length).toBe(0); + + expect(deleteAttestationsSpy).toHaveBeenCalledTimes(5); + expect(deleteAttestationsSpy).toHaveBeenCalledWith(BigInt(1)); + expect(deleteAttestationsSpy).toHaveBeenCalledWith(BigInt(2)); + expect(deleteAttestationsSpy).toHaveBeenCalledWith(BigInt(3)); + expect(deleteAttestationsSpy).toHaveBeenCalledWith(BigInt(69)); + expect(deleteAttestationsSpy).toHaveBeenCalledWith(BigInt(72)); + }); }); diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts index 9364130c4f8..fa738340c9a 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts @@ -58,6 +58,27 @@ export class InMemoryAttestationPool implements AttestationPool { return total; } + public deleteAttestationsOlderThan(oldestSlot: bigint): Promise { + const olderThan = []; + + // Entries are iterated in insertion order, so we can break as soon as we find a slot that is older than the oldestSlot. + // Note: this will only prune correctly if attestations are added in order of rising slot, it is important that we do not allow + // insertion of attestations that are old. #(https://github.com/AztecProtocol/aztec-packages/issues/10322) + const slots = this.attestations.keys(); + for (const slot of slots) { + if (slot < oldestSlot) { + olderThan.push(slot); + } else { + break; + } + } + + for (const oldSlot of olderThan) { + this.deleteAttestationsForSlot(oldSlot); + } + return Promise.resolve(); + } + public deleteAttestationsForSlot(slot: bigint): Promise { // We count the number of attestations we are removing const numberOfAttestations = this.#getNumberOfAttestationsInSlot(slot); diff --git a/yarn-project/p2p/src/service/reqresp/reqresp.integration.test.ts b/yarn-project/p2p/src/service/reqresp/reqresp.integration.test.ts index 3e28c031a0d..57445e9d396 100644 --- a/yarn-project/p2p/src/service/reqresp/reqresp.integration.test.ts +++ b/yarn-project/p2p/src/service/reqresp/reqresp.integration.test.ts @@ -64,6 +64,7 @@ const makeMockPools = () => { addAttestations: jest.fn(), deleteAttestations: jest.fn(), deleteAttestationsForSlot: jest.fn(), + deleteAttestationsOlderThan: jest.fn(), getAttestationsForSlot: jest.fn().mockReturnValue(undefined), }, epochProofQuotePool: { From 67ea59c5c475fdd0b0dc4be894dac000a95b13e4 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Sat, 30 Nov 2024 15:58:39 +0000 Subject: [PATCH 2/4] fmt --- yarn-project/p2p/src/client/p2p_client.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 9fcb8dd1f69..8e6e86f1761 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -623,6 +623,7 @@ export class P2PClient extends WithTracer implements P2P { const lastBlockNum = blocks[blocks.length - 1].number; const lastBlockSlot = blocks[blocks.length - 1].header.globalVariables.slotNumber.toBigInt(); + // If keepProvenTxsFor is 0, we delete all txs from all proven blocks. if (this.keepProvenTxsFor === 0) { await this.deleteTxsFromBlocks(blocks); } else if (lastBlockNum - this.keepProvenTxsFor >= INITIAL_L2_BLOCK_NUM) { @@ -634,8 +635,9 @@ export class P2PClient extends WithTracer implements P2P { } // We delete attestations older than the last block slot minus the number of slots we want to keep in the pool. - if (lastBlockSlot - BigInt(this.keepAttestationsInPoolFor) >= BigInt(INITIAL_L2_BLOCK_NUM)) { - await this.attestationPool.deleteAttestationsOlderThan(lastBlockSlot - BigInt(this.keepAttestationsInPoolFor)); + const lastBlockSlotMinusKeepAttestationsInPoolFor = lastBlockSlot - BigInt(this.keepAttestationsInPoolFor); + if (lastBlockSlotMinusKeepAttestationsInPoolFor >= BigInt(INITIAL_L2_BLOCK_NUM)) { + await this.attestationPool.deleteAttestationsOlderThan(lastBlockSlotMinusKeepAttestationsInPoolFor); } await this.synchedProvenBlockNumber.set(lastBlockNum); From b95bcef2b6207cf62a8fc1f06704013be3e95b54 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Sat, 30 Nov 2024 16:19:40 +0000 Subject: [PATCH 3/4] test: attestation pool pruning --- yarn-project/p2p/src/client/p2p_client.test.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index b277a21509a..43b68442c1c 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -327,5 +327,20 @@ describe('In-Memory P2P Client', () => { }); }); - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7971): tests for attestation pool pruning + describe('Attestation pool pruning', () => { + it('deletes attestations older than the number of slots we want to keep in the pool', async () => { + const advanceToProvenBlockNumber = 20; + const keepAttestationsInPoolFor = 12; + + blockSource.setProvenBlockNumber(0); + (client as any).keepAttestationsInPoolFor = keepAttestationsInPoolFor; + await client.start(); + expect(attestationPool.deleteAttestationsOlderThan).not.toHaveBeenCalled(); + + await advanceToProvenBlock(advanceToProvenBlockNumber); + + expect(attestationPool.deleteAttestationsOlderThan).toHaveBeenCalledTimes(1); + expect(attestationPool.deleteAttestationsOlderThan).toHaveBeenCalledWith(BigInt(advanceToProvenBlockNumber - keepAttestationsInPoolFor)); + }); + }); }); From f6ac46631f4c05762d653cdcd45b4d34bda05a01 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Sat, 30 Nov 2024 16:24:21 +0000 Subject: [PATCH 4/4] fmt --- yarn-project/p2p/src/client/p2p_client.test.ts | 4 +++- yarn-project/p2p/src/client/p2p_client.ts | 4 +--- yarn-project/p2p/src/config.ts | 1 - .../p2p/src/mem_pools/attestation_pool/attestation_pool.ts | 1 - .../attestation_pool/memory_attestation_pool.test.ts | 2 +- .../src/mem_pools/attestation_pool/memory_attestation_pool.ts | 4 ++-- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index 43b68442c1c..219d2caeded 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -340,7 +340,9 @@ describe('In-Memory P2P Client', () => { await advanceToProvenBlock(advanceToProvenBlockNumber); expect(attestationPool.deleteAttestationsOlderThan).toHaveBeenCalledTimes(1); - expect(attestationPool.deleteAttestationsOlderThan).toHaveBeenCalledWith(BigInt(advanceToProvenBlockNumber - keepAttestationsInPoolFor)); + expect(attestationPool.deleteAttestationsOlderThan).toHaveBeenCalledWith( + BigInt(advanceToProvenBlockNumber - keepAttestationsInPoolFor), + ); }); }); }); diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 8e6e86f1761..58575d5e599 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -227,8 +227,7 @@ export class P2PClient extends WithTracer implements P2P { ) { super(telemetry, 'P2PClient'); - const { blockCheckIntervalMS, blockRequestBatchSize, keepAttestationsInPoolFor } = - getP2PConfigFromEnv(); + const { blockCheckIntervalMS, blockRequestBatchSize, keepAttestationsInPoolFor } = getP2PConfigFromEnv(); this.keepAttestationsInPoolFor = keepAttestationsInPoolFor; @@ -647,7 +646,6 @@ export class P2PClient extends WithTracer implements P2P { this.epochProofQuotePool.deleteQuotesToEpoch(BigInt(provenEpochNumber)); } - await this.startServiceIfSynched(); } diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index b16f0f46543..3150722a21a 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -94,7 +94,6 @@ export interface P2PConfig extends P2PReqRespConfig { /** How many slots to keep attestations for. */ keepAttestationsInPoolFor: number; - /** * The interval of the gossipsub heartbeat to perform maintenance tasks. */ 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 5c57e85b87b..bb7ecb5b704 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 @@ -30,7 +30,6 @@ export interface AttestationPool { */ deleteAttestationsOlderThan(slot: bigint): Promise; - /** * Delete Attestations for slot * diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts index 0840f5e7d94..ef80dad21ec 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.test.ts @@ -3,9 +3,9 @@ import { Secp256k1Signer } from '@aztec/foundation/crypto'; import { Fr } from '@aztec/foundation/fields'; import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; +import { jest } from '@jest/globals'; import { type MockProxy, mock } from 'jest-mock-extended'; -import { jest } from '@jest/globals'; import { type PoolInstrumentation } from '../instrumentation.js'; import { InMemoryAttestationPool } from './memory_attestation_pool.js'; import { mockAttestation } from './mocks.js'; diff --git a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts index fa738340c9a..95f9af415cb 100644 --- a/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts +++ b/yarn-project/p2p/src/mem_pools/attestation_pool/memory_attestation_pool.ts @@ -58,7 +58,7 @@ export class InMemoryAttestationPool implements AttestationPool { return total; } - public deleteAttestationsOlderThan(oldestSlot: bigint): Promise { + public async deleteAttestationsOlderThan(oldestSlot: bigint): Promise { const olderThan = []; // Entries are iterated in insertion order, so we can break as soon as we find a slot that is older than the oldestSlot. @@ -74,7 +74,7 @@ export class InMemoryAttestationPool implements AttestationPool { } for (const oldSlot of olderThan) { - this.deleteAttestationsForSlot(oldSlot); + await this.deleteAttestationsForSlot(oldSlot); } return Promise.resolve(); }