feat(avm-transpiler): add emitnotehash and emitnullifier opcodes to a…

…vm transpiler and simulator tests (#4746)

avm-transpiler/src/transpile.rs

@@ -148,19 +148,19 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
                 destination,
                 source,
             } => {
-                avm_instrs.push(emit_mov(Some(ALL_DIRECT), source.to_usize() as u32, destination.to_usize() as u32));
+                avm_instrs.push(generate_mov_instruction(Some(ALL_DIRECT), source.to_usize() as u32, destination.to_usize() as u32));
             }
             BrilligOpcode::Load {
                 destination,
                 source_pointer,
             } => {
-                avm_instrs.push(emit_mov(Some(ZEROTH_OPERAND_INDIRECT), source_pointer.to_usize() as u32, destination.to_usize() as u32));
+                avm_instrs.push(generate_mov_instruction(Some(ZEROTH_OPERAND_INDIRECT), source_pointer.to_usize() as u32, destination.to_usize() as u32));
             }
             BrilligOpcode::Store {
                 destination_pointer,
                 source,
             } => {
-                avm_instrs.push(emit_mov(Some(FIRST_OPERAND_INDIRECT), source.to_usize() as u32, destination_pointer.to_usize() as u32));
+                avm_instrs.push(generate_mov_instruction(Some(FIRST_OPERAND_INDIRECT), source.to_usize() as u32, destination_pointer.to_usize() as u32));
             }
             BrilligOpcode::Call { location } => {
                 let avm_loc = brillig_pcs_to_avm_pcs[*location];
@@ -202,7 +202,7 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
                 });
             },
             BrilligOpcode::Cast { destination, source, bit_size } => {
-                avm_instrs.push(emit_cast(source.to_usize() as u32, destination.to_usize() as u32, tag_from_bit_size(*bit_size)));
+                avm_instrs.push(generate_cast_instruction(source.to_usize() as u32, destination.to_usize() as u32, tag_from_bit_size(*bit_size)));
             }
             BrilligOpcode::ForeignCall { function, destinations, inputs, destination_value_types:_, input_value_types:_ } => {
                 handle_foreign_call(&mut avm_instrs, function, destinations, inputs);
@@ -225,8 +225,11 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
     bytecode
 }
 
