diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.test.ts b/yarn-project/circuit-types/src/p2p/block_attestation.test.ts index e60fdb09940..908e4551c17 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.test.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.test.ts @@ -5,23 +5,27 @@ import { BlockAttestation } from './block_attestation.js'; import { makeBlockAttestation } from './mocks.js'; describe('Block Attestation serialization / deserialization', () => { + const checkEquivalence = (serialized: BlockAttestation, deserialized: BlockAttestation) => { + expect(deserialized.getSize()).toEqual(serialized.getSize()); + expect(deserialized).toEqual(serialized); + }; + it('Should serialize / deserialize', () => { const attestation = makeBlockAttestation(); const serialized = attestation.toBuffer(); const deserialized = BlockAttestation.fromBuffer(serialized); - - expect(deserialized).toEqual(attestation); + checkEquivalence(attestation, deserialized); }); it('Should serialize / deserialize + recover sender', () => { const account = Secp256k1Signer.random(); - const proposal = makeBlockAttestation(account); - const serialized = proposal.toBuffer(); + const attestation = makeBlockAttestation(account); + const serialized = attestation.toBuffer(); const deserialized = BlockAttestation.fromBuffer(serialized); - expect(deserialized).toEqual(proposal); + checkEquivalence(attestation, deserialized); // Recover signature const sender = deserialized.getSender(); diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index c808417baf5..3800fa926df 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -72,4 +72,8 @@ export class BlockAttestation extends Gossipable { static empty(): BlockAttestation { return new BlockAttestation(ConsensusPayload.empty(), Signature.empty()); } + + getSize(): number { + return this.payload.getSize() + this.signature.getSize(); + } } diff --git a/yarn-project/circuit-types/src/p2p/block_proposal.test.ts b/yarn-project/circuit-types/src/p2p/block_proposal.test.ts index af69c619532..4be10936fec 100644 --- a/yarn-project/circuit-types/src/p2p/block_proposal.test.ts +++ b/yarn-project/circuit-types/src/p2p/block_proposal.test.ts @@ -5,13 +5,17 @@ import { BlockProposal } from './block_proposal.js'; import { makeBlockProposal } from './mocks.js'; describe('Block Proposal serialization / deserialization', () => { + const checkEquivalence = (serialized: BlockProposal, deserialized: BlockProposal) => { + expect(deserialized.getSize()).toEqual(serialized.getSize()); + expect(deserialized).toEqual(serialized); + }; + it('Should serialize / deserialize', () => { const proposal = makeBlockProposal(); const serialized = proposal.toBuffer(); const deserialized = BlockProposal.fromBuffer(serialized); - - expect(deserialized).toEqual(proposal); + checkEquivalence(proposal, deserialized); }); it('Should serialize / deserialize + recover sender', () => { @@ -21,7 +25,7 @@ describe('Block Proposal serialization / deserialization', () => { const serialized = proposal.toBuffer(); const deserialized = BlockProposal.fromBuffer(serialized); - expect(deserialized).toEqual(proposal); + checkEquivalence(proposal, deserialized); // Recover signature const sender = deserialized.getSender(); diff --git a/yarn-project/circuit-types/src/p2p/block_proposal.ts b/yarn-project/circuit-types/src/p2p/block_proposal.ts index 80cbb80c596..0e8dec898e5 100644 --- a/yarn-project/circuit-types/src/p2p/block_proposal.ts +++ b/yarn-project/circuit-types/src/p2p/block_proposal.ts @@ -75,4 +75,8 @@ export class BlockProposal extends Gossipable { const reader = BufferReader.asReader(buf); return new BlockProposal(reader.readObject(ConsensusPayload), reader.readObject(Signature)); } + + getSize(): number { + return this.payload.getSize() + this.signature.getSize(); + } } diff --git a/yarn-project/circuit-types/src/p2p/consensus_payload.ts b/yarn-project/circuit-types/src/p2p/consensus_payload.ts index f9cb6101b06..d3d13f2d2d0 100644 --- a/yarn-project/circuit-types/src/p2p/consensus_payload.ts +++ b/yarn-project/circuit-types/src/p2p/consensus_payload.ts @@ -9,6 +9,8 @@ import { TxHash } from '../tx/tx_hash.js'; import { type Signable } from './signature_utils.js'; export class ConsensusPayload implements Signable { + private size: number | undefined; + constructor( /** The block header the attestation is made over */ public readonly header: Header, @@ -31,7 +33,9 @@ export class ConsensusPayload implements Signable { } toBuffer(): Buffer { - return serializeToBuffer([this.header, this.archive, this.txHashes.length, this.txHashes]); + const buffer = serializeToBuffer([this.header, this.archive, this.txHashes.length, this.txHashes]); + this.size = buffer.length; + return buffer; } static fromBuffer(buf: Buffer | BufferReader): ConsensusPayload { @@ -50,4 +54,17 @@ export class ConsensusPayload implements Signable { static empty(): ConsensusPayload { return new ConsensusPayload(Header.empty(), Fr.ZERO, []); } + + /** + * Get the size of the consensus payload in bytes. + * @returns The size of the consensus payload. + */ + getSize(): number { + // We cache size to avoid recalculating it + if (this.size) { + return this.size; + } + this.size = this.toBuffer().length; + return this.size; + } } diff --git a/yarn-project/circuit-types/src/p2p/gossipable.ts b/yarn-project/circuit-types/src/p2p/gossipable.ts index 4b9c407cb03..4de52bce26f 100644 --- a/yarn-project/circuit-types/src/p2p/gossipable.ts +++ b/yarn-project/circuit-types/src/p2p/gossipable.ts @@ -23,4 +23,11 @@ export abstract class Gossipable { * - Serialization method */ abstract toBuffer(): Buffer; + + /** + * Get the size of the gossipable object. + * + * This is used for metrics recording. + */ + abstract getSize(): number; } diff --git a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.test.ts b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.test.ts index 78c2e0572c3..78f68edee04 100644 --- a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.test.ts +++ b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.test.ts @@ -19,11 +19,18 @@ describe('epoch proof quote', () => { quote = new EpochProofQuote(payload, Signature.random()); }); + const checkEquivalence = (serialized: EpochProofQuote, deserialized: EpochProofQuote) => { + expect(deserialized.getSize()).toEqual(serialized.getSize()); + expect(deserialized).toEqual(serialized); + }; + it('should serialize and deserialize from buffer', () => { - expect(EpochProofQuote.fromBuffer(quote.toBuffer())).toEqual(quote); + const deserialised = EpochProofQuote.fromBuffer(quote.toBuffer()); + checkEquivalence(quote, deserialised); }); it('should serialize and deserialize from JSON', () => { - expect(EpochProofQuote.fromJSON(quote.toJSON())).toEqual(quote); + const deserialised = EpochProofQuote.fromJSON(quote.toJSON()); + checkEquivalence(quote, deserialised); }); }); diff --git a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.ts b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.ts index a26de238aa6..be551f48f8d 100644 --- a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.ts +++ b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote.ts @@ -69,4 +69,12 @@ export class EpochProofQuote extends Gossipable { signature: this.signature.toViemSignature(), }; } + + /** + * Get the size of the epoch proof quote in bytes. + * @returns The size of the epoch proof quote in bytes. + */ + getSize(): number { + return this.payload.getSize() + this.signature.getSize(); + } } diff --git a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote_payload.ts b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote_payload.ts index 77e75598b73..15086276c06 100644 --- a/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote_payload.ts +++ b/yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote_payload.ts @@ -5,6 +5,10 @@ import { type FieldsOf } from '@aztec/foundation/types'; import { inspect } from 'util'; export class EpochProofQuotePayload { + // Cached values + private asBuffer: Buffer | undefined; + private size: number | undefined; + constructor( public readonly epochToProve: bigint, public readonly validUntilSlot: bigint, @@ -24,7 +28,13 @@ export class EpochProofQuotePayload { } toBuffer(): Buffer { - return serializeToBuffer(...EpochProofQuotePayload.getFields(this)); + // We cache the buffer to avoid recalculating it + if (this.asBuffer) { + return this.asBuffer; + } + this.asBuffer = serializeToBuffer(...EpochProofQuotePayload.getFields(this)); + this.size = this.asBuffer.length; + return this.asBuffer; } static fromBuffer(buf: Buffer | BufferReader): EpochProofQuotePayload { @@ -84,6 +94,16 @@ export class EpochProofQuotePayload { }; } + getSize(): number { + // We cache size to avoid recalculating it + if (this.size) { + return this.size; + } + // Size is cached when calling toBuffer + this.toBuffer(); + return this.size!; + } + [inspect.custom](): string { return `EpochProofQuotePayload { epochToProve: ${this.epochToProve}, validUntilSlot: ${this.validUntilSlot}, bondAmount: ${this.bondAmount}, prover: ${this.prover}, basisPointFee: ${this.basisPointFee} }`; } diff --git a/yarn-project/foundation/src/eth-signature/eth_signature.test.ts b/yarn-project/foundation/src/eth-signature/eth_signature.test.ts index e93c199eacf..aa1c7f93665 100644 --- a/yarn-project/foundation/src/eth-signature/eth_signature.test.ts +++ b/yarn-project/foundation/src/eth-signature/eth_signature.test.ts @@ -20,16 +20,21 @@ describe('eth signature', () => { signature = signer.sign(message); }); + const checkEquivalence = (serialized: Signature, deserialized: Signature) => { + expect(deserialized.getSize()).toEqual(serialized.getSize()); + expect(deserialized).toEqual(serialized); + }; + it('should serialize / deserialize to buffer', () => { const serialized = signature.toBuffer(); const deserialized = Signature.fromBuffer(serialized); - expect(deserialized).toEqual(signature); + checkEquivalence(signature, deserialized); }); it('should serialize / deserialize real signature to hex string', () => { const serialized = signature.to0xString(); const deserialized = Signature.from0xString(serialized); - expect(deserialized).toEqual(signature); + checkEquivalence(signature, deserialized); }); it('should recover signer from signature', () => { @@ -41,13 +46,13 @@ describe('eth signature', () => { const signature = new Signature(Buffer32.random(), Buffer32.random(), 1, false); const serialized = signature.to0xString(); const deserialized = Signature.from0xString(serialized); - expect(deserialized).toEqual(signature); + checkEquivalence(signature, deserialized); }); it('should serialize / deserialize to hex string with 2-digit v', () => { const signature = new Signature(Buffer32.random(), Buffer32.random(), 26, false); const serialized = signature.to0xString(); const deserialized = Signature.from0xString(serialized); - expect(deserialized).toEqual(signature); + checkEquivalence(signature, deserialized); }); }); diff --git a/yarn-project/foundation/src/eth-signature/eth_signature.ts b/yarn-project/foundation/src/eth-signature/eth_signature.ts index b985a8ec74c..4226deb801c 100644 --- a/yarn-project/foundation/src/eth-signature/eth_signature.ts +++ b/yarn-project/foundation/src/eth-signature/eth_signature.ts @@ -19,6 +19,9 @@ export type ViemSignature = { * Contains a signature split into it's primary components (r,s,v) */ export class Signature { + // Cached values + private size: number | undefined; + constructor( /** The r value of the signature */ public readonly r: Buffer32, @@ -73,7 +76,19 @@ export class Signature { } toBuffer(): Buffer { - return serializeToBuffer([this.r, this.s, this.v]); + const buffer = serializeToBuffer([this.r, this.s, this.v]); + this.size = buffer.length; + return buffer; + } + + getSize(): number { + // We cache size to avoid recalculating it + if (this.size) { + return this.size; + } + + this.size = this.toBuffer().length; + return this.size; } to0xString(): `0x${string}` { diff --git a/yarn-project/p2p/src/client/index.ts b/yarn-project/p2p/src/client/index.ts index 5cf0b03f2e9..886426c6aa3 100644 --- a/yarn-project/p2p/src/client/index.ts +++ b/yarn-project/p2p/src/client/index.ts @@ -38,8 +38,8 @@ export const createP2PClient = async ( const mempools: MemPools = { txPool: deps.txPool ?? new AztecKVTxPool(store, telemetry), - attestationPool: deps.attestationPool ?? new InMemoryAttestationPool(), - epochProofQuotePool: deps.epochProofQuotePool ?? new MemoryEpochProofQuotePool(), + attestationPool: deps.attestationPool ?? new InMemoryAttestationPool(telemetry), + epochProofQuotePool: deps.epochProofQuotePool ?? new MemoryEpochProofQuotePool(telemetry), }; let p2pService; 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 a798dedff59..695f2699b02 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 @@ -1,7 +1,11 @@ +import { type BlockAttestation } from '@aztec/circuit-types'; import { Fr } from '@aztec/foundation/fields'; +import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; +import { type MockProxy, mock } from 'jest-mock-extended'; import { type PrivateKeyAccount } from 'viem'; +import { type PoolInstrumentation } from '../instrumentation.js'; import { InMemoryAttestationPool } from './memory_attestation_pool.js'; import { generateAccount, mockAttestation } from './mocks.js'; @@ -10,10 +14,20 @@ const NUMBER_OF_SIGNERS_PER_TEST = 4; describe('MemoryAttestationPool', () => { let ap: InMemoryAttestationPool; let signers: PrivateKeyAccount[]; + const telemetry = new NoopTelemetryClient(); + + // Check that metrics are recorded correctly + let metricsMock: MockProxy>; beforeEach(() => { - ap = new InMemoryAttestationPool(); + // Use noop telemetry client while testing. + + ap = new InMemoryAttestationPool(telemetry); signers = Array.from({ length: NUMBER_OF_SIGNERS_PER_TEST }, generateAccount); + + metricsMock = mock>(); + // Can i overwrite this like this?? + (ap as any).metrics = metricsMock; }); it('should add attestations to pool', async () => { @@ -25,6 +39,9 @@ describe('MemoryAttestationPool', () => { await ap.addAttestations(attestations); + // Check metrics have been updated. + expect(metricsMock.recordAddedObjects).toHaveBeenCalledWith(attestations.length); + const retreivedAttestations = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestations.length).toBe(NUMBER_OF_SIGNERS_PER_TEST); @@ -33,6 +50,8 @@ describe('MemoryAttestationPool', () => { // Delete by slot await ap.deleteAttestationsForSlot(BigInt(slotNumber)); + expect(metricsMock.recordRemovedObjects).toHaveBeenCalledWith(attestations.length); + const retreivedAttestationsAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestationsAfterDelete.length).toBe(0); }); @@ -82,12 +101,16 @@ describe('MemoryAttestationPool', () => { await ap.addAttestations(attestations); + expect(metricsMock.recordAddedObjects).toHaveBeenCalledWith(attestations.length); + const retreivedAttestations = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestations.length).toBe(NUMBER_OF_SIGNERS_PER_TEST); expect(retreivedAttestations).toEqual(attestations); await ap.deleteAttestations(attestations); + expect(metricsMock.recordRemovedObjects).toHaveBeenCalledWith(attestations.length); + const gottenAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(gottenAfterDelete.length).toBe(0); }); @@ -118,12 +141,16 @@ describe('MemoryAttestationPool', () => { await ap.addAttestations(attestations); + expect(metricsMock.recordAddedObjects).toHaveBeenCalledWith(attestations.length); + const retreivedAttestations = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestations.length).toBe(NUMBER_OF_SIGNERS_PER_TEST); expect(retreivedAttestations).toEqual(attestations); await ap.deleteAttestationsForSlotAndProposal(BigInt(slotNumber), proposalId); + expect(metricsMock.recordRemovedObjects).toHaveBeenCalledWith(attestations.length); + const retreivedAttestationsAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); expect(retreivedAttestationsAfterDelete.length).toBe(0); }); 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 34337109afc..daf998b3fb5 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 @@ -1,13 +1,18 @@ import { type BlockAttestation } from '@aztec/circuit-types'; import { createDebugLogger } from '@aztec/foundation/log'; +import { type TelemetryClient } from '@aztec/telemetry-client'; +import { PoolInstrumentation } from '../instrumentation.js'; import { type AttestationPool } from './attestation_pool.js'; export class InMemoryAttestationPool implements AttestationPool { + private metrics: PoolInstrumentation; + private attestations: Map>>; - constructor(private log = createDebugLogger('aztec:attestation_pool')) { + constructor(telemetry: TelemetryClient, private log = createDebugLogger('aztec:attestation_pool')) { this.attestations = new Map(); + this.metrics = new PoolInstrumentation(telemetry, 'InMemoryAttestationPool'); } public getAttestationsForSlot(slot: bigint, proposalId: string): Promise { @@ -35,21 +40,46 @@ export class InMemoryAttestationPool implements AttestationPool { this.log.verbose(`Added attestation for slot ${slotNumber} from ${address}`); } + + // TODO: set these to pending or something ???? + this.metrics.recordAddedObjects(attestations.length); return Promise.resolve(); } + #getNumberOfAttestationsInSlot(slot: bigint): number { + let total = 0; + const slotAttestationMap = getSlotOrDefault(this.attestations, slot); + + if (slotAttestationMap) { + for (const proposalAttestationMap of slotAttestationMap.values() ?? []) { + total += proposalAttestationMap.size; + } + } + return total; + } + public deleteAttestationsForSlot(slot: bigint): Promise { - // TODO(md): check if this will free the memory of the inner hash map + // We count the number of attestations we are removing + const numberOfAttestations = this.#getNumberOfAttestationsInSlot(slot); + this.attestations.delete(slot); - this.log.verbose(`Removed attestation for slot ${slot}`); + this.log.verbose(`Removed ${numberOfAttestations} attestations for slot ${slot}`); + + this.metrics.recordRemovedObjects(numberOfAttestations); return Promise.resolve(); } public deleteAttestationsForSlotAndProposal(slot: bigint, proposalId: string): Promise { - const slotAttestationMap = this.attestations.get(slot); + const slotAttestationMap = getSlotOrDefault(this.attestations, slot); if (slotAttestationMap) { - slotAttestationMap.delete(proposalId); - this.log.verbose(`Removed attestation for slot ${slot}`); + if (slotAttestationMap.has(proposalId)) { + const numberOfAttestations = slotAttestationMap.get(proposalId)?.size ?? 0; + + slotAttestationMap.delete(proposalId); + + this.log.verbose(`Removed ${numberOfAttestations} attestations for slot ${slot} and proposal ${proposalId}`); + this.metrics.recordRemovedObjects(numberOfAttestations); + } } return Promise.resolve(); } @@ -68,6 +98,7 @@ export class InMemoryAttestationPool implements AttestationPool { } } } + this.metrics.recordRemovedObjects(attestations.length); return Promise.resolve(); } } diff --git a/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.test.ts b/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.test.ts index 16ea4aeec52..2358c833a75 100644 --- a/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.test.ts +++ b/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.test.ts @@ -1,12 +1,22 @@ -import { mockEpochProofQuote } from '@aztec/circuit-types'; +import { type EpochProofQuote, mockEpochProofQuote } from '@aztec/circuit-types'; +import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; +import { type MockProxy, mock } from 'jest-mock-extended'; + +import { type PoolInstrumentation } from '../instrumentation.js'; import { MemoryEpochProofQuotePool } from './memory_epoch_proof_quote_pool.js'; describe('MemoryEpochProofQuotePool', () => { let pool: MemoryEpochProofQuotePool; + let metricsMock: MockProxy>; + beforeEach(() => { - pool = new MemoryEpochProofQuotePool(); + const telemetry = new NoopTelemetryClient(); + pool = new MemoryEpochProofQuotePool(telemetry); + + metricsMock = mock>(); + (pool as any).metrics = metricsMock; }); it('should add/get quotes to/from pool', () => { @@ -14,6 +24,8 @@ describe('MemoryEpochProofQuotePool', () => { pool.addQuote(quote); + expect(metricsMock.recordAddedObjects).toHaveBeenCalledWith(1); + const quotes = pool.getQuotes(quote.payload.epochToProve); expect(quotes).toHaveLength(1); @@ -36,6 +48,7 @@ describe('MemoryEpochProofQuotePool', () => { const quotes3 = pool.getQuotes(3n); const quotesForEpoch3 = proofQuotes.filter(x => x.payload.epochToProve === 3n); + const quotesForEpoch2 = proofQuotes.filter(x => x.payload.epochToProve === 2n); expect(quotes3).toHaveLength(quotesForEpoch3.length); expect(quotes3).toEqual(quotesForEpoch3); @@ -43,6 +56,8 @@ describe('MemoryEpochProofQuotePool', () => { // should delete all quotes for epochs 2 and 3 pool.deleteQuotesToEpoch(3n); + expect(metricsMock.recordRemovedObjects).toHaveBeenCalledWith(quotesForEpoch2.length + quotesForEpoch3.length); + expect(pool.getQuotes(2n)).toHaveLength(0); expect(pool.getQuotes(3n)).toHaveLength(0); diff --git a/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.ts b/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.ts index a9166838a1b..419f1c61463 100644 --- a/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.ts +++ b/yarn-project/p2p/src/mem_pools/epoch_proof_quote_pool/memory_epoch_proof_quote_pool.ts @@ -1,26 +1,41 @@ import { type EpochProofQuote } from '@aztec/circuit-types'; +import { type TelemetryClient } from '@aztec/telemetry-client'; +import { PoolInstrumentation } from '../instrumentation.js'; import { type EpochProofQuotePool } from './epoch_proof_quote_pool.js'; export class MemoryEpochProofQuotePool implements EpochProofQuotePool { private quotes: Map; - constructor() { + private metrics: PoolInstrumentation; + + constructor(telemetry: TelemetryClient) { this.quotes = new Map(); + this.metrics = new PoolInstrumentation(telemetry, 'MemoryEpochProofQuotePool'); } + addQuote(quote: EpochProofQuote) { const epoch = quote.payload.epochToProve; if (!this.quotes.has(epoch)) { this.quotes.set(epoch, []); } this.quotes.get(epoch)!.push(quote); + + this.metrics.recordAddedObjects(1); } getQuotes(epoch: bigint): EpochProofQuote[] { return this.quotes.get(epoch) || []; } deleteQuotesToEpoch(epoch: bigint): void { const expiredEpochs = Array.from(this.quotes.keys()).filter(k => k <= epoch); + + let removedObjectsCount = 0; for (const expiredEpoch of expiredEpochs) { + // For logging + removedObjectsCount += this.quotes.get(expiredEpoch)?.length || 0; + this.quotes.delete(expiredEpoch); } + + this.metrics.recordRemovedObjects(removedObjectsCount); } } diff --git a/yarn-project/p2p/src/mem_pools/instrumentation.ts b/yarn-project/p2p/src/mem_pools/instrumentation.ts new file mode 100644 index 00000000000..bb3c8480c9c --- /dev/null +++ b/yarn-project/p2p/src/mem_pools/instrumentation.ts @@ -0,0 +1,71 @@ +import { type Gossipable } from '@aztec/circuit-types'; +import { Attributes, type Histogram, Metrics, type TelemetryClient, type UpDownCounter } from '@aztec/telemetry-client'; + +/** + * Instrumentation class for the Pools (TxPool, AttestationPool, etc). + */ +export class PoolInstrumentation { + /** The number of txs in the mempool */ + private objectsInMempool: UpDownCounter; + /** Tracks tx size */ + private objectSize: Histogram; + + constructor(telemetry: TelemetryClient, name: string) { + const meter = telemetry.getMeter(name); + this.objectsInMempool = meter.createUpDownCounter(Metrics.MEMPOOL_TX_COUNT, { + description: 'The current number of transactions in the mempool', + }); + + this.objectSize = meter.createHistogram(Metrics.MEMPOOL_TX_SIZE, { + unit: 'By', + description: 'The size of transactions in the mempool', + advice: { + explicitBucketBoundaries: [ + 5_000, // 5KB + 10_000, + 20_000, + 50_000, + 75_000, + 100_000, // 100KB + 200_000, + ], + }, + }); + } + + public recordSize(poolObject: PoolObject) { + this.objectSize.record(poolObject.getSize()); + } + + /** + * Updates the metrics with the new objects. + * @param txs - The objects to record + */ + public recordAddedObjects(count = 1, status?: string) { + if (count < 0) { + throw new Error('Count must be positive'); + } + if (count === 0) { + return; + } + const attributes = status ? { [Attributes.STATUS]: status } : {}; + + this.objectsInMempool.add(count, attributes); + } + + /** + * Updates the metrics by removing objects from the mempool. + * @param count - The number of objects to remove from the mempool + */ + public recordRemovedObjects(count = 1, status?: string) { + if (count < 0) { + throw new Error('Count must be positive'); + } + if (count === 0) { + return; + } + + const attributes = status ? { [Attributes.STATUS]: status } : {}; + this.objectsInMempool.add(-1 * count, attributes); + } +} diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts b/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts index 4037e4cbb15..432eefd012a 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts @@ -4,7 +4,7 @@ import { type Logger, createDebugLogger } from '@aztec/foundation/log'; import { type AztecKVStore, type AztecMap, type AztecSet } from '@aztec/kv-store'; import { type TelemetryClient } from '@aztec/telemetry-client'; -import { TxPoolInstrumentation } from './instrumentation.js'; +import { PoolInstrumentation } from '../instrumentation.js'; import { type TxPool } from './tx_pool.js'; /** @@ -23,7 +23,7 @@ export class AztecKVTxPool implements TxPool { #log: Logger; - #metrics: TxPoolInstrumentation; + #metrics: PoolInstrumentation; /** * Class constructor for in-memory TxPool. Initiates our transaction pool as a JS Map. @@ -37,7 +37,7 @@ export class AztecKVTxPool implements TxPool { this.#store = store; this.#log = log; - this.#metrics = new TxPoolInstrumentation(telemetry, 'AztecKVTxPool'); + this.#metrics = new PoolInstrumentation(telemetry, 'AztecKVTxPool'); } public markAsMined(txHashes: TxHash[]): Promise { @@ -51,8 +51,8 @@ export class AztecKVTxPool implements TxPool { void this.#pendingTxs.delete(key); } } - this.#metrics.recordRemovedTxs('pending', deleted); - this.#metrics.recordAddedTxs('mined', txHashes.length); + this.#metrics.recordRemovedObjects(deleted, 'pending'); + this.#metrics.recordAddedObjects(txHashes.length, 'mined'); }); } @@ -107,11 +107,11 @@ export class AztecKVTxPool implements TxPool { pendingCount++; // REFACTOR: Use an lmdb conditional write to avoid race conditions with this write tx void this.#pendingTxs.add(key); - this.#metrics.recordTxSize(tx); + this.#metrics.recordSize(tx); } } - this.#metrics.recordAddedTxs('pending', pendingCount); + this.#metrics.recordAddedObjects(pendingCount, 'pending'); }); } @@ -138,8 +138,8 @@ export class AztecKVTxPool implements TxPool { } } - this.#metrics.recordRemovedTxs('pending', pendingDeleted); - this.#metrics.recordRemovedTxs('mined', minedDeleted); + this.#metrics.recordRemovedObjects(pendingDeleted, 'pending'); + this.#metrics.recordRemovedObjects(minedDeleted, 'mined'); }); } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/instrumentation.ts b/yarn-project/p2p/src/mem_pools/tx_pool/instrumentation.ts deleted file mode 100644 index 941a2f46168..00000000000 --- a/yarn-project/p2p/src/mem_pools/tx_pool/instrumentation.ts +++ /dev/null @@ -1,73 +0,0 @@ -import { type Tx } from '@aztec/circuit-types'; -import { Attributes, type Histogram, Metrics, type TelemetryClient, type UpDownCounter } from '@aztec/telemetry-client'; - -export type TxStatus = 'pending' | 'mined'; - -/** - * Instrumentation class for the TxPool. - */ -export class TxPoolInstrumentation { - /** The number of txs in the mempool */ - private txInMempool: UpDownCounter; - /** Tracks tx size */ - private txSize: Histogram; - - constructor(telemetry: TelemetryClient, name: string) { - const meter = telemetry.getMeter(name); - this.txInMempool = meter.createUpDownCounter(Metrics.MEMPOOL_TX_COUNT, { - description: 'The current number of transactions in the mempool', - }); - - this.txSize = meter.createHistogram(Metrics.MEMPOOL_TX_SIZE, { - unit: 'By', - description: 'The size of transactions in the mempool', - advice: { - explicitBucketBoundaries: [ - 5_000, // 5KB - 10_000, - 20_000, - 50_000, - 75_000, - 100_000, // 100KB - 200_000, - ], - }, - }); - } - - public recordTxSize(tx: Tx) { - this.txSize.record(tx.getSize()); - } - - /** - * Updates the metrics with the new transactions. - * @param txs - The transactions to record - */ - public recordAddedTxs(status: string, count = 1) { - if (count < 0) { - throw new Error('Count must be positive'); - } - if (count === 0) { - return; - } - this.txInMempool.add(count, { - [Attributes.STATUS]: status, - }); - } - - /** - * Updates the metrics by removing transactions from the mempool. - * @param count - The number of transactions to remove from the mempool - */ - public recordRemovedTxs(status: string, count = 1) { - if (count < 0) { - throw new Error('Count must be positive'); - } - if (count === 0) { - return; - } - this.txInMempool.add(-1 * count, { - [Attributes.STATUS]: status, - }); - } -} diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts b/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts index 6fcf2c263c6..9e6d72ea5a4 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts @@ -3,7 +3,7 @@ import { type TxAddedToPoolStats } from '@aztec/circuit-types/stats'; import { createDebugLogger } from '@aztec/foundation/log'; import { type TelemetryClient } from '@aztec/telemetry-client'; -import { TxPoolInstrumentation } from './instrumentation.js'; +import { PoolInstrumentation } from '../instrumentation.js'; import { type TxPool } from './tx_pool.js'; /** @@ -17,7 +17,7 @@ export class InMemoryTxPool implements TxPool { private minedTxs: Set; private pendingTxs: Set; - private metrics: TxPoolInstrumentation; + private metrics: PoolInstrumentation; /** * Class constructor for in-memory TxPool. Initiates our transaction pool as a JS Map. @@ -27,7 +27,7 @@ export class InMemoryTxPool implements TxPool { this.txs = new Map(); this.minedTxs = new Set(); this.pendingTxs = new Set(); - this.metrics = new TxPoolInstrumentation(telemetry, 'InMemoryTxPool'); + this.metrics = new PoolInstrumentation(telemetry, 'InMemoryTxPool'); } public markAsMined(txHashes: TxHash[]): Promise { @@ -36,8 +36,8 @@ export class InMemoryTxPool implements TxPool { this.minedTxs.add(key); this.pendingTxs.delete(key); } - this.metrics.recordRemovedTxs('pending', txHashes.length); - this.metrics.recordAddedTxs('mined', txHashes.length); + this.metrics.recordRemovedObjects(txHashes.length, 'pending'); + this.metrics.recordAddedObjects(txHashes.length, 'mined'); return Promise.resolve(); } @@ -88,12 +88,12 @@ export class InMemoryTxPool implements TxPool { this.txs.set(key, tx); if (!this.minedTxs.has(key)) { pending++; - this.metrics.recordTxSize(tx); + this.metrics.recordSize(tx); this.pendingTxs.add(key); } } - this.metrics.recordAddedTxs('pending', pending); + this.metrics.recordAddedObjects(pending, 'pending'); return Promise.resolve(); } @@ -113,8 +113,8 @@ export class InMemoryTxPool implements TxPool { deletedMined += this.minedTxs.delete(key) ? 1 : 0; } - this.metrics.recordRemovedTxs('pending', deletedPending); - this.metrics.recordRemovedTxs('mined', deletedMined); + this.metrics.recordRemovedObjects(deletedPending, 'pending'); + this.metrics.recordRemovedObjects(deletedMined, 'mined'); return Promise.resolve(); }