From 7e64b39dc739ac8ab4132e18403d27a81b102526 Mon Sep 17 00:00:00 2001 From: just-mitch Date: Wed, 9 Oct 2024 11:02:26 +0000 Subject: [PATCH 01/11] feat: better tracing/metrics in validator and archiver --- yarn-project/archiver/src/archiver/archiver.ts | 1 + .../aztec-node/src/aztec-node/server.ts | 2 +- .../sequencer-client/src/sequencer/sequencer.ts | 5 +++++ yarn-project/telemetry-client/src/attributes.ts | 6 ++++++ yarn-project/validator-client/package.json | 1 + yarn-project/validator-client/src/factory.ts | 5 +++-- yarn-project/validator-client/src/metrics.ts | 8 ++++++++ .../validator-client/src/validator.test.ts | 7 +++++-- yarn-project/validator-client/src/validator.ts | 17 ++++++++++++++++- yarn-project/yarn.lock | 1 + 10 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 yarn-project/validator-client/src/metrics.ts diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index 5707fbf86cf3..10f0271f6e16 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -317,6 +317,7 @@ export class Archiver implements ArchiveSource { // if we are here then we must have a valid proven epoch number await this.store.setProvenL2EpochNumber(Number(provenEpochNumber)); } + this.instrumentation.updateLastProvenBlock(Number(provenBlockNumber)); }; // This is an edge case that we only hit if there are no proposed blocks. diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index a4ab38f8d6ee..0362c93e7785 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -166,7 +166,7 @@ export class AztecNodeService implements AztecNode { const simulationProvider = await createSimulationProvider(config, log); - const validatorClient = createValidatorClient(config, p2pClient); + const validatorClient = createValidatorClient(config, p2pClient, telemetry); // now create the sequencer const sequencer = config.disableSequencer diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 7710e9a1f21b..6e3ac4f1618f 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -509,6 +509,11 @@ export class Sequencer { this.isFlushing = true; } + @trackSpan('Sequencer.collectAttestations', (block, txHashes) => ({ + [Attributes.BLOCK_NUMBER]: block.number, + [Attributes.BLOCK_ARCHIVE]: block.archive.toString(), + [Attributes.BLOCK_TXS_COUNT]: txHashes.length, + })) protected async collectAttestations(block: L2Block, txHashes: TxHash[]): Promise { // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7962): inefficient to have a round trip in here - this should be cached const committee = await this.publisher.getCurrentEpochCommittee(); diff --git a/yarn-project/telemetry-client/src/attributes.ts b/yarn-project/telemetry-client/src/attributes.ts index 4a320161d699..9895f209c423 100644 --- a/yarn-project/telemetry-client/src/attributes.ts +++ b/yarn-project/telemetry-client/src/attributes.ts @@ -38,8 +38,12 @@ export const APP_CIRCUIT_NAME = 'aztec.circuit.app_circuit_name'; */ export const APP_CIRCUIT_TYPE = 'aztec.circuit.app_circuit_type'; +/** The block archive */ +export const BLOCK_ARCHIVE = 'aztec.block.archive'; /** The block number */ export const BLOCK_NUMBER = 'aztec.block.number'; +/** The slot number */ +export const SLOT_NUMBER = 'aztec.slot.number'; /** The parent's block number */ export const BLOCK_PARENT = 'aztec.block.parent'; /** How many txs are being processed to build this block */ @@ -70,3 +74,5 @@ export const MERKLE_TREE_NAME = 'aztec.merkle_tree.name'; export const ROLLUP_PROVER_ID = 'aztec.rollup.prover_id'; /** Whether the proof submission was timed out (delayed more than 20 min) */ export const PROOF_TIMED_OUT = 'aztec.proof.timed_out'; + +export const P2P_ID = 'aztec.p2p.id'; diff --git a/yarn-project/validator-client/package.json b/yarn-project/validator-client/package.json index a3cb3cf04faf..7fff0272c513 100644 --- a/yarn-project/validator-client/package.json +++ b/yarn-project/validator-client/package.json @@ -64,6 +64,7 @@ "@aztec/ethereum": "workspace:^", "@aztec/foundation": "workspace:^", "@aztec/p2p": "workspace:^", + "@aztec/telemetry-client": "workspace:^", "@aztec/types": "workspace:^", "koa": "^2.14.2", "koa-router": "^12.0.0", diff --git a/yarn-project/validator-client/src/factory.ts b/yarn-project/validator-client/src/factory.ts index 1438448d8509..0d48ef4c3730 100644 --- a/yarn-project/validator-client/src/factory.ts +++ b/yarn-project/validator-client/src/factory.ts @@ -1,11 +1,12 @@ import { type P2P } from '@aztec/p2p'; +import { type TelemetryClient } from '@aztec/telemetry-client'; import { generatePrivateKey } from 'viem/accounts'; import { type ValidatorClientConfig } from './config.js'; import { ValidatorClient } from './validator.js'; -export function createValidatorClient(config: ValidatorClientConfig, p2pClient: P2P) { +export function createValidatorClient(config: ValidatorClientConfig, p2pClient: P2P, telemetry: TelemetryClient) { if (config.disableValidator) { return undefined; } @@ -13,5 +14,5 @@ export function createValidatorClient(config: ValidatorClientConfig, p2pClient: if (config.validatorPrivateKey === undefined || config.validatorPrivateKey === '') { config.validatorPrivateKey = generatePrivateKey(); } - return ValidatorClient.new(config, p2pClient); + return ValidatorClient.new(config, p2pClient, telemetry); } diff --git a/yarn-project/validator-client/src/metrics.ts b/yarn-project/validator-client/src/metrics.ts new file mode 100644 index 000000000000..bb1dba470696 --- /dev/null +++ b/yarn-project/validator-client/src/metrics.ts @@ -0,0 +1,8 @@ +import { type TelemetryClient, type Tracer } from '@aztec/telemetry-client'; + +export class ValidatorMetrics { + public readonly tracer: Tracer; + constructor(client: TelemetryClient, name = 'Validator') { + this.tracer = client.getTracer(name); + } +} diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index fc870184fecd..8edc491ce0e6 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -6,6 +6,7 @@ import { makeHeader } from '@aztec/circuits.js/testing'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; import { type P2P } from '@aztec/p2p'; +import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { describe, expect, it } from '@jest/globals'; import { type MockProxy, mock } from 'jest-mock-extended'; @@ -39,12 +40,14 @@ describe('ValidationService', () => { attestationWaitTimeoutMs: 1000, disableValidator: false, }; - validatorClient = ValidatorClient.new(config, p2pClient); + validatorClient = ValidatorClient.new(config, p2pClient, new NoopTelemetryClient()); }); it('Should throw error if an invalid private key is provided', () => { config.validatorPrivateKey = '0x1234567890123456789'; - expect(() => ValidatorClient.new(config, p2pClient)).toThrow(InvalidValidatorPrivateKeyError); + expect(() => ValidatorClient.new(config, p2pClient, new NoopTelemetryClient())).toThrow( + InvalidValidatorPrivateKeyError, + ); }); it('Should create a valid block proposal', async () => { diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 7e96391197fc..e7125780f4c8 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -5,6 +5,7 @@ import { type Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { sleep } from '@aztec/foundation/sleep'; import { type P2P } from '@aztec/p2p'; +import { Attributes, TelemetryClient, Tracer, trackSpan } from '@aztec/telemetry-client'; import { type ValidatorClientConfig } from './config.js'; import { ValidationService } from './duties/validation_service.js'; @@ -15,6 +16,7 @@ import { } from './errors/validator.error.js'; import { type ValidatorKeyStore } from './key_store/interface.js'; import { LocalKeyStore } from './key_store/local_key_store.js'; +import { ValidatorMetrics } from './metrics.js'; export interface Validator { start(): Promise; @@ -32,21 +34,24 @@ export interface Validator { */ export class ValidatorClient implements Validator { private validationService: ValidationService; + private metrics: ValidatorMetrics; constructor( keyStore: ValidatorKeyStore, private p2pClient: P2P, private attestationPoolingIntervalMs: number, private attestationWaitTimeoutMs: number, + telemetry: TelemetryClient, private log = createDebugLogger('aztec:validator'), ) { //TODO: We need to setup and store all of the currently active validators https://github.com/AztecProtocol/aztec-packages/issues/7962 + this.metrics = new ValidatorMetrics(telemetry); this.validationService = new ValidationService(keyStore); this.log.verbose('Initialized validator'); } - static new(config: ValidatorClientConfig, p2pClient: P2P) { + static new(config: ValidatorClientConfig, p2pClient: P2P, telemetry: TelemetryClient) { if (!config.validatorPrivateKey) { throw new InvalidValidatorPrivateKeyError(); } @@ -59,11 +64,16 @@ export class ValidatorClient implements Validator { p2pClient, config.attestationPoolingIntervalMs, config.attestationWaitTimeoutMs, + telemetry, ); validator.registerBlockProposalHandler(); return validator; } + get tracer(): Tracer { + return this.metrics.tracer; + } + public start() { // Sync the committee from the smart contract // https://github.com/AztecProtocol/aztec-packages/issues/7962 @@ -128,6 +138,11 @@ export class ValidatorClient implements Validator { return this.validationService.createBlockProposal(header, archive, txs); } + @trackSpan('ValidatorClient.broadcastBlockProposal', proposal => ({ + [Attributes.BLOCK_NUMBER]: proposal.payload.header.globalVariables.blockNumber.toNumber(), + [Attributes.SLOT_NUMBER]: proposal.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.P2P_ID]: proposal.p2pMessageIdentifier().toString(), + })) broadcastBlockProposal(proposal: BlockProposal): void { this.p2pClient.broadcastProposal(proposal); } diff --git a/yarn-project/yarn.lock b/yarn-project/yarn.lock index 9b1d0fc366e6..a2950f5137a2 100644 --- a/yarn-project/yarn.lock +++ b/yarn-project/yarn.lock @@ -1227,6 +1227,7 @@ __metadata: "@aztec/ethereum": "workspace:^" "@aztec/foundation": "workspace:^" "@aztec/p2p": "workspace:^" + "@aztec/telemetry-client": "workspace:^" "@aztec/types": "workspace:^" "@jest/globals": ^29.5.0 "@types/jest": ^29.5.0 From 7dbea350a0f7ddad79d33defcb38bef4e78dc629 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 12:48:38 +0000 Subject: [PATCH 02/11] fix: Add tracing of receiving block proposal --- yarn-project/p2p/src/client/index.ts | 1 + yarn-project/p2p/src/service/libp2p_service.ts | 14 +++++++++++++- yarn-project/p2p/src/service/service_metrics.ts | 11 +++++++++++ yarn-project/telemetry-client/src/index.ts | 1 + yarn-project/telemetry-client/src/with_tracer.ts | 13 +++++++++++++ yarn-project/validator-client/src/metrics.ts | 8 -------- yarn-project/validator-client/src/validator.ts | 15 +++++---------- 7 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 yarn-project/p2p/src/service/service_metrics.ts create mode 100644 yarn-project/telemetry-client/src/with_tracer.ts delete mode 100644 yarn-project/validator-client/src/metrics.ts diff --git a/yarn-project/p2p/src/client/index.ts b/yarn-project/p2p/src/client/index.ts index 237bc1518015..85eb9f35d5e6 100644 --- a/yarn-project/p2p/src/client/index.ts +++ b/yarn-project/p2p/src/client/index.ts @@ -58,6 +58,7 @@ export const createP2PClient = async ( proofVerifier, worldStateSynchronizer, store, + telemetry, ); } else { p2pService = new DummyP2PService(); diff --git a/yarn-project/p2p/src/service/libp2p_service.ts b/yarn-project/p2p/src/service/libp2p_service.ts index 1180dffdc9f5..43a97d516e06 100644 --- a/yarn-project/p2p/src/service/libp2p_service.ts +++ b/yarn-project/p2p/src/service/libp2p_service.ts @@ -59,6 +59,7 @@ import { } from './reqresp/interface.js'; import { ReqResp } from './reqresp/reqresp.js'; import type { P2PService, PeerDiscoveryService } from './service.js'; +import { Attributes, TelemetryClient, trackSpan, WithTracer } from '@aztec/telemetry-client'; /** * Create a libp2p peer ID from the private key if provided, otherwise creates a new random ID. @@ -79,7 +80,7 @@ export async function createLibP2PPeerId(privateKey?: string): Promise { /** * Lib P2P implementation of the P2PService interface. */ -export class LibP2PService implements P2PService { +export class LibP2PService extends WithTracer implements P2PService { private jobQueue: SerialQueue = new SerialQueue(); private peerManager: PeerManager; private discoveryRunningPromise?: RunningPromise; @@ -104,9 +105,13 @@ export class LibP2PService implements P2PService { private l2BlockSource: L2BlockSource, private proofVerifier: ClientProtocolCircuitVerifier, private worldStateSynchronizer: WorldStateSynchronizer, + telemetry: TelemetryClient, private requestResponseHandlers: ReqRespSubProtocolHandlers = DEFAULT_SUB_PROTOCOL_HANDLERS, private logger = createDebugLogger('aztec:libp2p_service'), ) { + // Instatntiate tracer + super(telemetry, 'LibP2PService'); + this.peerManager = new PeerManager(node, peerDiscoveryService, config, logger); this.node.services.pubsub.score.params.appSpecificScore = (peerId: string) => { return this.peerManager.getPeerScore(peerId); @@ -210,6 +215,7 @@ export class LibP2PService implements P2PService { proofVerifier: ClientProtocolCircuitVerifier, worldStateSynchronizer: WorldStateSynchronizer, store: AztecKVStore, + telemetry: TelemetryClient, ) { const { tcpListenAddress, tcpAnnounceAddress, minPeerCount, maxPeerCount } = config; const bindAddrTcp = convertToMultiaddr(tcpListenAddress, 'tcp'); @@ -299,6 +305,7 @@ export class LibP2PService implements P2PService { l2BlockSource, proofVerifier, worldStateSynchronizer, + telemetry, requestResponseHandlers, ); } @@ -402,6 +409,11 @@ export class LibP2PService implements P2PService { * @param block - The block to process. */ // REVIEW: callback pattern https://github.com/AztecProtocol/aztec-packages/issues/7963 + @trackSpan('Libp2pService.processBlockFromPeer', block => ({ + [Attributes.BLOCK_NUMBER]: block.payload.header.globalVariables.blockNumber.toNumber(), + [Attributes.SLOT_NUMBER]: block.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.P2P_ID]: block.p2pMessageIdentifier().toString(), + })) private async processBlockFromPeer(block: BlockProposal): Promise { this.logger.verbose(`Received block ${block.p2pMessageIdentifier()} from external peer.`); const attestation = await this.blockReceivedCallback(block); diff --git a/yarn-project/p2p/src/service/service_metrics.ts b/yarn-project/p2p/src/service/service_metrics.ts new file mode 100644 index 000000000000..09147218deb1 --- /dev/null +++ b/yarn-project/p2p/src/service/service_metrics.ts @@ -0,0 +1,11 @@ +// Goal +// - Emit something that is the source of a gossip message +// - Measure when we receive a message from a peer +// From this - and from looking at loki, we should be able to determine that amount of +// time that it takes for messages to make their way across the p2p network + +// - ReqResp +// - Store the time that it takes to send a message and to receive a response +// - but do not store the undefined messages + +export class LibP2PServiceMetrics {} diff --git a/yarn-project/telemetry-client/src/index.ts b/yarn-project/telemetry-client/src/index.ts index 909f43f644c8..2fd66afb9ebf 100644 --- a/yarn-project/telemetry-client/src/index.ts +++ b/yarn-project/telemetry-client/src/index.ts @@ -1,2 +1,3 @@ export * from './telemetry.js'; export * from './histogram_utils.js'; +export * from './with_tracer.js'; diff --git a/yarn-project/telemetry-client/src/with_tracer.ts b/yarn-project/telemetry-client/src/with_tracer.ts new file mode 100644 index 000000000000..a5ef7fc9babe --- /dev/null +++ b/yarn-project/telemetry-client/src/with_tracer.ts @@ -0,0 +1,13 @@ +import { type Tracer } from "./telemetry.js"; +import { type TelemetryClient } from "./telemetry.js"; + +/** + * A minimal class that enables recording metrics with a telemetry client. + * This base class enables tracing + */ +export class WithTracer { + public readonly tracer: Tracer; + constructor(client: TelemetryClient, name: string) { + this.tracer = client.getTracer(name); + } +} diff --git a/yarn-project/validator-client/src/metrics.ts b/yarn-project/validator-client/src/metrics.ts deleted file mode 100644 index bb1dba470696..000000000000 --- a/yarn-project/validator-client/src/metrics.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { type TelemetryClient, type Tracer } from '@aztec/telemetry-client'; - -export class ValidatorMetrics { - public readonly tracer: Tracer; - constructor(client: TelemetryClient, name = 'Validator') { - this.tracer = client.getTracer(name); - } -} diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index e7125780f4c8..2c767d438486 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -5,7 +5,7 @@ import { type Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { sleep } from '@aztec/foundation/sleep'; import { type P2P } from '@aztec/p2p'; -import { Attributes, TelemetryClient, Tracer, trackSpan } from '@aztec/telemetry-client'; +import { Attributes, TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client'; import { type ValidatorClientConfig } from './config.js'; import { ValidationService } from './duties/validation_service.js'; @@ -16,7 +16,6 @@ import { } from './errors/validator.error.js'; import { type ValidatorKeyStore } from './key_store/interface.js'; import { LocalKeyStore } from './key_store/local_key_store.js'; -import { ValidatorMetrics } from './metrics.js'; export interface Validator { start(): Promise; @@ -32,9 +31,8 @@ export interface Validator { /** Validator Client */ -export class ValidatorClient implements Validator { +export class ValidatorClient extends WithTracer implements Validator { private validationService: ValidationService; - private metrics: ValidatorMetrics; constructor( keyStore: ValidatorKeyStore, @@ -44,9 +42,10 @@ export class ValidatorClient implements Validator { telemetry: TelemetryClient, private log = createDebugLogger('aztec:validator'), ) { - //TODO: We need to setup and store all of the currently active validators https://github.com/AztecProtocol/aztec-packages/issues/7962 + // Instatntiate tracer + super(telemetry, 'Validator'); - this.metrics = new ValidatorMetrics(telemetry); + //TODO: We need to setup and store all of the currently active validators https://github.com/AztecProtocol/aztec-packages/issues/7962 this.validationService = new ValidationService(keyStore); this.log.verbose('Initialized validator'); } @@ -70,10 +69,6 @@ export class ValidatorClient implements Validator { return validator; } - get tracer(): Tracer { - return this.metrics.tracer; - } - public start() { // Sync the committee from the smart contract // https://github.com/AztecProtocol/aztec-packages/issues/7962 From 58dddb6bc06a2478e3e601d588aea74ccb400924 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 12:57:44 +0000 Subject: [PATCH 03/11] feat: add tracing for block proposals + attestations --- .../src/p2p/block_attestation.ts | 4 ++-- .../memory_attestation_pool.ts | 2 +- yarn-project/p2p/src/client/p2p_client.ts | 13 ++++++++++- .../p2p/src/service/libp2p_service.ts | 23 ++++++++++++++++++- .../validator-client/src/validator.ts | 5 ---- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index c808417baf5d..0e868382a2e1 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -1,6 +1,6 @@ import { type EthAddress } from '@aztec/circuits.js'; import { Buffer32 } from '@aztec/foundation/buffer'; -import { recoverAddress } from '@aztec/foundation/crypto'; +import { keccak256, recoverAddress } from '@aztec/foundation/crypto'; import { Signature } from '@aztec/foundation/eth-signature'; import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; @@ -37,7 +37,7 @@ export class BlockAttestation extends Gossipable { } override p2pMessageIdentifier(): Buffer32 { - return BlockAttestationHash.fromField(this.payload.archive); + return new BlockAttestationHash(keccak256(this.signature.toBuffer())); } /**Get sender diff --git a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts index 34337109afcf..4c1c414c7b7d 100644 --- a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts +++ b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts @@ -26,7 +26,7 @@ export class InMemoryAttestationPool implements AttestationPool { // Perf: order and group by slot before insertion const slotNumber = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.p2pMessageIdentifier().toString(); + const proposalId = attestation.payload.archive.toString(); const address = attestation.getSender(); const slotAttestationMap = getSlotOrDefault(this.attestations, slotNumber.toBigInt()); diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index b22b42700be7..39733800c939 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -20,6 +20,7 @@ import { type EpochProofQuotePool } from '../epoch_proof_quote_pool/epoch_proof_ import { TX_REQ_PROTOCOL } from '../service/reqresp/interface.js'; import type { P2PService } from '../service/service.js'; import { type TxPool } from '../tx_pool/index.js'; +import { Attributes, TelemetryClient, trackSpan, WithTracer } from '@aztec/telemetry-client'; /** * Enum defining the possible states of the p2p client. @@ -168,7 +169,7 @@ export interface P2P { /** * The P2P client implementation. */ -export class P2PClient implements P2P { +export class P2PClient extends WithTracer implements P2P { /** L2 block download to stay in sync with latest blocks. */ private latestBlockDownloader: L2BlockDownloader; @@ -207,8 +208,12 @@ export class P2PClient implements P2P { private epochProofQuotePool: EpochProofQuotePool, private p2pService: P2PService, private keepProvenTxsFor: number, + telemetryClient: TelemetryClient, private log = createDebugLogger('aztec:p2p'), ) { + super(telemetryClient, 'P2PClient'); + + const { blockCheckIntervalMS: checkInterval, l2QueueSize: p2pL2QueueSize } = getP2PConfigEnvVars(); const l2DownloaderOpts = { maxQueueSize: p2pL2QueueSize, pollIntervalMS: checkInterval }; // TODO(palla/prover-node): This effectively downloads blocks twice from the archiver, which is an issue @@ -311,6 +316,12 @@ export class P2PClient implements P2P { this.log.info('P2P client stopped.'); } + @trackSpan('p2pClient.broadcastProposal', proposal => ({ + [Attributes.BLOCK_NUMBER]: proposal.payload.header.globalVariables.blockNumber.toNumber(), + [Attributes.SLOT_NUMBER]: proposal.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.BLOCK_ARCHIVE]: proposal.payload.archive.toString(), + [Attributes.P2P_ID]: proposal.p2pMessageIdentifier().toString(), + })) public broadcastProposal(proposal: BlockProposal): void { this.log.verbose(`Broadcasting proposal ${proposal.p2pMessageIdentifier()} to peers`); return this.p2pService.propagate(proposal); diff --git a/yarn-project/p2p/src/service/libp2p_service.ts b/yarn-project/p2p/src/service/libp2p_service.ts index 43a97d516e06..42199949d1c6 100644 --- a/yarn-project/p2p/src/service/libp2p_service.ts +++ b/yarn-project/p2p/src/service/libp2p_service.ts @@ -397,6 +397,11 @@ export class LibP2PService extends WithTracer implements P2PService { * * @param attestation - The attestation to process. */ + @trackSpan('Libp2pService.processAttestationFromPeer', attestation => ({ + [Attributes.BLOCK_NUMBER]: attestation.payload.header.globalVariables.blockNumber.toNumber(), + [Attributes.SLOT_NUMBER]: attestation.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.P2P_ID]: attestation.p2pMessageIdentifier().toString(), + })) private async processAttestationFromPeer(attestation: BlockAttestation): Promise { this.logger.verbose(`Received attestation ${attestation.p2pMessageIdentifier()} from external peer.`); await this.attestationPool.addAttestations([attestation]); @@ -412,6 +417,7 @@ export class LibP2PService extends WithTracer implements P2PService { @trackSpan('Libp2pService.processBlockFromPeer', block => ({ [Attributes.BLOCK_NUMBER]: block.payload.header.globalVariables.blockNumber.toNumber(), [Attributes.SLOT_NUMBER]: block.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.BLOCK_ARCHIVE]: block.payload.archive.toString(), [Attributes.P2P_ID]: block.p2pMessageIdentifier().toString(), })) private async processBlockFromPeer(block: BlockProposal): Promise { @@ -421,10 +427,25 @@ export class LibP2PService extends WithTracer implements P2PService { // TODO: fix up this pattern - the abstraction is not nice // The attestation can be undefined if no handler is registered / the validator deems the block invalid if (attestation != undefined) { - this.propagate(attestation); + this.broadcastAttestation(attestation); } } + /** + * Broadcast an attestation to all peers. + * @param attestation - The attestation to broadcast. + */ + @trackSpan('Libp2pService.broadcastAttestation', attestation => ({ + [Attributes.BLOCK_NUMBER]: attestation.payload.header.globalVariables.blockNumber.toNumber(), + [Attributes.SLOT_NUMBER]: attestation.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.BLOCK_ARCHIVE]: attestation.payload.archive.toString(), + [Attributes.P2P_ID]: attestation.p2pMessageIdentifier().toString(), + })) + private broadcastAttestation(attestation: BlockAttestation): void { + this.propagate(attestation); + } + + private processEpochProofQuoteFromPeer(epochProofQuote: EpochProofQuote): void { this.logger.verbose(`Received epoch proof quote ${epochProofQuote.p2pMessageIdentifier()} from external peer.`); this.epochProofQuotePool.addQuote(epochProofQuote); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 2c767d438486..544d7cf65d06 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -133,11 +133,6 @@ export class ValidatorClient extends WithTracer implements Validator { return this.validationService.createBlockProposal(header, archive, txs); } - @trackSpan('ValidatorClient.broadcastBlockProposal', proposal => ({ - [Attributes.BLOCK_NUMBER]: proposal.payload.header.globalVariables.blockNumber.toNumber(), - [Attributes.SLOT_NUMBER]: proposal.payload.header.globalVariables.slotNumber.toNumber(), - [Attributes.P2P_ID]: proposal.p2pMessageIdentifier().toString(), - })) broadcastBlockProposal(proposal: BlockProposal): void { this.p2pClient.broadcastProposal(proposal); } From 5526dc64b2167fa4f7ca814571ac435a5bc68cf5 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:02:33 +0000 Subject: [PATCH 04/11] chore: update attestation pool, give proposal access to parent identifier --- .../circuit-types/src/p2p/block_attestation.ts | 4 ++++ .../attestation_pool/memory_attestation_pool.test.ts | 12 ++++++------ .../src/attestation_pool/memory_attestation_pool.ts | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index 0e868382a2e1..d01dddbb8ae1 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -40,6 +40,10 @@ export class BlockAttestation extends Gossipable { return new BlockAttestationHash(keccak256(this.signature.toBuffer())); } + public proposalP2PMessageIdentifier(): Buffer32 { + return Buffer32.fromField(this.payload.archive); + } + /**Get sender * * Lazily evaluate and cache the sender of the attestation diff --git a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.test.ts b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.test.ts index a798dedff595..172c390e0163 100644 --- a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.test.ts +++ b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.test.ts @@ -21,7 +21,7 @@ describe('MemoryAttestationPool', () => { const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].p2pMessageIdentifier().toString(); + const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); await ap.addAttestations(attestations); @@ -45,7 +45,7 @@ describe('MemoryAttestationPool', () => { for (const attestation of attestations) { const slot = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.p2pMessageIdentifier().toString(); + const proposalId = attestation.proposalP2PMessageIdentifier().toString(); const retreivedAttestations = await ap.getAttestationsForSlot(slot.toBigInt(), proposalId); expect(retreivedAttestations.length).toBe(1); @@ -65,7 +65,7 @@ describe('MemoryAttestationPool', () => { for (const attestation of attestations) { const slot = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.p2pMessageIdentifier().toString(); + const proposalId = attestation.proposalP2PMessageIdentifier().toString(); const retreivedAttestations = await ap.getAttestationsForSlot(slot.toBigInt(), proposalId); expect(retreivedAttestations.length).toBe(1); @@ -78,7 +78,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].p2pMessageIdentifier().toString(); + const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); await ap.addAttestations(attestations); @@ -96,7 +96,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].p2pMessageIdentifier().toString(); + const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); await ap.addAttestations(attestations); @@ -114,7 +114,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].p2pMessageIdentifier().toString(); + const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); await ap.addAttestations(attestations); diff --git a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts index 4c1c414c7b7d..2dacc65f8a46 100644 --- a/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts +++ b/yarn-project/p2p/src/attestation_pool/memory_attestation_pool.ts @@ -26,7 +26,7 @@ export class InMemoryAttestationPool implements AttestationPool { // Perf: order and group by slot before insertion const slotNumber = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.payload.archive.toString(); + const proposalId = attestation.proposalP2PMessageIdentifier().toString(); const address = attestation.getSender(); const slotAttestationMap = getSlotOrDefault(this.attestations, slotNumber.toBigInt()); @@ -59,7 +59,7 @@ export class InMemoryAttestationPool implements AttestationPool { const slotNumber = attestation.payload.header.globalVariables.slotNumber; const slotAttestationMap = this.attestations.get(slotNumber.toBigInt()); if (slotAttestationMap) { - const proposalId = attestation.p2pMessageIdentifier().toString(); + const proposalId = attestation.proposalP2PMessageIdentifier().toString(); const proposalAttestationMap = getProposalOrDefault(slotAttestationMap, proposalId); if (proposalAttestationMap) { const address = attestation.getSender(); From 3a157cbc38f0a4b92849afe3b2460a4bfe989b14 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:04:14 +0000 Subject: [PATCH 05/11] fmt --- yarn-project/p2p/src/client/p2p_client.ts | 3 +-- yarn-project/p2p/src/service/libp2p_service.ts | 3 +-- yarn-project/telemetry-client/src/with_tracer.ts | 11 +++++------ yarn-project/validator-client/src/validator.ts | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 39733800c939..5da4f7519549 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -11,6 +11,7 @@ import { import { INITIAL_L2_BLOCK_NUM } from '@aztec/circuits.js/constants'; import { createDebugLogger } from '@aztec/foundation/log'; import { type AztecKVStore, type AztecSingleton } from '@aztec/kv-store'; +import { Attributes, type TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client'; import { type ENR } from '@chainsafe/enr'; @@ -20,7 +21,6 @@ import { type EpochProofQuotePool } from '../epoch_proof_quote_pool/epoch_proof_ import { TX_REQ_PROTOCOL } from '../service/reqresp/interface.js'; import type { P2PService } from '../service/service.js'; import { type TxPool } from '../tx_pool/index.js'; -import { Attributes, TelemetryClient, trackSpan, WithTracer } from '@aztec/telemetry-client'; /** * Enum defining the possible states of the p2p client. @@ -213,7 +213,6 @@ export class P2PClient extends WithTracer implements P2P { ) { super(telemetryClient, 'P2PClient'); - const { blockCheckIntervalMS: checkInterval, l2QueueSize: p2pL2QueueSize } = getP2PConfigEnvVars(); const l2DownloaderOpts = { maxQueueSize: p2pL2QueueSize, pollIntervalMS: checkInterval }; // TODO(palla/prover-node): This effectively downloads blocks twice from the archiver, which is an issue diff --git a/yarn-project/p2p/src/service/libp2p_service.ts b/yarn-project/p2p/src/service/libp2p_service.ts index 42199949d1c6..95b1c410f0e2 100644 --- a/yarn-project/p2p/src/service/libp2p_service.ts +++ b/yarn-project/p2p/src/service/libp2p_service.ts @@ -18,6 +18,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { SerialQueue } from '@aztec/foundation/queue'; import { RunningPromise } from '@aztec/foundation/running-promise'; import type { AztecKVStore } from '@aztec/kv-store'; +import { Attributes, type TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client'; import { type ENR } from '@chainsafe/enr'; import { type GossipSub, type GossipSubComponents, gossipsub } from '@chainsafe/libp2p-gossipsub'; @@ -59,7 +60,6 @@ import { } from './reqresp/interface.js'; import { ReqResp } from './reqresp/reqresp.js'; import type { P2PService, PeerDiscoveryService } from './service.js'; -import { Attributes, TelemetryClient, trackSpan, WithTracer } from '@aztec/telemetry-client'; /** * Create a libp2p peer ID from the private key if provided, otherwise creates a new random ID. @@ -445,7 +445,6 @@ export class LibP2PService extends WithTracer implements P2PService { this.propagate(attestation); } - private processEpochProofQuoteFromPeer(epochProofQuote: EpochProofQuote): void { this.logger.verbose(`Received epoch proof quote ${epochProofQuote.p2pMessageIdentifier()} from external peer.`); this.epochProofQuotePool.addQuote(epochProofQuote); diff --git a/yarn-project/telemetry-client/src/with_tracer.ts b/yarn-project/telemetry-client/src/with_tracer.ts index a5ef7fc9babe..e23b2b5822ed 100644 --- a/yarn-project/telemetry-client/src/with_tracer.ts +++ b/yarn-project/telemetry-client/src/with_tracer.ts @@ -1,13 +1,12 @@ -import { type Tracer } from "./telemetry.js"; -import { type TelemetryClient } from "./telemetry.js"; +import { type TelemetryClient, type Tracer } from './telemetry.js'; /** * A minimal class that enables recording metrics with a telemetry client. * This base class enables tracing */ export class WithTracer { - public readonly tracer: Tracer; - constructor(client: TelemetryClient, name: string) { - this.tracer = client.getTracer(name); - } + public readonly tracer: Tracer; + constructor(client: TelemetryClient, name: string) { + this.tracer = client.getTracer(name); + } } diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 544d7cf65d06..518b7abebec6 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -5,7 +5,7 @@ import { type Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { sleep } from '@aztec/foundation/sleep'; import { type P2P } from '@aztec/p2p'; -import { Attributes, TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client'; +import { type TelemetryClient, WithTracer } from '@aztec/telemetry-client'; import { type ValidatorClientConfig } from './config.js'; import { ValidationService } from './duties/validation_service.js'; From 238925230fb9b4ace88330d118c7acaec408f259 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:12:17 +0000 Subject: [PATCH 06/11] fmt 2: electric boogaloo --- yarn-project/p2p/src/client/index.ts | 1 + .../p2p/src/client/p2p_client.test.ts | 58 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/yarn-project/p2p/src/client/index.ts b/yarn-project/p2p/src/client/index.ts index 85eb9f35d5e6..902f307791e3 100644 --- a/yarn-project/p2p/src/client/index.ts +++ b/yarn-project/p2p/src/client/index.ts @@ -71,6 +71,7 @@ export const createP2PClient = async ( epochProofQuotePool, p2pService, config.keepProvenTxsInPoolFor, + telemetry, ); }; diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index 3d16237dfb17..77c2014acf48 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -3,6 +3,8 @@ import { mockEpochProofQuote, mockTx } from '@aztec/circuit-types'; import { retryUntil } from '@aztec/foundation/retry'; import { type AztecKVStore } from '@aztec/kv-store'; import { openTmpStore } from '@aztec/kv-store/utils'; +import { type TelemetryClient } from '@aztec/telemetry-client'; +import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { expect, jest } from '@jest/globals'; @@ -26,6 +28,7 @@ describe('In-Memory P2P Client', () => { let p2pService: Mockify; let kvStore: AztecKVStore; let client: P2PClient; + const telemetryClient: TelemetryClient = new NoopTelemetryClient(); beforeEach(() => { txPool = { @@ -65,7 +68,16 @@ describe('In-Memory P2P Client', () => { blockSource = new MockBlockSource(); kvStore = openTmpStore(); - client = new P2PClient(kvStore, blockSource, txPool, attestationPool, epochProofQuotePool, p2pService, 0); + client = new P2PClient( + kvStore, + blockSource, + txPool, + attestationPool, + epochProofQuotePool, + p2pService, + 0, + telemetryClient, + ); }); const advanceToProvenBlock = async (getProvenBlockNumber: number, provenEpochNumber = getProvenBlockNumber) => { @@ -135,7 +147,16 @@ describe('In-Memory P2P Client', () => { await client.start(); await client.stop(); - const client2 = new P2PClient(kvStore, blockSource, txPool, attestationPool, epochProofQuotePool, p2pService, 0); + const client2 = new P2PClient( + kvStore, + blockSource, + txPool, + attestationPool, + epochProofQuotePool, + p2pService, + 0, + telemetryClient, + ); expect(client2.getSyncedLatestBlockNum()).toEqual(client.getSyncedLatestBlockNum()); }); @@ -150,7 +171,16 @@ describe('In-Memory P2P Client', () => { }); it('deletes txs after waiting the set number of blocks', async () => { - client = new P2PClient(kvStore, blockSource, txPool, attestationPool, epochProofQuotePool, p2pService, 10); + client = new P2PClient( + kvStore, + blockSource, + txPool, + attestationPool, + epochProofQuotePool, + p2pService, + 10, + telemetryClient, + ); blockSource.setProvenBlockNumber(0); await client.start(); expect(txPool.deleteTxs).not.toHaveBeenCalled(); @@ -167,7 +197,16 @@ describe('In-Memory P2P Client', () => { }); it('stores and returns epoch proof quotes', async () => { - client = new P2PClient(kvStore, blockSource, txPool, attestationPool, epochProofQuotePool, p2pService, 0); + client = new P2PClient( + kvStore, + blockSource, + txPool, + attestationPool, + epochProofQuotePool, + p2pService, + 0, + telemetryClient, + ); blockSource.setProvenEpochNumber(2); await client.start(); @@ -198,7 +237,16 @@ describe('In-Memory P2P Client', () => { }); it('deletes expired proof quotes', async () => { - client = new P2PClient(kvStore, blockSource, txPool, attestationPool, epochProofQuotePool, p2pService, 0); + client = new P2PClient( + kvStore, + blockSource, + txPool, + attestationPool, + epochProofQuotePool, + p2pService, + 0, + telemetryClient, + ); blockSource.setProvenEpochNumber(1); blockSource.setProvenBlockNumber(1); From 34c8bde52a432793408d52f4b68e1474ae67b15f Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:15:53 +0000 Subject: [PATCH 07/11] chore: document with tracer and usage --- .../p2p/src/service/service_metrics.ts | 11 --------- .../telemetry-client/src/with_tracer.ts | 23 +++++++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) delete mode 100644 yarn-project/p2p/src/service/service_metrics.ts diff --git a/yarn-project/p2p/src/service/service_metrics.ts b/yarn-project/p2p/src/service/service_metrics.ts deleted file mode 100644 index 09147218deb1..000000000000 --- a/yarn-project/p2p/src/service/service_metrics.ts +++ /dev/null @@ -1,11 +0,0 @@ -// Goal -// - Emit something that is the source of a gossip message -// - Measure when we receive a message from a peer -// From this - and from looking at loki, we should be able to determine that amount of -// time that it takes for messages to make their way across the p2p network - -// - ReqResp -// - Store the time that it takes to send a message and to receive a response -// - but do not store the undefined messages - -export class LibP2PServiceMetrics {} diff --git a/yarn-project/telemetry-client/src/with_tracer.ts b/yarn-project/telemetry-client/src/with_tracer.ts index e23b2b5822ed..6033f1686ea1 100644 --- a/yarn-project/telemetry-client/src/with_tracer.ts +++ b/yarn-project/telemetry-client/src/with_tracer.ts @@ -3,6 +3,29 @@ import { type TelemetryClient, type Tracer } from './telemetry.js'; /** * A minimal class that enables recording metrics with a telemetry client. * This base class enables tracing + * + * In other words: + * Enables the ability to call `@trackSpan` on methods. + * + * Example: + * + * ``` + * import {Attributes, type TelemetryClient, WithTracer, trackSpan } from '@aztec/telemetry-client'; + * + * class MyClass extends WithTracer { + * // Constructor is required to be passed the TelemetryClient implementation. + * constructor(client: TelemetryClient) { + * super(client, 'MyClass'); + * } + * + * @trackSpan('MyClass.myMethod', (arg: string) => ({ + * [Attributes.ARG]: arg, + * })) + * public myMethod(arg: string) { + * // ... + * } + * } + * ``` */ export class WithTracer { public readonly tracer: Tracer; From 6eb948c8708d066e67bcf7b4d2d2b7d8ec58cfff Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:56:12 +0000 Subject: [PATCH 08/11] fmt 3: beats me --- yarn-project/p2p/src/client/p2p_client.test.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/yarn-project/p2p/src/client/p2p_client.test.ts b/yarn-project/p2p/src/client/p2p_client.test.ts index 78dcf5f6a0bb..dae29f255056 100644 --- a/yarn-project/p2p/src/client/p2p_client.test.ts +++ b/yarn-project/p2p/src/client/p2p_client.test.ts @@ -146,14 +146,7 @@ describe('In-Memory P2P Client', () => { await client.start(); await client.stop(); - const client2 = new P2PClient( - kvStore, - blockSource, - mempools, - p2pService, - 0, - telemetryClient, - ); + const client2 = new P2PClient(kvStore, blockSource, mempools, p2pService, 0, telemetryClient); expect(client2.getSyncedLatestBlockNum()).toEqual(client.getSyncedLatestBlockNum()); }); From 663bae02d2415d1fdde2ee21fa0f17844d374246 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:00:32 +0000 Subject: [PATCH 09/11] chore: update yarn prepare --- yarn-project/validator-client/tsconfig.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/yarn-project/validator-client/tsconfig.json b/yarn-project/validator-client/tsconfig.json index a4198a7f0c2f..17533523097e 100644 --- a/yarn-project/validator-client/tsconfig.json +++ b/yarn-project/validator-client/tsconfig.json @@ -21,6 +21,9 @@ { "path": "../p2p" }, + { + "path": "../telemetry-client" + }, { "path": "../types" } From 32762de24392e7a675feae6d3a5f0d268cb46e17 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:16:48 +0000 Subject: [PATCH 10/11] fix: use archive getter rather than payload identifier name --- .../circuit-types/src/p2p/block_attestation.ts | 7 ++++--- .../circuit-types/src/p2p/block_proposal.ts | 7 ++++++- yarn-project/p2p/src/client/p2p_client.ts | 2 +- .../memory_attestation_pool.test.ts | 18 ++++++++---------- .../memory_attestation_pool.ts | 4 ++-- yarn-project/p2p/src/service/libp2p_service.ts | 5 +++-- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index d01dddbb8ae1..2b15c746bbd6 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -1,8 +1,9 @@ -import { type EthAddress } from '@aztec/circuits.js'; +import { type EthAddress } from '@aztec/foundation/eth-address'; import { Buffer32 } from '@aztec/foundation/buffer'; import { keccak256, recoverAddress } from '@aztec/foundation/crypto'; import { Signature } from '@aztec/foundation/eth-signature'; import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; +import { type Fr } from '@aztec/foundation/fields'; import { ConsensusPayload } from './consensus_payload.js'; import { Gossipable } from './gossipable.js'; @@ -40,8 +41,8 @@ export class BlockAttestation extends Gossipable { return new BlockAttestationHash(keccak256(this.signature.toBuffer())); } - public proposalP2PMessageIdentifier(): Buffer32 { - return Buffer32.fromField(this.payload.archive); + get archive(): Fr { + return this.payload.archive; } /**Get sender diff --git a/yarn-project/circuit-types/src/p2p/block_proposal.ts b/yarn-project/circuit-types/src/p2p/block_proposal.ts index 80cbb80c596d..f6561297292f 100644 --- a/yarn-project/circuit-types/src/p2p/block_proposal.ts +++ b/yarn-project/circuit-types/src/p2p/block_proposal.ts @@ -1,4 +1,5 @@ -import { type EthAddress } from '@aztec/circuits.js'; +import { type EthAddress } from '@aztec/foundation/eth-address'; +import { type Fr } from '@aztec/foundation/fields'; import { Buffer32 } from '@aztec/foundation/buffer'; import { recoverAddress } from '@aztec/foundation/crypto'; import { Signature } from '@aztec/foundation/eth-signature'; @@ -40,6 +41,10 @@ export class BlockProposal extends Gossipable { return BlockProposalHash.fromField(this.payload.archive); } + get archive(): Fr { + return this.payload.archive; + } + static async createProposalFromSigner( payload: ConsensusPayload, payloadSigner: (payload: Buffer32) => Promise, diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index d071e397cd04..f4fee0b8abc3 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -325,7 +325,7 @@ export class P2PClient extends WithTracer implements P2P { @trackSpan('p2pClient.broadcastProposal', proposal => ({ [Attributes.BLOCK_NUMBER]: proposal.payload.header.globalVariables.blockNumber.toNumber(), [Attributes.SLOT_NUMBER]: proposal.payload.header.globalVariables.slotNumber.toNumber(), - [Attributes.BLOCK_ARCHIVE]: proposal.payload.archive.toString(), + [Attributes.BLOCK_ARCHIVE]: proposal.archive.toString(), [Attributes.P2P_ID]: proposal.p2pMessageIdentifier().toString(), })) public broadcastProposal(proposal: BlockProposal): void { 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 172c390e0163..d948a13c4b58 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 @@ -21,11 +21,9 @@ describe('MemoryAttestationPool', () => { const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); - await ap.addAttestations(attestations); - const retreivedAttestations = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); + const retreivedAttestations = await ap.getAttestationsForSlot(BigInt(slotNumber), archive.toString()); expect(retreivedAttestations.length).toBe(NUMBER_OF_SIGNERS_PER_TEST); expect(retreivedAttestations).toEqual(attestations); @@ -33,7 +31,7 @@ describe('MemoryAttestationPool', () => { // Delete by slot await ap.deleteAttestationsForSlot(BigInt(slotNumber)); - const retreivedAttestationsAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), proposalId); + const retreivedAttestationsAfterDelete = await ap.getAttestationsForSlot(BigInt(slotNumber), archive.toString()); expect(retreivedAttestationsAfterDelete.length).toBe(0); }); @@ -45,9 +43,9 @@ describe('MemoryAttestationPool', () => { for (const attestation of attestations) { const slot = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.proposalP2PMessageIdentifier().toString(); + const archive = attestation.archive.toString(); - const retreivedAttestations = await ap.getAttestationsForSlot(slot.toBigInt(), proposalId); + const retreivedAttestations = await ap.getAttestationsForSlot(slot.toBigInt(), archive); expect(retreivedAttestations.length).toBe(1); expect(retreivedAttestations[0]).toEqual(attestation); expect(retreivedAttestations[0].payload.header.globalVariables.slotNumber).toEqual(slot); @@ -65,7 +63,7 @@ describe('MemoryAttestationPool', () => { for (const attestation of attestations) { const slot = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.proposalP2PMessageIdentifier().toString(); + const proposalId = attestation.archive.toString(); const retreivedAttestations = await ap.getAttestationsForSlot(slot.toBigInt(), proposalId); expect(retreivedAttestations.length).toBe(1); @@ -78,7 +76,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); + const proposalId = attestations[0].archive.toString(); await ap.addAttestations(attestations); @@ -96,7 +94,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); + const proposalId = attestations[0].archive.toString(); await ap.addAttestations(attestations); @@ -114,7 +112,7 @@ describe('MemoryAttestationPool', () => { const slotNumber = 420; const archive = Fr.random(); const attestations = await Promise.all(signers.map(signer => mockAttestation(signer, slotNumber, archive))); - const proposalId = attestations[0].proposalP2PMessageIdentifier().toString(); + const proposalId = attestations[0].archive.toString(); await ap.addAttestations(attestations); 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 2dacc65f8a46..5c163cd83f4b 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 @@ -26,7 +26,7 @@ export class InMemoryAttestationPool implements AttestationPool { // Perf: order and group by slot before insertion const slotNumber = attestation.payload.header.globalVariables.slotNumber; - const proposalId = attestation.proposalP2PMessageIdentifier().toString(); + const proposalId = attestation.archive.toString(); const address = attestation.getSender(); const slotAttestationMap = getSlotOrDefault(this.attestations, slotNumber.toBigInt()); @@ -59,7 +59,7 @@ export class InMemoryAttestationPool implements AttestationPool { const slotNumber = attestation.payload.header.globalVariables.slotNumber; const slotAttestationMap = this.attestations.get(slotNumber.toBigInt()); if (slotAttestationMap) { - const proposalId = attestation.proposalP2PMessageIdentifier().toString(); + const proposalId = attestation.archive.toString(); const proposalAttestationMap = getProposalOrDefault(slotAttestationMap, proposalId); if (proposalAttestationMap) { const address = attestation.getSender(); diff --git a/yarn-project/p2p/src/service/libp2p_service.ts b/yarn-project/p2p/src/service/libp2p_service.ts index 2fccd3067014..350387ede526 100644 --- a/yarn-project/p2p/src/service/libp2p_service.ts +++ b/yarn-project/p2p/src/service/libp2p_service.ts @@ -392,6 +392,7 @@ export class LibP2PService extends WithTracer implements P2PService { @trackSpan('Libp2pService.processAttestationFromPeer', attestation => ({ [Attributes.BLOCK_NUMBER]: attestation.payload.header.globalVariables.blockNumber.toNumber(), [Attributes.SLOT_NUMBER]: attestation.payload.header.globalVariables.slotNumber.toNumber(), + [Attributes.BLOCK_ARCHIVE]: attestation.archive.toString(), [Attributes.P2P_ID]: attestation.p2pMessageIdentifier().toString(), })) private async processAttestationFromPeer(attestation: BlockAttestation): Promise { @@ -409,7 +410,7 @@ export class LibP2PService extends WithTracer implements P2PService { @trackSpan('Libp2pService.processBlockFromPeer', block => ({ [Attributes.BLOCK_NUMBER]: block.payload.header.globalVariables.blockNumber.toNumber(), [Attributes.SLOT_NUMBER]: block.payload.header.globalVariables.slotNumber.toNumber(), - [Attributes.BLOCK_ARCHIVE]: block.payload.archive.toString(), + [Attributes.BLOCK_ARCHIVE]: block.archive.toString(), [Attributes.P2P_ID]: block.p2pMessageIdentifier().toString(), })) private async processBlockFromPeer(block: BlockProposal): Promise { @@ -430,7 +431,7 @@ export class LibP2PService extends WithTracer implements P2PService { @trackSpan('Libp2pService.broadcastAttestation', attestation => ({ [Attributes.BLOCK_NUMBER]: attestation.payload.header.globalVariables.blockNumber.toNumber(), [Attributes.SLOT_NUMBER]: attestation.payload.header.globalVariables.slotNumber.toNumber(), - [Attributes.BLOCK_ARCHIVE]: attestation.payload.archive.toString(), + [Attributes.BLOCK_ARCHIVE]: attestation.archive.toString(), [Attributes.P2P_ID]: attestation.p2pMessageIdentifier().toString(), })) private broadcastAttestation(attestation: BlockAttestation): void { From aebe7b621908a695e36fd153fcf0b91fb6981c26 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:18:13 +0000 Subject: [PATCH 11/11] fmt --- yarn-project/circuit-types/src/p2p/block_attestation.ts | 4 ++-- yarn-project/circuit-types/src/p2p/block_proposal.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn-project/circuit-types/src/p2p/block_attestation.ts b/yarn-project/circuit-types/src/p2p/block_attestation.ts index 2b15c746bbd6..f7eaa5c97d14 100644 --- a/yarn-project/circuit-types/src/p2p/block_attestation.ts +++ b/yarn-project/circuit-types/src/p2p/block_attestation.ts @@ -1,9 +1,9 @@ -import { type EthAddress } from '@aztec/foundation/eth-address'; import { Buffer32 } from '@aztec/foundation/buffer'; import { keccak256, recoverAddress } from '@aztec/foundation/crypto'; +import { type EthAddress } from '@aztec/foundation/eth-address'; import { Signature } from '@aztec/foundation/eth-signature'; -import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { type Fr } from '@aztec/foundation/fields'; +import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { ConsensusPayload } from './consensus_payload.js'; import { Gossipable } from './gossipable.js'; diff --git a/yarn-project/circuit-types/src/p2p/block_proposal.ts b/yarn-project/circuit-types/src/p2p/block_proposal.ts index f6561297292f..7764b316b889 100644 --- a/yarn-project/circuit-types/src/p2p/block_proposal.ts +++ b/yarn-project/circuit-types/src/p2p/block_proposal.ts @@ -1,8 +1,8 @@ -import { type EthAddress } from '@aztec/foundation/eth-address'; -import { type Fr } from '@aztec/foundation/fields'; import { Buffer32 } from '@aztec/foundation/buffer'; import { recoverAddress } from '@aztec/foundation/crypto'; +import { type EthAddress } from '@aztec/foundation/eth-address'; import { Signature } from '@aztec/foundation/eth-signature'; +import { type Fr } from '@aztec/foundation/fields'; import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { ConsensusPayload } from './consensus_payload.js';