init

fix: fixing private voting by correctly throwing an error if simulation fails (#7840)

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

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.

Note: this update does not make this validation necessary, and defaults
to offline simulation. Offline simulation is previous non-validated
behavior, and is potentially useful if we ever move to a model where a
node is optional to a pxe.

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

Resolves #4781.

Apply suggestions from code review

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

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

@@ -2,6 +2,7 @@ import { createArchiver } from '@aztec/archiver';
 import { BBCircuitVerifier, TestCircuitVerifier } from '@aztec/bb-prover';
 import {
   type AztecNode,
+  type ClientProtocolCircuitVerifier,
   type FromLogType,
   type GetUnencryptedLogsResponse,
   type L1ToL2MessageSource,
@@ -24,7 +25,6 @@ import {
   type TxHash,
   TxReceipt,
   TxStatus,
-  type TxValidator,
   partitionReverts,
 } from '@aztec/circuit-types';
 import {
@@ -57,8 +57,15 @@ import { getCanonicalFeeJuice } from '@aztec/protocol-contracts/fee-juice';
 import { getCanonicalInstanceDeployer } from '@aztec/protocol-contracts/instance-deployer';
 import { getCanonicalKeyRegistryAddress } from '@aztec/protocol-contracts/key-registry';
 import { getCanonicalMultiCallEntrypointAddress } from '@aztec/protocol-contracts/multi-call-entrypoint';
-import { AggregateTxValidator, DataTxValidator, GlobalVariableBuilder, SequencerClient } from '@aztec/sequencer-client';
-import { PublicProcessorFactory, WASMSimulator, createSimulationProvider } from '@aztec/simulator';
+import {
+  AggregateTxValidator,
+  DataTxValidator,
+  DoubleSpendTxValidator,
+  GlobalVariableBuilder,
+  MetadataTxValidator,
+  SequencerClient,
+} from '@aztec/sequencer-client';
+import { PublicProcessorFactory, WASMSimulator, WorldStateDB, createSimulationProvider } from '@aztec/simulator';
 import { type TelemetryClient } from '@aztec/telemetry-client';
 import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
 import {
@@ -72,7 +79,6 @@ import { MerkleTrees, type WorldStateSynchronizer, createWorldStateSynchronizer
 
 import { type AztecNodeConfig, getPackageInfo } from './config.js';
 import { NodeMetrics } from './node_metrics.js';
-import { MetadataTxValidator } from './tx_validator/tx_metadata_validator.js';
 import { TxProofValidator } from './tx_validator/tx_proof_validator.js';
 
 /**
@@ -97,7 +103,7 @@ export class AztecNodeService implements AztecNode {
     protected readonly version: number,
     protected readonly globalVariableBuilder: GlobalVariableBuilder,
     protected readonly merkleTreesDb: AztecKVStore,
-    private txValidator: TxValidator,
+    private proofVerifier: ClientProtocolCircuitVerifier,
     private telemetry: TelemetryClient,
     private log = createDebugLogger('aztec:node'),
   ) {
@@ -158,11 +164,6 @@ export class AztecNodeService implements AztecNode {
     await Promise.all([p2pClient.start(), worldStateSynchronizer.start()]);
 
     const proofVerifier = config.realProofs ? await BBCircuitVerifier.new(config) : new TestCircuitVerifier();
-    const txValidator = new AggregateTxValidator(
-      new DataTxValidator(),
-      new MetadataTxValidator(config.l1ChainId),
-      new TxProofValidator(proofVerifier),
-    );
 
     const simulationProvider = await createSimulationProvider(config, log);
 
@@ -197,7 +198,7 @@ export class AztecNodeService implements AztecNode {
       config.version,
       new GlobalVariableBuilder(config),
       store,
-      txValidator,
+      proofVerifier,
       telemetry,
       log,
     );
@@ -762,7 +763,26 @@ export class AztecNodeService implements AztecNode {
   }
 
   public async isValidTx(tx: Tx): Promise<boolean> {
-    const [_, invalidTxs] = await this.txValidator.validateTxs([tx]);
+    const blockNumber = (await this.blockSource.getBlockNumber()) + 1;
+
+    const newGlobalVariables = await this.globalVariableBuilder.buildGlobalVariables(
+      new Fr(blockNumber),
+      // We only need chainId and block number, thus coinbase and fee recipient can be set to 0.
+      EthAddress.ZERO,
+      AztecAddress.ZERO,
+    );
+
+    // 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(
+      new DataTxValidator(),
+      new MetadataTxValidator(newGlobalVariables),
+      new DoubleSpendTxValidator(new WorldStateDB(this.worldStateSynchronizer.getLatest())),
+      new TxProofValidator(this.proofVerifier),
+    );
+
+    const [_, invalidTxs] = await txValidator.validateTxs([tx]);
     if (invalidTxs.length > 0) {
       this.log.warn(`Rejecting tx ${tx.getTxHash()} because of validation errors`);
 
@@ -777,12 +797,7 @@ export class AztecNodeService implements AztecNode {
     this.sequencer?.updateSequencerConfig(config);
 
     if (newConfig.realProofs !== this.config.realProofs) {
-      const proofVerifier = config.realProofs ? await BBCircuitVerifier.new(newConfig) : new TestCircuitVerifier();
-
-      this.txValidator = new AggregateTxValidator(
-        new MetadataTxValidator(this.l1ChainId),
-        new TxProofValidator(proofVerifier),
-      );
+      this.proofVerifier = config.realProofs ? await BBCircuitVerifier.new(newConfig) : new TestCircuitVerifier();
     }
 
     this.config = newConfig;

yarn-project/sequencer-client/src/index.ts

@@ -4,6 +4,8 @@ export * from './publisher/index.js';
 export * from './sequencer/index.js';
 export * from './tx_validator/aggregate_tx_validator.js';
 export * from './tx_validator/data_validator.js';
+export * from './tx_validator/double_spend_validator.js';
+export * from './tx_validator/metadata_validator.js';
 
 // Used by the node to simulate public parts of transactions. Should these be moved to a shared library?
 export * from './global_variable_builder/index.js';