From 2e58f0a315ec037a212d7f33b8c73b1b0c30a2e2 Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Sat, 23 Nov 2024 02:35:38 +0800 Subject: [PATCH] feat(val): reex (#9768) --- scripts/ci/get_e2e_jobs.sh | 1 - .../aztec-network/templates/validator.yaml | 2 + spartan/aztec-network/values.yaml | 1 + .../src/structs/public_data_update_request.ts | 10 ++ .../end-to-end/scripts/e2e_test_config.yml | 2 + .../src/e2e_p2p/gossip_network.test.ts | 2 + .../end-to-end/src/e2e_p2p/p2p_network.ts | 52 ++++++- .../end-to-end/src/e2e_p2p/reex.test.ts | 130 ++++++++++++++++ yarn-project/end-to-end/src/e2e_p2p/shared.ts | 27 +++- .../end-to-end/src/e2e_synching.test.ts | 1 - .../src/fixtures/snapshot_manager.ts | 2 +- yarn-project/foundation/src/config/env_var.ts | 1 + .../orchestrator/block-building-helpers.ts | 1 + .../src/block_builder/light.ts | 2 +- yarn-project/sequencer-client/src/index.ts | 1 + .../src/sequencer/sequencer.ts | 142 ++++++++++++------ .../telemetry-client/src/attributes.ts | 2 + yarn-project/telemetry-client/src/metrics.ts | 3 + yarn-project/validator-client/src/config.ts | 8 + .../src/errors/validator.error.ts | 18 +++ yarn-project/validator-client/src/factory.ts | 2 +- yarn-project/validator-client/src/metrics.ts | 50 ++++++ .../validator-client/src/validator.test.ts | 26 +++- .../validator-client/src/validator.ts | 119 ++++++++++++--- 24 files changed, 525 insertions(+), 80 deletions(-) create mode 100644 yarn-project/end-to-end/src/e2e_p2p/reex.test.ts create mode 100644 yarn-project/validator-client/src/metrics.ts diff --git a/scripts/ci/get_e2e_jobs.sh b/scripts/ci/get_e2e_jobs.sh index 6671e546e20..2dbdb42ac40 100755 --- a/scripts/ci/get_e2e_jobs.sh +++ b/scripts/ci/get_e2e_jobs.sh @@ -69,7 +69,6 @@ done # Add the input labels and expanded matches to allow_list allow_list+=("${input_labels[@]}" "${expanded_allow_list[@]}") - # Generate full list of targets, excluding specific entries, on one line test_list=$(echo "${full_list[@]}" | grep -v 'base' | grep -v 'bench' | grep -v "network" | grep -v 'devnet' | xargs echo) diff --git a/spartan/aztec-network/templates/validator.yaml b/spartan/aztec-network/templates/validator.yaml index f5a2fb8ce54..6f8aba191b2 100644 --- a/spartan/aztec-network/templates/validator.yaml +++ b/spartan/aztec-network/templates/validator.yaml @@ -151,6 +151,8 @@ spec: value: "{{ .Values.validator.p2p.enabled }}" - name: VALIDATOR_DISABLED value: "{{ .Values.validator.validator.disabled }}" + - name: VALIDATOR_REEXECUTE + value: "{{ .Values.validator.validator.reexecute }}" - name: SEQ_MAX_SECONDS_BETWEEN_BLOCKS value: "{{ .Values.validator.sequencer.maxSecondsBetweenBlocks }}" - name: SEQ_MIN_TX_PER_BLOCK diff --git a/spartan/aztec-network/values.yaml b/spartan/aztec-network/values.yaml index 2bdf0b6458a..0be51cd0d26 100644 --- a/spartan/aztec-network/values.yaml +++ b/spartan/aztec-network/values.yaml @@ -94,6 +94,7 @@ validator: enforceTimeTable: true validator: disabled: false + reexecute: true p2p: enabled: "true" startupProbe: diff --git a/yarn-project/circuits.js/src/structs/public_data_update_request.ts b/yarn-project/circuits.js/src/structs/public_data_update_request.ts index 35aa6a4a9a2..ab1973fb081 100644 --- a/yarn-project/circuits.js/src/structs/public_data_update_request.ts +++ b/yarn-project/circuits.js/src/structs/public_data_update_request.ts @@ -1,8 +1,12 @@ +import { type AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; import { BufferReader, FieldReader, serializeToBuffer } from '@aztec/foundation/serialize'; import { inspect } from 'util'; +import { computePublicDataTreeLeafSlot } from '../hash/hash.js'; +import { type ContractStorageUpdateRequest } from './contract_storage_update_request.js'; + // TO BE REMOVED. /** * Write operations on the public data tree including the previous value. @@ -75,6 +79,12 @@ export class PublicDataUpdateRequest { return new PublicDataUpdateRequest(Fr.fromBuffer(reader), Fr.fromBuffer(reader), reader.readNumber()); } + static fromContractStorageUpdateRequest(contractAddress: AztecAddress, updateRequest: ContractStorageUpdateRequest) { + const leafSlot = computePublicDataTreeLeafSlot(contractAddress, updateRequest.storageSlot); + + return new PublicDataUpdateRequest(leafSlot, updateRequest.newValue, updateRequest.counter); + } + static empty() { return new PublicDataUpdateRequest(Fr.ZERO, Fr.ZERO, 0); } diff --git a/yarn-project/end-to-end/scripts/e2e_test_config.yml b/yarn-project/end-to-end/scripts/e2e_test_config.yml index fb59dacee9c..ffee94e2933 100644 --- a/yarn-project/end-to-end/scripts/e2e_test_config.yml +++ b/yarn-project/end-to-end/scripts/e2e_test_config.yml @@ -90,6 +90,8 @@ tests: test_path: 'e2e_p2p/rediscovery.test.ts' e2e_p2p_reqresp: test_path: 'e2e_p2p/reqresp.test.ts' + e2e_p2p_reex: + test_path: 'e2e_p2p/reex.test.ts' flakey_e2e_tests: test_path: './src/flakey' ignore_failures: true diff --git a/yarn-project/end-to-end/src/e2e_p2p/gossip_network.test.ts b/yarn-project/end-to-end/src/e2e_p2p/gossip_network.test.ts index b089dc94dad..de2c603e404 100644 --- a/yarn-project/end-to-end/src/e2e_p2p/gossip_network.test.ts +++ b/yarn-project/end-to-end/src/e2e_p2p/gossip_network.test.ts @@ -45,6 +45,8 @@ describe('e2e_p2p_network', () => { throw new Error('Bootstrap node ENR is not available'); } + t.ctx.aztecNodeConfig.validatorReexecute = true; + // create our network of nodes and submit txs into each of them // the number of txs per node and the number of txs per rollup // should be set so that the only way for rollups to be built diff --git a/yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts b/yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts index 1ec2f200360..95d263156e6 100644 --- a/yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts +++ b/yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts @@ -1,9 +1,11 @@ +import { getSchnorrAccount } from '@aztec/accounts/schnorr'; import { type AztecNodeConfig, type AztecNodeService } from '@aztec/aztec-node'; -import { EthCheatCodes } from '@aztec/aztec.js'; +import { type AccountWalletWithSecretKey, EthCheatCodes } from '@aztec/aztec.js'; import { EthAddress } from '@aztec/circuits.js'; import { getL1ContractsConfigEnvVars } from '@aztec/ethereum'; import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log'; import { RollupAbi } from '@aztec/l1-artifacts'; +import { SpamContract } from '@aztec/noir-contracts.js'; import { type BootstrapNode } from '@aztec/p2p'; import { createBootstrapNodeFromPrivateKey } from '@aztec/p2p/mocks'; @@ -17,7 +19,12 @@ import { generateNodePrivateKeys, generatePeerIdPrivateKeys, } from '../fixtures/setup_p2p_test.js'; -import { type ISnapshotManager, type SubsystemsContext, createSnapshotManager } from '../fixtures/snapshot_manager.js'; +import { + type ISnapshotManager, + type SubsystemsContext, + addAccounts, + createSnapshotManager, +} from '../fixtures/snapshot_manager.js'; import { getPrivateKeyFromIndex } from '../fixtures/utils.js'; import { getEndToEndTestTelemetryClient } from '../fixtures/with_telemetry_utils.js'; @@ -39,6 +46,10 @@ export class P2PNetworkTest { public bootstrapNodeEnr: string = ''; + // The re-execution test needs a wallet and a spam contract + public wallet?: AccountWalletWithSecretKey; + public spamContract?: SpamContract; + constructor( testName: string, public bootstrapNode: BootstrapNode, @@ -108,12 +119,16 @@ export class P2PNetworkTest { client: deployL1ContractsValues.walletClient, }); + this.logger.verbose(`Adding ${this.numberOfNodes} validators`); + const txHashes: `0x${string}`[] = []; for (let i = 0; i < this.numberOfNodes; i++) { const account = privateKeyToAccount(this.nodePrivateKeys[i]!); this.logger.debug(`Adding ${account.address} as validator`); const txHash = await rollup.write.addValidator([account.address]); txHashes.push(txHash); + + this.logger.debug(`Adding ${account.address} as validator`); } // Wait for all the transactions adding validators to be mined @@ -148,6 +163,39 @@ export class P2PNetworkTest { }); } + async setupAccount() { + await this.snapshotManager.snapshot( + 'setup-account', + addAccounts(1, this.logger, false), + async ({ accountKeys }, ctx) => { + const accountManagers = accountKeys.map(ak => getSchnorrAccount(ctx.pxe, ak[0], ak[1], 1)); + await Promise.all(accountManagers.map(a => a.register())); + const wallets = await Promise.all(accountManagers.map(a => a.getWallet())); + this.wallet = wallets[0]; + }, + ); + } + + async deploySpamContract() { + await this.snapshotManager.snapshot( + 'add-spam-contract', + async () => { + if (!this.wallet) { + throw new Error('Call snapshot t.setupAccount before deploying account contract'); + } + + const spamContract = await SpamContract.deploy(this.wallet).send().deployed(); + return { contractAddress: spamContract.address }; + }, + async ({ contractAddress }) => { + if (!this.wallet) { + throw new Error('Call snapshot t.setupAccount before deploying account contract'); + } + this.spamContract = await SpamContract.at(contractAddress, this.wallet); + }, + ); + } + async removeInitialNode() { await this.snapshotManager.snapshot( 'remove-inital-validator', diff --git a/yarn-project/end-to-end/src/e2e_p2p/reex.test.ts b/yarn-project/end-to-end/src/e2e_p2p/reex.test.ts new file mode 100644 index 00000000000..631f2910596 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_p2p/reex.test.ts @@ -0,0 +1,130 @@ +import { type AztecNodeService } from '@aztec/aztec-node'; +import { type SentTx, sleep } from '@aztec/aztec.js'; + +/* eslint-disable-next-line no-restricted-imports */ +import { BlockProposal, SignatureDomainSeperator, getHashedSignaturePayload } from '@aztec/circuit-types'; + +import { beforeAll, describe, it, jest } from '@jest/globals'; +import fs from 'fs'; + +import { createNodes } from '../fixtures/setup_p2p_test.js'; +import { P2PNetworkTest } from './p2p_network.js'; +import { submitComplexTxsTo } from './shared.js'; + +const NUM_NODES = 4; +const NUM_TXS_PER_NODE = 1; +const BOOT_NODE_UDP_PORT = 41000; + +const DATA_DIR = './data/re-ex'; + +describe('e2e_p2p_reex', () => { + let t: P2PNetworkTest; + let nodes: AztecNodeService[]; + + beforeAll(async () => { + nodes = []; + + t = await P2PNetworkTest.create({ + testName: 'e2e_p2p_reex', + numberOfNodes: NUM_NODES, + basePort: BOOT_NODE_UDP_PORT, + }); + + t.logger.verbose('Setup account'); + await t.setupAccount(); + + t.logger.verbose('Deploy spam contract'); + await t.deploySpamContract(); + + t.logger.verbose('Apply base snapshots'); + await t.applyBaseSnapshots(); + + t.logger.verbose('Setup nodes'); + await t.setup(); + }); + + afterAll(async () => { + // shutdown all nodes. + await t.stopNodes(nodes); + await t.teardown(); + for (let i = 0; i < NUM_NODES; i++) { + fs.rmSync(`${DATA_DIR}-${i}`, { recursive: true, force: true }); + } + }); + + it('validators should re-execute transactions before attesting', async () => { + // create the bootstrap node for the network + if (!t.bootstrapNodeEnr) { + throw new Error('Bootstrap node ENR is not available'); + } + + t.ctx.aztecNodeConfig.validatorReexecute = true; + + nodes = await createNodes( + t.ctx.aztecNodeConfig, + t.peerIdPrivateKeys, + t.bootstrapNodeEnr, + NUM_NODES, + BOOT_NODE_UDP_PORT, + ); + + // Hook into the node and intercept re-execution logic, ensuring that it was infact called + const reExecutionSpies = []; + for (const node of nodes) { + // Make sure the nodes submit faulty proposals, in this case a faulty proposal is one where we remove one of the transactions + // Such that the calculated archive will be different! + jest.spyOn((node as any).p2pClient, 'broadcastProposal').mockImplementation(async (...args: unknown[]) => { + // We remove one of the transactions, therefore the block root will be different! + const proposal = args[0] as BlockProposal; + const { txHashes } = proposal.payload; + + // We need to mutate the proposal, so we cast to any + (proposal.payload as any).txHashes = txHashes.slice(0, txHashes.length - 1); + + // We sign over the proposal using the node's signing key + // Abusing javascript to access the nodes signing key + const signer = (node as any).sequencer.sequencer.validatorClient.validationService.keyStore; + const newProposal = new BlockProposal( + proposal.payload, + await signer.signMessage(getHashedSignaturePayload(proposal.payload, SignatureDomainSeperator.blockProposal)), + ); + + return (node as any).p2pClient.p2pService.propagate(newProposal); + }); + + // Store re-execution spys node -> sequencer Client -> seqeuncer -> validator + const spy = jest.spyOn((node as any).sequencer.sequencer.validatorClient, 'reExecuteTransactions'); + reExecutionSpies.push(spy); + } + + // wait a bit for peers to discover each other + await sleep(4000); + + nodes.forEach(node => { + node.getSequencer()?.updateSequencerConfig({ + minTxsPerBlock: NUM_TXS_PER_NODE, + maxTxsPerBlock: NUM_TXS_PER_NODE, + }); + }); + const txs = await submitComplexTxsTo(t.logger, t.spamContract!, NUM_TXS_PER_NODE); + + // We ensure that the transactions are NOT mined + try { + await Promise.all( + txs.map(async (tx: SentTx, i: number) => { + t.logger.info(`Waiting for tx ${i}: ${await tx.getTxHash()} to be mined`); + return tx.wait(); + }), + ); + } catch (e) { + t.logger.info('Failed to mine all txs, as planned'); + } + + // Expect that all of the re-execution attempts failed with an invalid root + for (const spy of reExecutionSpies) { + for (const result of spy.mock.results) { + await expect(result.value).rejects.toThrow('Validator Error: Re-execution state mismatch'); + } + } + }); +}); diff --git a/yarn-project/end-to-end/src/e2e_p2p/shared.ts b/yarn-project/end-to-end/src/e2e_p2p/shared.ts index 9b787eaa564..d1c35dfdb66 100644 --- a/yarn-project/end-to-end/src/e2e_p2p/shared.ts +++ b/yarn-project/end-to-end/src/e2e_p2p/shared.ts @@ -1,12 +1,37 @@ import { getSchnorrAccount } from '@aztec/accounts/schnorr'; import { type AztecNodeService } from '@aztec/aztec-node'; -import { type DebugLogger } from '@aztec/aztec.js'; +import { type DebugLogger, type SentTx } from '@aztec/aztec.js'; import { CompleteAddress, TxStatus } from '@aztec/aztec.js'; import { Fr, GrumpkinScalar } from '@aztec/foundation/fields'; +import { type SpamContract } from '@aztec/noir-contracts.js'; import { type PXEService, createPXEService, getPXEServiceConfig as getRpcConfig } from '@aztec/pxe'; import { type NodeContext } from '../fixtures/setup_p2p_test.js'; +// submits a set of transactions to the provided Private eXecution Environment (PXE) +export const submitComplexTxsTo = async (logger: DebugLogger, spamContract: SpamContract, numTxs: number) => { + const txs: SentTx[] = []; + + const seed = 1234n; + const spamCount = 15; + for (let i = 0; i < numTxs; i++) { + const tx = spamContract.methods.spam(seed + BigInt(i * spamCount), spamCount, false).send(); + const txHash = await tx.getTxHash(); + + logger.info(`Tx sent with hash ${txHash}`); + const receipt = await tx.getReceipt(); + expect(receipt).toEqual( + expect.objectContaining({ + status: TxStatus.PENDING, + error: '', + }), + ); + logger.info(`Receipt received for ${txHash}`); + txs.push(tx); + } + return txs; +}; + // creates an instance of the PXE and submit a given number of transactions to it. export const createPXEServiceAndSubmitTransactions = async ( logger: DebugLogger, diff --git a/yarn-project/end-to-end/src/e2e_synching.test.ts b/yarn-project/end-to-end/src/e2e_synching.test.ts index 2a81a74257e..37c9237ab0c 100644 --- a/yarn-project/end-to-end/src/e2e_synching.test.ts +++ b/yarn-project/end-to-end/src/e2e_synching.test.ts @@ -11,7 +11,6 @@ * * To run the Setup run with the `AZTEC_GENERATE_TEST_DATA=1` flag. Without * this flag, we will run in execution. - * * There is functionality to store the `stats` of a sync, but currently we * will simply be writing it to the log instead. * diff --git a/yarn-project/end-to-end/src/fixtures/snapshot_manager.ts b/yarn-project/end-to-end/src/fixtures/snapshot_manager.ts index c92db7b1be2..3a85d0ff79b 100644 --- a/yarn-project/end-to-end/src/fixtures/snapshot_manager.ts +++ b/yarn-project/end-to-end/src/fixtures/snapshot_manager.ts @@ -260,6 +260,7 @@ async function setupFromFresh( opts: SetupOptions = {}, deployL1ContractsArgs: Partial = { assumeProvenThrough: Number.MAX_SAFE_INTEGER, + initialValidators: [], }, ): Promise { logger.verbose(`Initializing state...`); @@ -390,7 +391,6 @@ async function setupFromFresh( async function setupFromState(statePath: string, logger: Logger): Promise { logger.verbose(`Initializing with saved state at ${statePath}...`); - // Load config. // TODO: For some reason this is currently the union of a bunch of subsystems. That needs fixing. const aztecNodeConfig: AztecNodeConfig & SetupOptions = JSON.parse( readFileSync(`${statePath}/aztec_node_config.json`, 'utf-8'), diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index c114dafd698..4a2015ddd0f 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -144,6 +144,7 @@ export type EnvVar = | 'VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS' | 'VALIDATOR_DISABLED' | 'VALIDATOR_PRIVATE_KEY' + | 'VALIDATOR_REEXECUTE' | 'VERSION' | 'WS_BLOCK_CHECK_INTERVAL_MS' | 'WS_PROVEN_BLOCKS_ONLY' diff --git a/yarn-project/prover-client/src/orchestrator/block-building-helpers.ts b/yarn-project/prover-client/src/orchestrator/block-building-helpers.ts index a7377c2cd75..121d13a00d2 100644 --- a/yarn-project/prover-client/src/orchestrator/block-building-helpers.ts +++ b/yarn-project/prover-client/src/orchestrator/block-building-helpers.ts @@ -122,6 +122,7 @@ export async function buildBaseRollupHints( padArrayEnd(tx.txEffect.nullifiers, Fr.ZERO, MAX_NULLIFIERS_PER_TX).map(n => n.toBuffer()), NULLIFIER_SUBTREE_HEIGHT, ); + if (nullifierWitnessLeaves === undefined) { throw new Error(`Could not craft nullifier batch insertion proofs`); } diff --git a/yarn-project/sequencer-client/src/block_builder/light.ts b/yarn-project/sequencer-client/src/block_builder/light.ts index 90075c3f020..44d92ca2a23 100644 --- a/yarn-project/sequencer-client/src/block_builder/light.ts +++ b/yarn-project/sequencer-client/src/block_builder/light.ts @@ -32,7 +32,7 @@ export class LightweightBlockBuilder implements BlockBuilder { constructor(private db: MerkleTreeWriteOperations, private telemetry: TelemetryClient) {} async startNewBlock(numTxs: number, globalVariables: GlobalVariables, l1ToL2Messages: Fr[]): Promise { - this.logger.verbose('Starting new block', { numTxs, globalVariables, l1ToL2Messages }); + this.logger.verbose('Starting new block', { numTxs, globalVariables: globalVariables.toJSON(), l1ToL2Messages }); this.numTxs = numTxs; this.globalVariables = globalVariables; this.l1ToL2Messages = padArrayEnd(l1ToL2Messages, Fr.ZERO, NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP); diff --git a/yarn-project/sequencer-client/src/index.ts b/yarn-project/sequencer-client/src/index.ts index 66c24396853..1718ed0a3a6 100644 --- a/yarn-project/sequencer-client/src/index.ts +++ b/yarn-project/sequencer-client/src/index.ts @@ -4,4 +4,5 @@ export * from './publisher/index.js'; export * from './sequencer/index.js'; // Used by the node to simulate public parts of transactions. Should these be moved to a shared library? +// ISSUE(#9832) export * from './global_variable_builder/index.js'; diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 670b099a764..17e371c0738 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -15,6 +15,7 @@ import { AppendOnlyTreeSnapshot, ContentCommitment, GENESIS_ARCHIVE_ROOT, + type GlobalVariables, Header, StateReference, } from '@aztec/circuits.js'; @@ -112,6 +113,9 @@ export class Sequencer { this.updateConfig(config); this.metrics = new SequencerMetrics(telemetry, () => this.state, 'Sequencer'); this.log.verbose(`Initialized sequencer with ${this.minTxsPerBLock}-${this.maxTxsPerBlock} txs per block.`); + + // Register the block builder with the validator client for re-execution + this.validatorClient?.registerBlockBuilder(this.buildBlock.bind(this)); } get tracer(): Tracer { @@ -473,42 +477,21 @@ export class Sequencer { } /** - * @notice Build and propose a block to the chain + * Build a block * - * @dev MUST throw instead of exiting early to ensure that world-state - * is being rolled back if the block is dropped. + * Shared between the sequencer and the validator for re-execution * * @param validTxs - The valid transactions to construct the block from - * @param proposalHeader - The partial header constructed for the proposal + * @param newGlobalVariables - The global variables for the new block * @param historicalHeader - The historical header of the parent + * @param interrupt - The interrupt callback, used to validate the block for submission and check if we should propose the block */ - @trackSpan('Sequencer.buildBlockAndAttemptToPublish', (_validTxs, proposalHeader, _historicalHeader) => ({ - [Attributes.BLOCK_NUMBER]: proposalHeader.globalVariables.blockNumber.toNumber(), - })) - private async buildBlockAndAttemptToPublish( + private async buildBlock( validTxs: Tx[], - proposalHeader: Header, - historicalHeader: Header | undefined, - ): Promise { - await this.publisher.validateBlockForSubmission(proposalHeader); - - const newGlobalVariables = proposalHeader.globalVariables; - - this.metrics.recordNewBlock(newGlobalVariables.blockNumber.toNumber(), validTxs.length); - const workTimer = new Timer(); - const secondsIntoSlot = getSecondsIntoSlot( - this.l1GenesisTime, - this.aztecSlotDuration, - newGlobalVariables.slotNumber.toNumber(), - ); - this.setState(SequencerState.CREATING_BLOCK, secondsIntoSlot); - this.log.info( - `Building blockNumber=${newGlobalVariables.blockNumber.toNumber()} txCount=${ - validTxs.length - } slotNumber=${newGlobalVariables.slotNumber.toNumber()}`, - ); - - // Get l1 to l2 messages from the contract + newGlobalVariables: GlobalVariables, + historicalHeader?: Header, + interrupt?: (processedTxs: ProcessedTx[]) => Promise, + ) { this.log.debug('Requesting L1 to L2 messages from contract'); const l1ToL2Messages = await this.l1ToL2MessageSource.getL1ToL2Messages(newGlobalVariables.blockNumber.toBigInt()); this.log.verbose( @@ -518,11 +501,15 @@ export class Sequencer { const numRealTxs = validTxs.length; const blockSize = Math.max(2, numRealTxs); + // Sync to the previous block at least + await this.worldState.syncImmediate(newGlobalVariables.blockNumber.toNumber() - 1); + this.log.verbose(`Synced to previous block ${newGlobalVariables.blockNumber.toNumber() - 1}`); + // NB: separating the dbs because both should update the state const publicProcessorFork = await this.worldState.fork(); const orchestratorFork = await this.worldState.fork(); + try { - // We create a fresh processor each time to reset any cached state (eg storage writes) const processor = this.publicProcessorFactory.create(publicProcessorFork, historicalHeader, newGlobalVariables); const blockBuildingTimer = new Timer(); const blockBuilder = this.blockBuilderFactory.create(orchestratorFork); @@ -542,6 +529,62 @@ export class Sequencer { await this.p2pClient.deleteTxs(Tx.getHashes(failedTxData)); } + await interrupt?.(processedTxs); + + // All real transactions have been added, set the block as full and complete the proving. + const block = await blockBuilder.setBlockCompleted(); + + return { block, publicProcessorDuration, numProcessedTxs: processedTxs.length, blockBuildingTimer }; + } finally { + // We create a fresh processor each time to reset any cached state (eg storage writes) + await publicProcessorFork.close(); + await orchestratorFork.close(); + } + } + + /** + * @notice Build and propose a block to the chain + * + * @dev MUST throw instead of exiting early to ensure that world-state + * is being rolled back if the block is dropped. + * + * @param validTxs - The valid transactions to construct the block from + * @param proposalHeader - The partial header constructed for the proposal + * @param historicalHeader - The historical header of the parent + */ + @trackSpan('Sequencer.buildBlockAndAttemptToPublish', (_validTxs, proposalHeader, _historicalHeader) => ({ + [Attributes.BLOCK_NUMBER]: proposalHeader.globalVariables.blockNumber.toNumber(), + })) + private async buildBlockAndAttemptToPublish( + validTxs: Tx[], + proposalHeader: Header, + historicalHeader: Header | undefined, + ): Promise { + await this.publisher.validateBlockForSubmission(proposalHeader); + + const newGlobalVariables = proposalHeader.globalVariables; + + this.metrics.recordNewBlock(newGlobalVariables.blockNumber.toNumber(), validTxs.length); + const workTimer = new Timer(); + const secondsIntoSlot = getSecondsIntoSlot( + this.l1GenesisTime, + this.aztecSlotDuration, + newGlobalVariables.slotNumber.toNumber(), + ); + this.setState(SequencerState.CREATING_BLOCK, secondsIntoSlot); + this.log.info( + `Building blockNumber=${newGlobalVariables.blockNumber.toNumber()} txCount=${ + validTxs.length + } slotNumber=${newGlobalVariables.slotNumber.toNumber()}`, + ); + + /** + * BuildBlock is shared between the sequencer and the validator for re-execution + * We use the interrupt callback to validate the block for submission and check if we should propose the block + * + * If we fail, we throw an error in order to roll back + */ + const interrupt = async (processedTxs: ProcessedTx[]) => { await this.publisher.validateBlockForSubmission(proposalHeader); if ( @@ -553,9 +596,15 @@ export class Sequencer { // TODO: Roll back changes to world state throw new Error('Should not propose the block'); } + }; - // All real transactions have been added, set the block as full and complete the proving. - const block = await blockBuilder.setBlockCompleted(); + try { + const { block, publicProcessorDuration, numProcessedTxs, blockBuildingTimer } = await this.buildBlock( + validTxs, + newGlobalVariables, + historicalHeader, + interrupt, + ); // TODO(@PhilWindle) We should probably periodically check for things like another // block being published before ours instead of just waiting on our block @@ -592,21 +641,16 @@ export class Sequencer { const proofQuote = await this.createProofClaimForPreviousEpoch(newGlobalVariables.slotNumber.toBigInt()); this.log.info(proofQuote ? `Using proof quote ${inspect(proofQuote.payload)}` : 'No proof quote available'); - try { - await this.publishL2Block(block, attestations, txHashes, proofQuote); - this.metrics.recordPublishedBlock(workDuration); - this.log.info( - `Submitted rollup block ${block.number} with ${processedTxs.length} transactions duration=${Math.ceil( - workDuration, - )}ms (Submitter: ${this.publisher.getSenderAddress()})`, - ); - } catch (err) { - this.metrics.recordFailedBlock(); - throw err; - } - } finally { - await publicProcessorFork.close(); - await orchestratorFork.close(); + await this.publishL2Block(block, attestations, txHashes, proofQuote); + this.metrics.recordPublishedBlock(workDuration); + this.log.info( + `Submitted rollup block ${block.number} with ${numProcessedTxs} transactions duration=${Math.ceil( + workDuration, + )}ms (Submitter: ${this.publisher.getSenderAddress()})`, + ); + } catch (err) { + this.metrics.recordFailedBlock(); + throw err; } } @@ -626,7 +670,7 @@ export class Sequencer { this.log.debug(`Attesting committee length ${committee.length}`); if (committee.length === 0) { - this.log.debug(`Attesting committee length is 0, skipping`); + this.log.verbose(`Attesting committee length is 0, skipping`); return undefined; } diff --git a/yarn-project/telemetry-client/src/attributes.ts b/yarn-project/telemetry-client/src/attributes.ts index a1a2a21a550..32ce263917a 100644 --- a/yarn-project/telemetry-client/src/attributes.ts +++ b/yarn-project/telemetry-client/src/attributes.ts @@ -54,6 +54,8 @@ export const BLOCK_TXS_COUNT = 'aztec.block.txs_count'; export const BLOCK_SIZE = 'aztec.block.size'; /** How many blocks are included in this epoch */ export const EPOCH_SIZE = 'aztec.epoch.size'; +/** The proposer of a block */ +export const BLOCK_PROPOSER = 'aztec.block.proposer'; /** The epoch number */ export const EPOCH_NUMBER = 'aztec.epoch.number'; /** The tx hash */ diff --git a/yarn-project/telemetry-client/src/metrics.ts b/yarn-project/telemetry-client/src/metrics.ts index 32f2996487a..0d66d0cb8da 100644 --- a/yarn-project/telemetry-client/src/metrics.ts +++ b/yarn-project/telemetry-client/src/metrics.ts @@ -74,3 +74,6 @@ export const WORLD_STATE_MERKLE_TREE_SIZE = 'aztec.world_state.merkle_tree_size' export const WORLD_STATE_DB_SIZE = 'aztec.world_state.db_size'; export const PROOF_VERIFIER_COUNT = 'aztec.proof_verifier.count'; + +export const VALIDATOR_RE_EXECUTION_TIME = 'aztec.validator.re_execution_time'; +export const VALIDATOR_FAILED_REEXECUTION_COUNT = 'aztec.validator.failed_reexecution_count'; diff --git a/yarn-project/validator-client/src/config.ts b/yarn-project/validator-client/src/config.ts index 4adb17a82e6..075f6249920 100644 --- a/yarn-project/validator-client/src/config.ts +++ b/yarn-project/validator-client/src/config.ts @@ -22,6 +22,9 @@ export interface ValidatorClientConfig { /** Wait for attestations timeout */ attestationWaitTimeoutMs: number; + + /** Re-execute transactions before attesting */ + validatorReexecute: boolean; } export const validatorClientConfigMappings: ConfigMappingsType = { @@ -48,6 +51,11 @@ export const validatorClientConfigMappings: ConfigMappingsType void { + const start = performance.now(); + return () => { + const end = performance.now(); + this.recordReExecutionTime(end - start); + }; + } + + public recordReExecutionTime(time: number) { + this.reExecutionTime.record(time); + } + + public recordFailedReexecution(proposal: BlockProposal) { + this.failedReexecutionCounter.add(1, { + [Attributes.STATUS]: 'failed', + [Attributes.BLOCK_NUMBER]: proposal.payload.header.globalVariables.blockNumber.toString(), + [Attributes.BLOCK_PROPOSER]: proposal.getSender()?.toString(), + }); + } +} diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index ffce55a4d21..d60937f2fcc 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -1,7 +1,7 @@ /** * Validation logic unit tests */ -import { TxHash } from '@aztec/circuit-types'; +import { TxHash, mockTx } from '@aztec/circuit-types'; import { makeHeader } from '@aztec/circuits.js/testing'; import { Secp256k1Signer } from '@aztec/foundation/crypto'; import { EthAddress } from '@aztec/foundation/eth-address'; @@ -17,6 +17,7 @@ import { makeBlockAttestation, makeBlockProposal } from '../../circuit-types/src import { type ValidatorClientConfig } from './config.js'; import { AttestationTimeoutError, + BlockBuilderNotProvidedError, InvalidValidatorPrivateKeyError, TransactionsNotAvailableError, } from './errors/validator.error.js'; @@ -40,6 +41,7 @@ describe('ValidationService', () => { attestationPollingIntervalMs: 1000, attestationWaitTimeoutMs: 1000, disableValidator: false, + validatorReexecute: false, }; validatorClient = ValidatorClient.new(config, p2pClient, new NoopTelemetryClient()); }); @@ -51,6 +53,13 @@ describe('ValidationService', () => { ); }); + it('Should throw an error if re-execution is enabled but no block builder is provided', async () => { + config.validatorReexecute = true; + p2pClient.getTxByHash.mockImplementation(() => Promise.resolve(mockTx())); + const val = ValidatorClient.new(config, p2pClient); + await expect(val.reExecuteTransactions(makeBlockProposal())).rejects.toThrow(BlockBuilderNotProvidedError); + }); + it('Should create a valid block proposal', async () => { const header = makeHeader(); const archive = Fr.random(); @@ -83,6 +92,21 @@ describe('ValidationService', () => { ); }); + it('Should not return an attestation if re-execution fails', async () => { + const proposal = makeBlockProposal(); + + // mock the p2pClient.getTxStatus to return undefined for all transactions + p2pClient.getTxStatus.mockImplementation(() => undefined); + + const val = ValidatorClient.new(config, p2pClient, new NoopTelemetryClient()); + val.registerBlockBuilder(() => { + throw new Error('Failed to build block'); + }); + + const attestation = await val.attestToProposal(proposal); + expect(attestation).toBeUndefined(); + }); + it('Should collect attestations for a proposal', async () => { const signer = Secp256k1Signer.random(); const attestor1 = Secp256k1Signer.random(); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index bf8efbe13c5..7ced2639ab1 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -1,25 +1,50 @@ -import { type BlockAttestation, type BlockProposal, type TxHash } from '@aztec/circuit-types'; -import { type Header } from '@aztec/circuits.js'; +import { + type BlockAttestation, + type BlockProposal, + type L2Block, + type ProcessedTx, + type Tx, + type TxHash, +} from '@aztec/circuit-types'; +import { type GlobalVariables, type Header } from '@aztec/circuits.js'; import { Buffer32 } from '@aztec/foundation/buffer'; import { type Fr } from '@aztec/foundation/fields'; -import { attachedFixedDataToLogger, createDebugLogger } from '@aztec/foundation/log'; +import { createDebugLogger } from '@aztec/foundation/log'; import { sleep } from '@aztec/foundation/sleep'; +import { type Timer } from '@aztec/foundation/timer'; import { type P2P } from '@aztec/p2p'; import { type TelemetryClient, WithTracer } from '@aztec/telemetry-client'; +import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { type ValidatorClientConfig } from './config.js'; import { ValidationService } from './duties/validation_service.js'; import { AttestationTimeoutError, + BlockBuilderNotProvidedError, InvalidValidatorPrivateKeyError, + ReExStateMismatchError, TransactionsNotAvailableError, } 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'; + +/** + * Callback function for building a block + * + * We reuse the sequencer's block building functionality for re-execution + */ +type BlockBuilderCallback = ( + txs: Tx[], + globalVariables: GlobalVariables, + historicalHeader?: Header, + interrupt?: (processedTxs: ProcessedTx[]) => Promise, +) => Promise<{ block: L2Block; publicProcessorDuration: number; numProcessedTxs: number; blockBuildingTimer: Timer }>; export interface Validator { start(): Promise; registerBlockProposalHandler(): void; + registerBlockBuilder(blockBuilder: BlockBuilderCallback): void; // Block validation responsiblities createBlockProposal(header: Header, archive: Fr, txs: TxHash[]): Promise; @@ -29,30 +54,33 @@ export interface Validator { collectAttestations(proposal: BlockProposal, numberOfRequiredAttestations: number): Promise; } -/** Validator Client +/** + * Validator Client */ export class ValidatorClient extends WithTracer implements Validator { private validationService: ValidationService; + private metrics: ValidatorMetrics; + + // Callback registered to: sequencer.buildBlock + private blockBuilder?: BlockBuilderCallback = undefined; constructor( keyStore: ValidatorKeyStore, private p2pClient: P2P, - private attestationPollingIntervalMs: number, - private attestationWaitTimeoutMs: number, - telemetry: TelemetryClient, - private log = attachedFixedDataToLogger(createDebugLogger('aztec:validator'), { - validatorAddress: keyStore.getAddress().toString(), - }), + private config: ValidatorClientConfig, + telemetry: TelemetryClient = new NoopTelemetryClient(), + private log = createDebugLogger('aztec:validator'), ) { // Instantiate 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'); } - static new(config: ValidatorClientConfig, p2pClient: P2P, telemetry: TelemetryClient) { + static new(config: ValidatorClientConfig, p2pClient: P2P, telemetry: TelemetryClient = new NoopTelemetryClient()) { if (!config.validatorPrivateKey) { throw new InvalidValidatorPrivateKeyError(); } @@ -60,13 +88,7 @@ export class ValidatorClient extends WithTracer implements Validator { const privateKey = validatePrivateKey(config.validatorPrivateKey); const localKeyStore = new LocalKeyStore(privateKey); - const validator = new ValidatorClient( - localKeyStore, - p2pClient, - config.attestationPollingIntervalMs, - config.attestationWaitTimeoutMs, - telemetry, - ); + const validator = new ValidatorClient(localKeyStore, p2pClient, config, telemetry); validator.registerBlockProposalHandler(); return validator; } @@ -86,17 +108,34 @@ export class ValidatorClient extends WithTracer implements Validator { this.p2pClient.registerBlockProposalHandler(handler); } + /** + * Register a callback function for building a block + * + * We reuse the sequencer's block building functionality for re-execution + */ + public registerBlockBuilder(blockBuilder: BlockBuilderCallback) { + this.blockBuilder = blockBuilder; + } + async attestToProposal(proposal: BlockProposal): Promise { // Check that all of the tranasctions in the proposal are available in the tx pool before attesting this.log.verbose(`request to attest`, { archive: proposal.payload.archive.toString(), - txHashes: proposal.payload.txHashes, + txHashes: proposal.payload.txHashes.map(txHash => txHash.toString()), }); try { await this.ensureTransactionsAreAvailable(proposal); + + if (this.config.validatorReexecute) { + this.log.verbose(`Re-executing transactions in the proposal before attesting`); + await this.reExecuteTransactions(proposal); + } } catch (error: any) { if (error instanceof TransactionsNotAvailableError) { this.log.error(`Transactions not available, skipping attestation ${error.message}`); + } else { + // Catch all error handler + this.log.error(`Failed to attest to proposal: ${error.message}`); } return undefined; } @@ -108,6 +147,42 @@ export class ValidatorClient extends WithTracer implements Validator { return this.validationService.attestToProposal(proposal); } + /** + * Re-execute the transactions in the proposal and check that the state updates match the header state + * @param proposal - The proposal to re-execute + */ + async reExecuteTransactions(proposal: BlockProposal) { + const { header, txHashes } = proposal.payload; + + const txs = (await Promise.all(txHashes.map(tx => this.p2pClient.getTxByHash(tx)))).filter( + tx => tx !== undefined, + ) as Tx[]; + + // If we cannot request all of the transactions, then we should fail + if (txs.length !== txHashes.length) { + this.log.error(`Failed to get transactions from the network: ${txHashes.join(', ')}`); + throw new TransactionsNotAvailableError(txHashes); + } + + // Assertion: This check will fail if re-execution is not enabled + if (this.blockBuilder === undefined) { + throw new BlockBuilderNotProvidedError(); + } + + // Use the sequencer's block building logic to re-execute the transactions + const stopTimer = this.metrics.reExecutionTimer(); + const { block } = await this.blockBuilder(txs, header.globalVariables); + stopTimer(); + + this.log.verbose(`Re-ex: Re-execution complete`); + + // This function will throw an error if state updates do not match + if (!block.archive.root.equals(proposal.archive)) { + this.metrics.recordFailedReexecution(proposal); + throw new ReExStateMismatchError(); + } + } + /** * Ensure that all of the transactions in the proposal are available in the tx pool before attesting * @@ -166,15 +241,15 @@ export class ValidatorClient extends WithTracer implements Validator { } const elapsedTime = Date.now() - startTime; - if (elapsedTime > this.attestationWaitTimeoutMs) { + if (elapsedTime > this.config.attestationWaitTimeoutMs) { this.log.error(`Timeout waiting for ${numberOfRequiredAttestations} attestations for slot, ${slot}`); throw new AttestationTimeoutError(numberOfRequiredAttestations, slot); } this.log.verbose( - `Collected ${attestations.length} attestations so far, waiting ${this.attestationPollingIntervalMs}ms for more...`, + `Collected ${attestations.length} attestations so far, waiting ${this.config.attestationPollingIntervalMs}ms for more...`, ); - await sleep(this.attestationPollingIntervalMs); + await sleep(this.config.attestationPollingIntervalMs); } } }