diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 5d8bc7bd69d8..92d557749def 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -380,22 +380,24 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable { await validatorClient.registerHandlers(); } } + } - // If there's no validator client but alwaysReexecuteBlockProposals is enabled, - // create a BlockProposalHandler to reexecute block proposals for monitoring - if (!validatorClient && config.alwaysReexecuteBlockProposals) { - log.info('Setting up block proposal reexecution for monitoring'); - createBlockProposalHandler(config, { - checkpointsBuilder: validatorCheckpointsBuilder, - worldState: worldStateSynchronizer, - epochCache, - blockSource: archiver, - l1ToL2MessageSource: archiver, - p2pClient, - dateProvider, - telemetry, - }).registerForReexecution(p2pClient); - } + // If there's no validator client, create a BlockProposalHandler to handle block proposals + // for monitoring or reexecution. Reexecution (default) allows us to follow the pending chain, + // while non-reexecution is used for validating the proposals and collecting their txs. + if (!validatorClient) { + const reexecute = !!config.alwaysReexecuteBlockProposals; + log.info(`Setting up block proposal handler` + (reexecute ? ' with reexecution of proposals' : '')); + createBlockProposalHandler(config, { + checkpointsBuilder: validatorCheckpointsBuilder, + worldState: worldStateSynchronizer, + epochCache, + blockSource: archiver, + l1ToL2MessageSource: archiver, + p2pClient, + dateProvider, + telemetry, + }).register(p2pClient, reexecute); } // Start world state and wait for it to sync to the archiver. diff --git a/yarn-project/p2p/src/client/p2p_client.ts b/yarn-project/p2p/src/client/p2p_client.ts index 7996594ff9cb..a6be4b34799a 100644 --- a/yarn-project/p2p/src/client/p2p_client.ts +++ b/yarn-project/p2p/src/client/p2p_client.ts @@ -18,7 +18,6 @@ import { type L2TipsStore, } from '@aztec/stdlib/block'; import type { ContractDataSource } from '@aztec/stdlib/contract'; -import { getTimestampForSlot } from '@aztec/stdlib/epoch-helpers'; import { type PeerInfo, tryStop } from '@aztec/stdlib/interfaces/server'; import { type BlockProposal, CheckpointAttestation, type CheckpointProposal, type TopicType } from '@aztec/stdlib/p2p'; import type { BlockHeader, Tx, TxHash } from '@aztec/stdlib/tx'; @@ -111,27 +110,6 @@ export class P2PClient extends WithTracer implements P2P { this.telemetry, ); - // Default to collecting all txs when we see a valid proposal - // This can be overridden by the validator client to validate, and it will call getTxsForBlockProposal on its own - // Note: Validators do NOT attest to individual blocks - attestations are only for checkpoint proposals. - // TODO(palla/txs): We should not trigger a request for txs on a proposal before fully validating it. We need to bring - // validator-client code into here so we can validate a proposal is reasonable. - this.registerBlockProposalHandler(async (block, sender) => { - this.log.debug(`Received block proposal from ${sender.toString()}`); - // TODO(palla/txs): Need to subtract validatorReexecuteDeadlineMs from this deadline (see ValidatorClient.getReexecutionDeadline) - const constants = this.txCollection.getConstants(); - const nextSlotTimestampSeconds = Number(getTimestampForSlot(SlotNumber(block.slotNumber + 1), constants)); - const deadline = new Date(nextSlotTimestampSeconds * 1000); - const parentBlock = await this.l2BlockSource.getBlockHeaderByArchive(block.blockHeader.lastArchive.root); - if (!parentBlock) { - this.log.debug(`Cannot collect txs for proposal as parent block not found`); - return false; - } - const blockNumber = BlockNumber(parentBlock.getBlockNumber() + 1); - await this.txProvider.getTxsForBlockProposal(block, blockNumber, { pinnedPeer: sender, deadline }); - return true; - }); - this.l2Tips = new L2TipsKVStore(store, 'p2p_client'); this.synchedLatestSlot = store.openSingleton('p2p_pool_last_l2_slot'); } diff --git a/yarn-project/validator-client/src/block_proposal_handler.ts b/yarn-project/validator-client/src/block_proposal_handler.ts index 0c8812aee9a8..3aac5ec77b25 100644 --- a/yarn-project/validator-client/src/block_proposal_handler.ts +++ b/yarn-project/validator-client/src/block_proposal_handler.ts @@ -1,6 +1,7 @@ import { INITIAL_L2_BLOCK_NUM } from '@aztec/constants'; import type { EpochCache } from '@aztec/epoch-cache'; import { BlockNumber, CheckpointNumber, SlotNumber } from '@aztec/foundation/branded-types'; +import { pick } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/curves/bn254'; import { TimeoutError } from '@aztec/foundation/error'; import { createLogger } from '@aztec/foundation/log'; @@ -87,25 +88,28 @@ export class BlockProposalHandler { this.tracer = telemetry.getTracer('BlockProposalHandler'); } - registerForReexecution(p2pClient: P2P): BlockProposalHandler { - // Non-validator handler that re-executes for monitoring but does not attest. + register(p2pClient: P2P, shouldReexecute: boolean): BlockProposalHandler { + // Non-validator handler that processes or re-executes for monitoring but does not attest. // Returns boolean indicating whether the proposal was valid. const handler = async (proposal: BlockProposal, proposalSender: PeerId): Promise => { try { - const result = await this.handleBlockProposal(proposal, proposalSender, true); + const { slotNumber, blockNumber } = proposal; + const result = await this.handleBlockProposal(proposal, proposalSender, shouldReexecute); if (result.isValid) { - this.log.info(`Non-validator reexecution completed for slot ${proposal.slotNumber}`, { + this.log.info(`Non-validator block proposal ${blockNumber} at slot ${slotNumber} handled`, { blockNumber: result.blockNumber, + slotNumber, reexecutionTimeMs: result.reexecutionResult?.reexecutionTimeMs, totalManaUsed: result.reexecutionResult?.totalManaUsed, numTxs: result.reexecutionResult?.block?.body?.txEffects?.length ?? 0, + reexecuted: shouldReexecute, }); return true; } else { - this.log.warn(`Non-validator reexecution failed for slot ${proposal.slotNumber}`, { - blockNumber: result.blockNumber, - reason: result.reason, - }); + this.log.warn( + `Non-validator block proposal ${blockNumber} at slot ${slotNumber} failed processing with ${result.reason}`, + { blockNumber: result.blockNumber, slotNumber, reason: result.reason }, + ); return false; } } catch (error) { @@ -184,6 +188,15 @@ export class BlockProposalHandler { deadline: this.getReexecutionDeadline(slotNumber, config), }); + // If reexecution is disabled, bail. We are just interested in triggering tx collection. + if (!shouldReexecute) { + this.log.info( + `Received valid block ${blockNumber} proposal at index ${proposal.indexWithinCheckpoint} on slot ${slotNumber}`, + proposalInfo, + ); + return { isValid: true, blockNumber }; + } + // Compute the checkpoint number for this block and validate checkpoint consistency const checkpointResult = this.computeCheckpointNumber(proposal, parentBlock, proposalInfo); if (checkpointResult.reason) { @@ -210,30 +223,28 @@ export class BlockProposalHandler { return { isValid: false, blockNumber, reason: 'txs_not_available' }; } + // Collect the out hashes of all the checkpoints before this one in the same epoch + const epoch = getEpochAtSlot(slotNumber, this.epochCache.getL1Constants()); + const previousCheckpointOutHashes = (await this.blockSource.getCheckpointsDataForEpoch(epoch)) + .filter(c => c.checkpointNumber < checkpointNumber) + .map(c => c.checkpointOutHash); + // Try re-executing the transactions in the proposal if needed let reexecutionResult; - if (shouldReexecute) { - // Collect the out hashes of all the checkpoints before this one in the same epoch - const epoch = getEpochAtSlot(slotNumber, this.epochCache.getL1Constants()); - const previousCheckpointOutHashes = (await this.blockSource.getCheckpointsDataForEpoch(epoch)) - .filter(c => c.checkpointNumber < checkpointNumber) - .map(c => c.checkpointOutHash); - - try { - this.log.verbose(`Re-executing transactions in the proposal`, proposalInfo); - reexecutionResult = await this.reexecuteTransactions( - proposal, - blockNumber, - checkpointNumber, - txs, - l1ToL2Messages, - previousCheckpointOutHashes, - ); - } catch (error) { - this.log.error(`Error reexecuting txs while processing block proposal`, error, proposalInfo); - const reason = this.getReexecuteFailureReason(error); - return { isValid: false, blockNumber, reason, reexecutionResult }; - } + try { + this.log.verbose(`Re-executing transactions in the proposal`, proposalInfo); + reexecutionResult = await this.reexecuteTransactions( + proposal, + blockNumber, + checkpointNumber, + txs, + l1ToL2Messages, + previousCheckpointOutHashes, + ); + } catch (error) { + this.log.error(`Error reexecuting txs while processing block proposal`, error, proposalInfo); + const reason = this.getReexecuteFailureReason(error); + return { isValid: false, blockNumber, reason, reexecutionResult }; } // If we succeeded, push this block into the archiver (unless disabled) @@ -242,8 +253,8 @@ export class BlockProposalHandler { } this.log.info( - `Successfully processed block ${blockNumber} proposal at index ${proposal.indexWithinCheckpoint} on slot ${slotNumber}`, - proposalInfo, + `Successfully re-executed block ${blockNumber} proposal at index ${proposal.indexWithinCheckpoint} on slot ${slotNumber}`, + { ...proposalInfo, ...pick(reexecutionResult, 'reexecutionTimeMs', 'totalManaUsed') }, ); return { isValid: true, blockNumber, reexecutionResult }; @@ -488,10 +499,11 @@ export class BlockProposalHandler { const { block, failedTxs } = result; const numFailedTxs = failedTxs.length; - this.log.verbose(`Transaction re-execution complete for slot ${slot}`, { + this.log.verbose(`Block proposal ${blockNumber} at slot ${slot} transaction re-execution complete`, { numFailedTxs, numProposalTxs: txHashes.length, numProcessedTxs: block.body.txEffects.length, + blockNumber, slot, }); diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 7d9c4b975288..da7c38bf6d70 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -618,6 +618,7 @@ describe('ValidatorClient', () => { }); it('should return false if the transactions are not available', async () => { + enableReexecution(); txProvider.getTxsForBlockProposal.mockImplementation(proposal => Promise.resolve({ txs: [], @@ -694,6 +695,7 @@ describe('ValidatorClient', () => { // L1 messages for the checkpoint) will catch it. it('should return false if global variables do not match parent for non-first block in checkpoint', async () => { + enableReexecution(); // Create a proposal with indexWithinCheckpoint > 0 (non-first block in checkpoint) const parentSlotNumber = 100; const parentBlockNumber = 10;