feat: AVM uses native checkpointing trees instead of TS ephemeral forest

yarn-project/circuit-types/src/tx/validator/tx_validator.ts

@@ -7,6 +7,10 @@ import { type Tx } from '../tx.js';
 
 export type AnyTx = Tx | ProcessedTx;
 
+export function hasPublicCalls(tx: AnyTx): boolean {
+  return tx.data.numberOfPublicCallRequests() > 0;
+}
+
 export type TxValidationResult =
   | { result: 'valid' }
   | { result: 'invalid'; reason: string[] }

yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.test.ts

@@ -9,6 +9,9 @@ describe('DoubleSpendTxValidator', () => {
   let txValidator: DoubleSpendTxValidator<AnyTx>;
   let nullifierSource: MockProxy<NullifierSource>;
 
+  const expectValid = async (tx: AnyTx) => {
+    await expect(txValidator.validateTx(tx)).resolves.toEqual({ result: 'valid' });
+  };
   const expectInvalid = async (tx: AnyTx, reason: string) => {
     await expect(txValidator.validateTx(tx)).resolves.toEqual({ result: 'invalid', reason: [reason] });
   };
@@ -31,19 +34,24 @@ describe('DoubleSpendTxValidator', () => {
     await expectInvalid(badTx, 'Duplicate nullifier in tx');
   });
 
-  it('rejects duplicates across phases', async () => {
+  it('rejects duplicates against history', async () => {
+    const badTx = await mockTx(1, {
+      numberOfNonRevertiblePublicCallRequests: 0,
+      numberOfRevertiblePublicCallRequests: 0,
+    });
+    nullifierSource.nullifiersExist.mockResolvedValue([true]);
+    await expectInvalid(badTx, 'Existing nullifier');
+  });
+
+  // If the tx has public calls, all merkle insertions will be performed by the AVM,
+  // and the AVM will catch any duplicates. So we don't need to check during tx validation.
+  it('accepts duplicates if the tx has public calls', async () => {
     const badTx = await mockTx(1, {
       numberOfNonRevertiblePublicCallRequests: 1,
       numberOfRevertiblePublicCallRequests: 1,
     });
     badTx.data.forPublic!.revertibleAccumulatedData.nullifiers[0] =
       badTx.data.forPublic!.nonRevertibleAccumulatedData.nullifiers[0];
-    await expectInvalid(badTx, 'Duplicate nullifier in tx');
-  });
-
-  it('rejects duplicates against history', async () => {
-    const badTx = await mockTx();
-    nullifierSource.nullifiersExist.mockResolvedValue([true]);
-    await expectInvalid(badTx, 'Existing nullifier');
+    await expectValid(badTx);
   });
 });

yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.ts

@@ -1,4 +1,4 @@
-import { type AnyTx, Tx, type TxValidationResult, type TxValidator } from '@aztec/circuit-types';
+import { type AnyTx, Tx, type TxValidationResult, type TxValidator, hasPublicCalls } from '@aztec/circuit-types';
 import { createLogger } from '@aztec/foundation/log';
 
 export interface NullifierSource {
@@ -14,20 +14,25 @@ export class DoubleSpendTxValidator<T extends AnyTx> implements TxValidator<T> {
   }
 
   async validateTx(tx: T): Promise<TxValidationResult> {
-    const nullifiers = tx instanceof Tx ? tx.data.getNonEmptyNullifiers() : tx.txEffect.nullifiers;
+    // Don't need to check for duplicate nullifiers if the tx has public calls
+    // because the AVM will perform merkle insertions as it goes and will fail on
+    // duplicate nullifier. In fact we CANNOT check here because the nullifiers
+    // have already been inserted, and so they will exist in nullifierSource.
+    if (!hasPublicCalls(tx)) {
+      const nullifiers = tx instanceof Tx ? tx.data.getNonEmptyNullifiers() : tx.txEffect.nullifiers;
 
-    // Ditch this tx if it has repeated nullifiers
-    const uniqueNullifiers = new Set(nullifiers);
-    if (uniqueNullifiers.size !== nullifiers.length) {
-      this.#log.warn(`Rejecting tx ${await Tx.getHash(tx)} for emitting duplicate nullifiers`);
-      return { result: 'invalid', reason: ['Duplicate nullifier in tx'] };
-    }
+      // Ditch this tx if it has repeated nullifiers
+      const uniqueNullifiers = new Set(nullifiers);
+      if (uniqueNullifiers.size !== nullifiers.length) {
+        this.#log.warn(`Rejecting tx ${await Tx.getHash(tx)} for emitting duplicate nullifiers`);
+        return { result: 'invalid', reason: ['Duplicate nullifier in tx'] };
+      }
 
-    if ((await this.#nullifierSource.nullifiersExist(nullifiers.map(n => n.toBuffer()))).some(Boolean)) {
-      this.#log.warn(`Rejecting tx ${await Tx.getHash(tx)} for repeating a nullifier`);
-      return { result: 'invalid', reason: ['Existing nullifier'] };
+      if ((await this.#nullifierSource.nullifiersExist(nullifiers.map(n => n.toBuffer()))).some(Boolean)) {
+        this.#log.warn(`Rejecting tx ${await Tx.getHash(tx)} for repeating a nullifier`);
+        return { result: 'invalid', reason: ['Existing nullifier'] };
+      }
     }
-
     return { result: 'valid' };
   }
 }

yarn-project/simulator/src/avm/avm_context.test.ts

@@ -10,7 +10,7 @@ describe('Avm Context', () => {
     const newAddress = await AztecAddress.random();
     const newCalldata = [new Fr(1), new Fr(2)];
     const allocatedGas = { l2Gas: 2, daGas: 3 }; // How much of the current call gas we pass to the nested call
-    const newContext = context.createNestedContractCallContext(newAddress, newCalldata, allocatedGas, 'CALL');
+    const newContext = await context.createNestedContractCallContext(newAddress, newCalldata, allocatedGas, 'CALL');
 
     expect(newContext.environment).toEqual(
       allSameExcept(context.environment, {
@@ -29,7 +29,7 @@ describe('Avm Context', () => {
     );
 
     // We stringify to remove circular references (parentJournal)
-    expect(JSON.stringify(newContext.persistableState)).toEqual(JSON.stringify(context.persistableState.fork()));
+    expect(JSON.stringify(newContext.persistableState)).toEqual(JSON.stringify(await context.persistableState.fork()));
   });
 
   it('New static call should fork context correctly', async () => {
@@ -39,7 +39,12 @@ describe('Avm Context', () => {
     const newAddress = await AztecAddress.random();
     const newCalldata = [new Fr(1), new Fr(2)];
     const allocatedGas = { l2Gas: 2, daGas: 3 };
-    const newContext = context.createNestedContractCallContext(newAddress, newCalldata, allocatedGas, 'STATICCALL');
+    const newContext = await context.createNestedContractCallContext(
+      newAddress,
+      newCalldata,
+      allocatedGas,
+      'STATICCALL',
+    );
 
     expect(newContext.environment).toEqual(
       allSameExcept(context.environment, {
@@ -58,6 +63,6 @@ describe('Avm Context', () => {
     );
 
     // We stringify to remove circular references (parentJournal)
-    expect(JSON.stringify(newContext.persistableState)).toEqual(JSON.stringify(context.persistableState.fork()));
+    expect(JSON.stringify(newContext.persistableState)).toEqual(JSON.stringify(await context.persistableState.fork()));
   });
 });

yarn-project/simulator/src/avm/avm_context.ts

@@ -43,18 +43,18 @@ export class AvmContext {
    * @param callType - Type of call (CALL or STATICCALL)
    * @returns new AvmContext instance
    */
-  public createNestedContractCallContext(
+  public async createNestedContractCallContext(
     address: AztecAddress,
     calldata: Fr[],
     allocatedGas: Gas,
     callType: 'CALL' | 'STATICCALL',
-  ): AvmContext {
+  ): Promise<AvmContext> {
     const deriveFn =
       callType === 'CALL'
         ? this.environment.deriveEnvironmentForNestedCall
         : this.environment.deriveEnvironmentForNestedStaticCall;
     const newExecutionEnvironment = deriveFn.call(this.environment, address, calldata);
-    const forkedWorldState = this.persistableState.fork();
+    const forkedWorldState = await this.persistableState.fork();
     const machineState = AvmMachineState.fromState(gasToGasLeft(allocatedGas));
     return new AvmContext(forkedWorldState, newExecutionEnvironment, machineState);
   }