-/// Handle foreign function calls
-/// - Environment getting opcodes will be represented as foreign calls
+/// Handle brillig foreign calls
+/// Examples:
+/// - Tree access opcodes
+/// - Hashing/gadget opcodes
+/// - Environment getter opcodes
 /// - TODO: support for avm external calls through this function
 fn handle_foreign_call(
     avm_instrs: &mut Vec<AvmInstruction>,
@@ -235,16 +238,66 @@ fn handle_foreign_call(
     inputs: &Vec<ValueOrArray>,
 ) {
     match function.as_str() {
+        "emitNoteHash" | "emitNullifier" => handle_emit_note_hash_or_nullifier(
+            function.as_str() == "emitNullifier",
+            avm_instrs,
+            destinations,
+            inputs,
+        ),
         "keccak256" | "sha256" => {
-            emit_2_field_hash_instruction(avm_instrs, function, destinations, inputs)
+            handle_2_field_hash_instruction(avm_instrs, function, destinations, inputs)
         }
         "poseidon" => {
-            emit_single_field_hash_instruction(avm_instrs, function, destinations, inputs)
+            handle_single_field_hash_instruction(avm_instrs, function, destinations, inputs)
         }
         _ => handle_getter_instruction(avm_instrs, function, destinations, inputs),
     }
 }
 
+/// Handle an AVM EMITNOTEHASH or EMITNULLIFIER instruction
+/// (an emitNoteHash or emitNullifier brillig foreign call was encountered)
+/// Adds the new instruction to the avm instructions list.
+fn handle_emit_note_hash_or_nullifier(
+    is_nullifier: bool, // false for note hash, true for nullifier
+    avm_instrs: &mut Vec<AvmInstruction>,
+    destinations: &Vec<ValueOrArray>,
+    inputs: &Vec<ValueOrArray>,
+) {
+    let function_name = if is_nullifier {
+        "EMITNULLIFIER"
+    } else {
+        "EMITNOTEHASH"
+    };
+
+    if destinations.len() != 0 || inputs.len() != 1 {
+        panic!(
+            "Transpiler expects ForeignCall::{} to have 0 destinations and 1 input, got {} and {}",
+            function_name,
+            destinations.len(),
+            inputs.len()
+        );
+    }
+    let offset_operand = match &inputs[0] {
+        ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
+        _ => panic!(
+            "Transpiler does not know how to handle ForeignCall::{} with HeapArray/Vector inputs",
+            function_name
+        ),
+    };
+    avm_instrs.push(AvmInstruction {
+        opcode: if is_nullifier {
+            AvmOpcode::EMITNULLIFIER
+        } else {
+            AvmOpcode::EMITNOTEHASH
+        },
+        indirect: Some(ALL_DIRECT),
+        operands: vec![AvmOperand::U32 {
+            value: offset_operand,
+        }],
+        ..Default::default()
+    });
+}
+
 /// Two field hash instructions represent instruction's that's outputs are larger than a field element
 ///
 /// This includes:
@@ -253,15 +306,14 @@ fn handle_foreign_call(
 ///
 /// In the future the output of these may expand / contract depending on what is most efficient for the circuit
 /// to reason about. In order to decrease user friction we will use two field outputs.
-fn emit_2_field_hash_instruction(
+fn handle_2_field_hash_instruction(
     avm_instrs: &mut Vec<AvmInstruction>,
     function: &String,
     destinations: &[ValueOrArray],
     inputs: &[ValueOrArray],
 ) {
     // handle field returns differently
     let hash_offset_maybe = inputs[0];
-    println!("hash_offset_maybe: {:?}", hash_offset_maybe);
     let (hash_offset, hash_size) = match hash_offset_maybe {
         ValueOrArray::HeapArray(HeapArray { pointer, size }) => (pointer.0, size),
         _ => panic!("Keccak | Sha256 address inputs destination should be a single value"),
@@ -313,7 +365,7 @@ fn emit_2_field_hash_instruction(
 /// Pedersen is not implemented this way as the black box function representation has the correct api.
 /// As the Poseidon BBF only deals with a single permutation, it is not quite suitable for our current avm
 /// representation.
-fn emit_single_field_hash_instruction(
+fn handle_single_field_hash_instruction(
     avm_instrs: &mut Vec<AvmInstruction>,
     function: &String,
     destinations: &[ValueOrArray],
@@ -422,7 +474,7 @@ fn handle_const(
     let dest = destination.to_usize() as u32;
 
     if !matches!(tag, AvmTypeTag::FIELD) {
-        avm_instrs.push(emit_set(tag, dest, value.to_u128()));
+        avm_instrs.push(generate_set_instruction(tag, dest, value.to_u128()));
     } else {
         // Handling fields is a bit more complex since we cannot fit a field in a single instruction.
         // We need to split the field into 128-bit chunks and set them individually.
@@ -434,14 +486,14 @@ fn handle_const(
             panic!("SET: Field value doesn't fit in 128 bits, that's not supported yet!");
         }
         avm_instrs.extend([
-            emit_set(AvmTypeTag::UINT128, dest, field.to_u128()),
-            emit_cast(dest, dest, AvmTypeTag::FIELD),
+            generate_set_instruction(AvmTypeTag::UINT128, dest, field.to_u128()),
+            generate_cast_instruction(dest, dest, AvmTypeTag::FIELD),
         ]);
     }
 }
 
-/// Emits an AVM SET instruction.
-fn emit_set(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstruction {
+/// Generates an AVM SET instruction.
+fn generate_set_instruction(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstruction {
     AvmInstruction {
         opcode: AvmOpcode::SET,
         indirect: Some(ALL_DIRECT),
@@ -468,8 +520,8 @@ fn emit_set(tag: AvmTypeTag, dest: u32, value: u128) -> AvmInstruction {
     }
 }
 
-/// Emits an AVM CAST instruction.
-fn emit_cast(source: u32, destination: u32, dst_tag: AvmTypeTag) -> AvmInstruction {
+/// Generates an AVM CAST instruction.
+fn generate_cast_instruction(source: u32, destination: u32, dst_tag: AvmTypeTag) -> AvmInstruction {
     AvmInstruction {
         opcode: AvmOpcode::CAST,
         indirect: Some(ALL_DIRECT),
@@ -481,8 +533,8 @@ fn emit_cast(source: u32, destination: u32, dst_tag: AvmTypeTag) -> AvmInstructi
     }
 }
 
-/// Emits an AVM MOV instruction.
-fn emit_mov(indirect: Option<u8>, source: u32, dest: u32) -> AvmInstruction {
+/// Generates an AVM MOV instruction.
+fn generate_mov_instruction(indirect: Option<u8>, source: u32, dest: u32) -> AvmInstruction {
     AvmInstruction {
         opcode: AvmOpcode::MOV,
         indirect,

noir-projects/aztec-nr/aztec/src/avm.nr

@@ -1,2 +1 @@
-mod context;
 mod hash;

noir-projects/aztec-nr/aztec/src/context/avm.nr

@@ -45,6 +45,23 @@ impl AVMContext {
 
     #[oracle(timestamp)]
     pub fn timestamp(self) -> Field {}
+
     // #[oracle(contractCallDepth)]
     // pub fn contract_call_depth(self) -> Field {}
+
+    #[oracle(emitNoteHash)]
+    pub fn emit_note_hash(self, note_hash: Field) {}
+
+    #[oracle(emitNullifier)]
+    pub fn emit_nullifier(self, nullifier: Field) {}
+
+    // The functions below allow interface-equivalence with PrivateContext
+    // for emitting note hashes and nullifiers
+    pub fn push_new_note_hash(self: &mut Self, note_hash: Field) {
+        self.emit_note_hash(note_hash);
+    }
+    pub fn push_new_nullifier(self: &mut Self, nullifier: Field, _nullified_commitment: Field) {
+        // Cannot nullify pending commitments in AVM, so `nullified_commitment` is not used
+        self.emit_nullifier(nullifier);
+    }
 }

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -3,14 +3,7 @@ contract AvmTest {
     use dep::aztec::protocol_types::address::{AztecAddress, EthAddress};
 
     // avm lib
-    use dep::aztec::avm::{
-        context::AvmContext,
-        hash::{
-            keccak256,
-            poseidon,
-            sha256
-        }
-    };
+    use dep::aztec::avm::hash::{keccak256, poseidon, sha256};
 
     #[aztec(private)]
     fn constructor() {}
@@ -141,8 +134,29 @@ contract AvmTest {
     fn getTimestamp() -> pub Field {
         context.timestamp()
     }
+
     // #[aztec(public-vm)]
     // fn getContractCallDepth() -> pub Field {
     //     context.contract_call_depth()
     // }
+
+    // Use the standard context interface to emit a new note hash
+    #[aztec(public-vm)]
+    fn new_note_hash(note_hash: Field) {
+        context.push_new_note_hash(note_hash);
+    }
+
+    // Use the standard context interface to emit a new nullifier
+    #[aztec(public-vm)]
+    fn new_nullifier(nullifier: Field) {
+        context.push_new_nullifier(nullifier, 0);
+    }
+
+    // Create the same nullifier twice (shouldn't work!)
+    #[aztec(public-vm)]
+    fn nullifier_collision(nullifier: Field) {
+        context.push_new_nullifier(nullifier, 0);
+        // Can't do this twice!
+        context.push_new_nullifier(nullifier, 0);
+    }
 }

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

@@ -236,5 +236,72 @@ describe('AVM simulator', () => {
         await testEnvGetter('timestamp', timestamp, 'avm_getTimestamp', /*globalVar=*/ true);
       });
     });
+
+    describe('Test tree access functions from noir contract', () => {
+      it(`Should execute contract function to emit note hash (should be traced)`, async () => {
+        const utxo = new Fr(42);
+        const calldata = [utxo];
+
+        // Get contract function artifact
+        const artifact = AvmTestContractArtifact.functions.find(f => f.name === 'avm_new_note_hash')!;
+
+        // Decode bytecode into instructions
+        const bytecode = Buffer.from(artifact.bytecode, 'base64');
+
+        const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+        jest
+          .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
+          .mockReturnValue(Promise.resolve(bytecode));
+
+        const results = await new AvmSimulator(context).execute();
+
+        expect(results.reverted).toBe(false);
+
+        expect(context.persistableState.flush().newNoteHashes).toEqual([utxo]);
+      });
+      it(`Should execute contract function to emit nullifier (should be traced)`, async () => {
+        const utxo = new Fr(42);
+        const calldata = [utxo];
+
+        // Get contract function artifact
+        const artifact = AvmTestContractArtifact.functions.find(f => f.name === 'avm_new_nullifier')!;
+
+        // Decode bytecode into instructions
+        const bytecode = Buffer.from(artifact.bytecode, 'base64');
+
+        const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+        jest
+          .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
+          .mockReturnValue(Promise.resolve(bytecode));
+
+        const results = await new AvmSimulator(context).execute();
+
+        expect(results.reverted).toBe(false);
+
+        expect(context.persistableState.flush().newNullifiers).toEqual([utxo]);
+      });
+      it(`Should execute contract function that emits same nullifier twice (should fail)`, async () => {
+        const utxo = new Fr(42);
+        const calldata = [utxo];
+
+        // Get contract function artifact
+        const artifact = AvmTestContractArtifact.functions.find(f => f.name === 'avm_nullifier_collision')!;
+
+        // Decode bytecode into instructions
+        const bytecode = Buffer.from(artifact.bytecode, 'base64');
+
+        const context = initContext({ env: initExecutionEnvironment({ calldata }) });
+        jest
+          .spyOn(context.persistableState.hostStorage.contractsDb, 'getBytecode')
+          .mockReturnValue(Promise.resolve(bytecode));
+
+        await new AvmSimulator(context).execute();
+        const results = await new AvmSimulator(context).execute();
+        expect(results.reverted).toBe(true);
+
+        // Only the first nullifier should be in the trace, second one failed to add
+        expect(context.persistableState.flush().newNullifiers).toEqual([utxo]);
+      });
+    });
   });
 });

yarn-project/simulator/src/avm/opcodes/accrued_substate.ts

@@ -43,12 +43,6 @@ export class EmitNullifier extends Instruction {
     }
 
     const nullifier = context.machineState.memory.get(this.nullifierOffset).toFr();
-    const exists = await context.persistableState.checkNullifierExists(context.environment.storageAddress, nullifier);
-    if (exists) {
-      throw new InstructionExecutionError(
-        `Attempted to emit duplicate nullifier ${nullifier} (storage address: ${context.environment.storageAddress}).`,
-      );
-    }
     try {
       await context.persistableState.writeNullifier(context.environment.storageAddress, nullifier);
     } catch (e) {