Skip to content

Commit

Permalink
fix: make simulations validate resulting tx by default (#8157)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sklppy88 authored Aug 30, 2024
1 parent c7eaf92 commit f5e388d
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 9 deletions.
15 changes: 11 additions & 4 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
LogType,
MerkleTreeId,
NullifierMembershipWitness,
type ProcessedTx,
type ProverConfig,
PublicDataWitness,
PublicSimulationOutput,
Expand All @@ -25,6 +26,7 @@ import {
type TxHash,
TxReceipt,
TxStatus,
type TxValidator,
partitionReverts,
} from '@aztec/circuit-types';
import {
Expand Down Expand Up @@ -762,7 +764,7 @@ export class AztecNodeService implements AztecNode {
);
}

public async isValidTx(tx: Tx): Promise<boolean> {
public async isValidTx(tx: Tx, isSimulation: boolean = false): Promise<boolean> {
const blockNumber = (await this.blockSource.getBlockNumber()) + 1;

const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(
Expand All @@ -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<Tx | ProcessedTx>[] = [
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) {
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/src/contract/batch_call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand Down
3 changes: 2 additions & 1 deletion yarn-project/circuit-types/src/interfaces/aztec-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>;
isValidTx(tx: Tx, isSimulation?: boolean): Promise<boolean>;

/**
* Updates the configuration of this node.
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/package.local.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
4 changes: 2 additions & 2 deletions yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SimulatedTx> {
return await this.jobQueue.put(async () => {
Expand All @@ -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.');
}
}
Expand Down

0 comments on commit f5e388d

Please sign in to comment.