fix: lay plumbing for having simulations throw an error if they canno…

…t be added in a block (#7839)

This PR gives an option to make a simulation of a tx fail, if the tx
cannot be included in a block and added to the state.

To minimize impact on callsites / disruption, this update does not make
this validation necessary, and defaults to skipping tx validation,
exhibiting previous non-validated behavior. This will be changed to be
default behavior in the future.

e.g. If a simulation produces duplicate nullifiers, or nullifiers that
already exist in a state tree, the results of this simulation should not
be returned, but should warn users that the transaction simulated is
impossible to actually be added to a block due to being an invalid
transaction.

The method for achieving the above is that a new API on the node was
created, used for validating the correctness of the metadata and
side-effects produced by a transaction. A transaction is deemed valid if
and only if the transaction can be added to a block that can be used to
advance state.

Another note just for reference: there is weirdness in e2e_prover, that
fails the proof validation.

---------

Co-authored-by: Nicolás Venturo <[email protected]>

yarn-project/aztec-node/src/aztec-node/server.ts

@@ -324,9 +324,7 @@ export class AztecNodeService implements AztecNode {
     const timer = new Timer();
     this.log.info(`Received tx ${tx.getTxHash()}`);
 
-    const [_, invalidTxs] = await this.txValidator.validateTxs([tx]);
-    if (invalidTxs.length > 0) {
-      this.log.warn(`Rejecting tx ${tx.getTxHash()} because of validation errors`);
+    if (!(await this.isValidTx(tx))) {
       this.metrics.receivedTx(timer.ms(), false);
       return;
     }
@@ -752,6 +750,17 @@ export class AztecNodeService implements AztecNode {
     );
   }
 
+  public async isValidTx(tx: Tx): Promise<boolean> {
+    const [_, invalidTxs] = await this.txValidator.validateTxs([tx]);
+    if (invalidTxs.length > 0) {
+      this.log.warn(`Rejecting tx ${tx.getTxHash()} because of validation errors`);
+
+      return false;
+    }
+
+    return true;
+  }
+
   public async setConfig(config: Partial<SequencerConfig & ProverConfig>): Promise<void> {
     const newConfig = { ...this.config, ...config };
     this.sequencer?.updateSequencerConfig(config);

yarn-project/aztec.js/src/contract/contract_function_interaction.ts

@@ -23,6 +23,8 @@ export type SimulateMethodOptions = {
   from?: AztecAddress;
   /** Gas settings for the simulation. */
   gasSettings?: GasSettings;
+  /** Simulate without checking for the validity of the resulting transaction, e.g. whether it emits any existing nullifiers. */
+  skipTxValidation?: boolean;
 };
 
 /**
@@ -93,7 +95,7 @@ export class ContractFunctionInteraction extends BaseContractInteraction {
     }
 
     const txRequest = await this.create();
-    const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from);
+    const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from, options?.skipTxValidation);
 
     // As account entrypoints are private, for private functions we retrieve the return values from the first nested call
     // since we're interested in the first set of values AFTER the account entrypoint

yarn-project/aztec.js/src/wallet/base_wallet.ts

@@ -109,8 +109,13 @@ export abstract class BaseWallet implements Wallet {
   proveTx(txRequest: TxExecutionRequest, simulatePublic: boolean): Promise<Tx> {
     return this.pxe.proveTx(txRequest, simulatePublic, this.scopes);
   }
-  simulateTx(txRequest: TxExecutionRequest, simulatePublic: boolean, msgSender?: AztecAddress): Promise<SimulatedTx> {
-    return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, this.scopes);
+  simulateTx(
+    txRequest: TxExecutionRequest,
+    simulatePublic: boolean,
+    msgSender?: AztecAddress,
+    skipTxValidation?: boolean,
+  ): Promise<SimulatedTx> {
+    return this.pxe.simulateTx(txRequest, simulatePublic, msgSender, skipTxValidation, this.scopes);
   }
   sendTx(tx: Tx): Promise<TxHash> {
     return this.pxe.sendTx(tx);

yarn-project/circuit-types/src/interfaces/aztec-node.ts

@@ -316,6 +316,14 @@ export interface AztecNode {
    **/
   simulatePublicCalls(tx: Tx): Promise<PublicSimulationOutput>;
 
+  /**
+   * Returns true if the transaction is valid for inclusion at the current state. Valid transactions can be
+   * 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.
+   */
+  isValidTx(tx: Tx): Promise<boolean>;
+
   /**
    * Updates the configuration of this node.
    * @param config - Updated configuration to be merged with the current one.

yarn-project/circuit-types/src/interfaces/pxe.ts

@@ -180,6 +180,7 @@ export interface PXE {
    * @param txRequest - An authenticated tx request ready for simulation
    * @param simulatePublic - Whether to simulate the public part of the transaction.
    * @param msgSender - (Optional) The message sender to use for the simulation.
+   * @param skipTxValidation - (Optional) If false, this function throws if the transaction is unable to be included in a block at the current state.
    * @param scopes - (Optional) The accounts whose notes we can access in this call. Currently optional and will default to all.
    * @returns A simulated transaction object that includes a transaction that is potentially ready
    * to be sent to the network for execution, along with public and private return values.
@@ -190,6 +191,7 @@ export interface PXE {
     txRequest: TxExecutionRequest,
     simulatePublic: boolean,
     msgSender?: AztecAddress,
+    skipTxValidation?: boolean,
     scopes?: AztecAddress[],
   ): Promise<SimulatedTx>;
 

yarn-project/pxe/src/pxe_service/pxe_service.ts

@@ -513,6 +513,7 @@ export class PXEService implements PXE {
     txRequest: TxExecutionRequest,
     simulatePublic: boolean,
     msgSender: AztecAddress | undefined = undefined,
+    skipTxValidation: boolean = true, // TODO(#7956): make the default be false
     scopes?: AztecAddress[],
   ): Promise<SimulatedTx> {
     return await this.jobQueue.put(async () => {
@@ -521,6 +522,12 @@ export class PXEService implements PXE {
         simulatedTx.publicOutput = await this.#simulatePublicCalls(simulatedTx.tx);
       }
 
+      if (!skipTxValidation) {
+        if (!(await this.node.isValidTx(simulatedTx.tx))) {
+          throw new Error('The simulated transaction is unable to be added to state and is invalid.');
+        }
+      }
+
       // We log only if the msgSender is undefined, as simulating with a different msgSender
       // is unlikely to be a real transaction, and likely to be only used to read data.
       // Meaning that it will not necessarily have produced a nullifier (and thus have no TxHash)