From f5e388dd2d7c78d89da391603c50fda3a2309a76 Mon Sep 17 00:00:00 2001 From: esau <152162806+sklppy88@users.noreply.github.com> Date: Fri, 30 Aug 2024 14:02:30 +0200 Subject: [PATCH] fix: make simulations validate resulting tx by default (#8157) Closes #7956. This PR simply flips a switch to make all simulations fail by default if the transaction is an invalid one. Previously it did not throw due to wanting to minimize disruptions in the PR, but it seems like there were not that many effects. The only exception is the e2e prover. These are failing due to a prover verification, and I have made an issue for this to be looked into. --- yarn-project/aztec-node/src/aztec-node/server.ts | 15 +++++++++++---- yarn-project/aztec.js/src/contract/batch_call.ts | 2 +- .../circuit-types/src/interfaces/aztec-node.ts | 3 ++- yarn-project/end-to-end/package.local.json | 2 +- yarn-project/pxe/src/pxe_service/pxe_service.ts | 4 ++-- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 27d0cbc032f..da98547c0bf 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -15,6 +15,7 @@ import { LogType, MerkleTreeId, NullifierMembershipWitness, + type ProcessedTx, type ProverConfig, PublicDataWitness, PublicSimulationOutput, @@ -25,6 +26,7 @@ import { type TxHash, TxReceipt, TxStatus, + type TxValidator, partitionReverts, } from '@aztec/circuit-types'; import { @@ -762,7 +764,7 @@ export class AztecNodeService implements AztecNode { ); } - public async isValidTx(tx: Tx): Promise { + public async isValidTx(tx: Tx, isSimulation: boolean = false): Promise { const blockNumber = (await this.blockSource.getBlockNumber()) + 1; const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables( @@ -775,12 +777,17 @@ export class AztecNodeService implements AztecNode { // These validators are taken from the sequencer, and should match. // The reason why `phases` and `gas` tx validator is in the sequencer and not here is because // those tx validators are customizable by the sequencer. - const txValidator = new AggregateTxValidator( + const txValidators: TxValidator[] = [ new DataTxValidator(), new MetadataTxValidator(newGlobalVariables), new DoubleSpendTxValidator(new WorldStateDB(this.worldStateSynchronizer.getLatest())), - new TxProofValidator(this.proofVerifier), - ); + ]; + + if (!isSimulation) { + txValidators.push(new TxProofValidator(this.proofVerifier)); + } + + const txValidator = new AggregateTxValidator(...txValidators); const [_, invalidTxs] = await txValidator.validateTxs([tx]); if (invalidTxs.length > 0) { diff --git a/yarn-project/aztec.js/src/contract/batch_call.ts b/yarn-project/aztec.js/src/contract/batch_call.ts index 3a02a713901..19fb03999f6 100644 --- a/yarn-project/aztec.js/src/contract/batch_call.ts +++ b/yarn-project/aztec.js/src/contract/batch_call.ts @@ -78,7 +78,7 @@ export class BatchCall extends BaseContractInteraction { const [unconstrainedResults, simulatedTx] = await Promise.all([ Promise.all(unconstrainedCalls), - this.wallet.simulateTx(txRequest, true, options?.from), + this.wallet.simulateTx(txRequest, true, options?.from, options?.skipTxValidation), ]); const results: any[] = []; diff --git a/yarn-project/circuit-types/src/interfaces/aztec-node.ts b/yarn-project/circuit-types/src/interfaces/aztec-node.ts index 70dbc3fb8b4..16c64a1ac42 100644 --- a/yarn-project/circuit-types/src/interfaces/aztec-node.ts +++ b/yarn-project/circuit-types/src/interfaces/aztec-node.ts @@ -327,8 +327,9 @@ export interface AztecNode { * made invalid by *other* transactions if e.g. they emit the same nullifiers, or come become invalid * due to e.g. the max_block_number property. * @param tx - The transaction to validate for correctness. + * @param isSimulation - True if the transaction is a simulated one without generated proofs. (Optional) */ - isValidTx(tx: Tx): Promise; + isValidTx(tx: Tx, isSimulation?: boolean): Promise; /** * Updates the configuration of this node. diff --git a/yarn-project/end-to-end/package.local.json b/yarn-project/end-to-end/package.local.json index 62f136fa45d..a5214893419 100644 --- a/yarn-project/end-to-end/package.local.json +++ b/yarn-project/end-to-end/package.local.json @@ -5,4 +5,4 @@ "test": "LOG_LEVEL=${LOG_LEVEL:-verbose} DEBUG_COLORS=1 NODE_NO_WARNINGS=1 node --experimental-vm-modules ../node_modules/.bin/jest --testTimeout=300000 --forceExit", "test:unit": "NODE_NO_WARNINGS=1 node --experimental-vm-modules ../node_modules/.bin/jest src/fixtures" } -} \ No newline at end of file +} diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 972ef5ce916..1b058896756 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -532,7 +532,7 @@ export class PXEService implements PXE { txRequest: TxExecutionRequest, simulatePublic: boolean, msgSender: AztecAddress | undefined = undefined, - skipTxValidation: boolean = true, // TODO(#7956): make the default be false + skipTxValidation: boolean = false, scopes?: AztecAddress[], ): Promise { return await this.jobQueue.put(async () => { @@ -542,7 +542,7 @@ export class PXEService implements PXE { } if (!skipTxValidation) { - if (!(await this.node.isValidTx(simulatedTx.tx))) { + if (!(await this.node.isValidTx(simulatedTx.tx, true))) { throw new Error('The simulated transaction is unable to be added to state and is invalid.'); } }