chore(avm): add tag checking and missing indirects (#6936)

UINT32 memory offsets and length checks are commented out until Noir
uses UINT32 for memory.

Also change EMITUNENCRYPTEDLOG to actually take a slice of fields.

avm-transpiler/src/transpile.rs

@@ -435,7 +435,7 @@ fn handle_emit_unencrypted_log(
     destinations: &Vec<ValueOrArray>,
     inputs: &Vec<ValueOrArray>,
 ) {
-    if !destinations.is_empty() || inputs.len() != 2 {
+    if !destinations.is_empty() || inputs.len() != 3 {
         panic!(
             "Transpiler expects ForeignCall::EMITUNENCRYPTEDLOG to have 0 destinations and 3 inputs, got {} and {}",
             destinations.len(),
@@ -449,23 +449,20 @@ fn handle_emit_unencrypted_log(
             inputs[0]
         ),
     };
-    let (message_offset, message_size, message_offset_indirect) = match &inputs[1] {
-        ValueOrArray::HeapArray(array) => {
-            // Heap array, so offset to array is an indirect memory offset
-            (array.pointer.to_usize() as u32, array.size as u32, true)
-        }
-        ValueOrArray::MemoryAddress(single_val) => (single_val.to_usize() as u32, 1, false),
+    // The fields are a slice, and this is represented as a (length: Field, slice: HeapVector).
+    // The length field is redundant and we skipt it.
+    let (message_offset, message_size_offset) = match &inputs[2] {
+        ValueOrArray::HeapVector(vec) => (vec.pointer.to_usize() as u32, vec.size.0 as u32),
         _ => panic!("Unexpected inputs for ForeignCall::EMITUNENCRYPTEDLOG: {:?}", inputs),
     };
-    let indirect_flag = if message_offset_indirect { FIRST_OPERAND_INDIRECT } else { 0 };
     avm_instrs.push(AvmInstruction {
         opcode: AvmOpcode::EMITUNENCRYPTEDLOG,
         // The message array from Brillig is indirect.
-        indirect: Some(indirect_flag),
+        indirect: Some(FIRST_OPERAND_INDIRECT),
         operands: vec![
             AvmOperand::U32 { value: event_offset },
             AvmOperand::U32 { value: message_offset },
-            AvmOperand::U32 { value: message_size },
+            AvmOperand::U32 { value: message_size_offset },
         ],
         ..Default::default()
     });
@@ -971,7 +968,7 @@ fn handle_storage_read(
 
     avm_instrs.push(AvmInstruction {
         opcode: AvmOpcode::SLOAD,
-        indirect: Some(SECOND_OPERAND_INDIRECT),
+        indirect: Some(FIRST_OPERAND_INDIRECT),
         operands: vec![
             AvmOperand::U32 { value: slot_offset as u32 },
             AvmOperand::U32 { value: src_size as u32 },

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

@@ -1,6 +1,6 @@
 use crate::hash::{compute_secret_hash, compute_message_hash, compute_message_nullifier};
 use dep::protocol_types::address::{AztecAddress, EthAddress};
-use dep::protocol_types::traits::{Deserialize, Empty};
+use dep::protocol_types::traits::{Serialize, Deserialize, Empty};
 use dep::protocol_types::abis::function_selector::FunctionSelector;
 use crate::context::inputs::public_context_inputs::PublicContextInputs;
 use crate::context::gas::GasOpts;
@@ -28,11 +28,17 @@ impl PublicContext {
     *
     * @param event_selector The event selector for the log.
     * @param message The message to emit in the log.
-    * Should be automatically convertible to [Field; N]. For example str<N> works with
-    * one char per field. Otherwise you can use CompressedString.
     */
-    pub fn emit_unencrypted_log_with_selector<T>(&mut self, event_selector: Field, log: T) {
-        emit_unencrypted_log(event_selector, log);
+    pub fn emit_unencrypted_log_with_selector<T,N>(
+        &mut self,
+        event_selector: Field,
+        log: T
+    ) where T: Serialize<N> {
+        emit_unencrypted_log(event_selector, Serialize::serialize(log).as_slice());
+    }
+    // For compatibility with the selector-less API. We'll probably rename the above one.
+    pub fn emit_unencrypted_log<T,N>(&mut self, log: T) where T: Serialize<N> {
+        self.emit_unencrypted_log_with_selector(/*event_selector=*/ 5, log);
     }
     pub fn note_hash_exists(self, note_hash: Field, leaf_index: Field) -> bool {
         note_hash_exists(note_hash, leaf_index) == 1
@@ -57,11 +63,6 @@ impl PublicContext {
         nullifier_exists(unsiloed_nullifier, address.to_field()) == 1
     }
 
-    fn emit_unencrypted_log<T, N, M>(&mut self, log: T) {
-        let event_selector = 5; // Matches current PublicContext.
-        self.emit_unencrypted_log_with_selector(event_selector, log);
-    }
-
     fn consume_l1_to_l2_message(
         &mut self,
         content: Field,
@@ -234,7 +235,7 @@ unconstrained fn nullifier_exists(nullifier: Field, address: Field) -> u8 {
 unconstrained fn emit_nullifier(nullifier: Field) {
     emit_nullifier_opcode(nullifier)
 }
-unconstrained fn emit_unencrypted_log<T>(event_selector: Field, message: T) {
+unconstrained fn emit_unencrypted_log(event_selector: Field, message: [Field]) {
     emit_unencrypted_log_opcode(event_selector, message)
 }
 unconstrained fn l1_to_l2_msg_exists(msg_hash: Field, msg_leaf_index: Field) -> u8 {
@@ -319,7 +320,7 @@ fn nullifier_exists_opcode(nullifier: Field, address: Field) -> u8 {}
 fn emit_nullifier_opcode(nullifier: Field) {}
 
 #[oracle(amvOpcodeEmitUnencryptedLog)]
-fn emit_unencrypted_log_opcode<T>(event_selector: Field, message: T) {}
+fn emit_unencrypted_log_opcode(event_selector: Field, message: [Field]) {}
 
 #[oracle(avmOpcodeL1ToL2MsgExists)]
 fn l1_to_l2_msg_exists_opcode(msg_hash: Field, msg_leaf_index: Field) -> u8 {}
@@ -367,4 +368,4 @@ impl<N> FunctionReturns<N> {
     pub fn deserialize_into<T>(self) -> T where T: Deserialize<N> {
         Deserialize::deserialize(self.raw())
     }
-}
+}

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

@@ -300,10 +300,10 @@ contract AvmTest {
 
     #[aztec(public)]
     fn emit_unencrypted_log() {
-        context.emit_unencrypted_log_with_selector(/*event_selector=*/ 5, /*message=*/ [10, 20, 30]);
-        context.emit_unencrypted_log_with_selector(/*event_selector=*/ 8, /*message=*/ "Hello, world!");
+        context.emit_unencrypted_log(/*message=*/ [10, 20, 30]);
+        context.emit_unencrypted_log(/*message=*/ "Hello, world!");
         let s: CompressedString<2,44> = CompressedString::from_string("A long time ago, in a galaxy far far away...");
-        context.emit_unencrypted_log_with_selector(/*event_selector=*/ 10, /*message=*/ s);
+        context.emit_unencrypted_log(/*message=*/ s);
     }
 
     #[aztec(public)]

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

@@ -32,7 +32,6 @@ contract StaticChild {
     fn pub_set_value(new_value: Field) -> Field {
         storage.current_value.write(new_value);
         context.emit_unencrypted_log(new_value);
-
         new_value
     }
 
@@ -86,7 +85,6 @@ contract StaticChild {
         let old_value = storage.current_value.read();
         storage.current_value.write(old_value + new_value);
         context.emit_unencrypted_log(new_value);
-
         new_value
     }
 
@@ -97,7 +95,6 @@ contract StaticChild {
         let old_value = storage.current_value.read();
         storage.current_value.write(old_value + new_value);
         context.emit_unencrypted_log(new_value);
-
         new_value
     }
 }

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

@@ -343,11 +343,12 @@ contract Test {
     // docs:end:is-time-equal
 
     #[aztec(public)]
-    fn emit_unencrypted(value: Field) -> Field {
+    fn emit_unencrypted(value: Field) {
         // docs:start:emit_unencrypted
-        context.emit_unencrypted_log(value);
+        context.emit_unencrypted_log(/*message=*/ value);
+        context.emit_unencrypted_log(/*message=*/ [10, 20, 30]);
+        context.emit_unencrypted_log(/*message=*/ "Hello, world!");
         // docs:end:emit_unencrypted
-        0
     }
 
     #[aztec(public)]

noir-projects/noir-protocol-circuits/crates/types/src/traits.nr

@@ -90,6 +90,11 @@ trait Serialize<N> {
 }
 // docs:end:serialize
 
+impl<N> Serialize<N> for [Field; N] {
+    fn serialize(self) -> [Field; N] {
+        self
+    }
+}
 impl<N> Serialize<N> for str<N> {
     fn serialize(self) -> [Field; N] {
         let mut result = [0; N];

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

@@ -347,10 +347,10 @@ describe('AVM simulator: transpiled Noir contracts', () => {
         ),
         new UnencryptedL2Log(
           context.environment.address,
-          new EventSelector(8),
+          new EventSelector(5),
           Buffer.concat(expectedString.map(f => f.toBuffer())),
         ),
-        new UnencryptedL2Log(context.environment.address, new EventSelector(10), expectedCompressedString),
+        new UnencryptedL2Log(context.environment.address, new EventSelector(5), expectedCompressedString),
       ]);
     });
 

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

@@ -6,7 +6,7 @@ import { mock } from 'jest-mock-extended';
 
 import { type CommitmentsDB } from '../../index.js';
 import { type AvmContext } from '../avm_context.js';
-import { Field, Uint8 } from '../avm_memory_types.js';
+import { Field, Uint8, Uint32 } from '../avm_memory_types.js';
 import { InstructionExecutionError, StaticCallAlterationError } from '../errors.js';
 import { initContext, initExecutionEnvironment, initHostStorage } from '../fixtures/index.js';
 import { AvmPersistableStateManager } from '../journal/journal.js';
@@ -392,14 +392,14 @@ describe('Accrued Substate', () => {
         EmitUnencryptedLog.opcode, // opcode
         0x01, // indirect
         ...Buffer.from('02345678', 'hex'), // event selector offset
-        ...Buffer.from('12345678', 'hex'), // offset
-        ...Buffer.from('a2345678', 'hex'), // length
+        ...Buffer.from('12345678', 'hex'), // log offset
+        ...Buffer.from('a2345678', 'hex'), // length offset
       ]);
       const inst = new EmitUnencryptedLog(
         /*indirect=*/ 0x01,
         /*eventSelectorOffset=*/ 0x02345678,
         /*offset=*/ 0x12345678,
-        /*length=*/ 0xa2345678,
+        /*lengthOffset=*/ 0xa2345678,
       );
 
       expect(EmitUnencryptedLog.deserialize(buf)).toEqual(inst);
@@ -410,16 +410,18 @@ describe('Accrued Substate', () => {
       const startOffset = 0;
       const eventSelector = 5;
       const eventSelectorOffset = 10;
+      const logSizeOffset = 20;
 
       const values = [new Field(69n), new Field(420n), new Field(Field.MODULUS - 1n)];
       context.machineState.memory.setSlice(startOffset, values);
       context.machineState.memory.set(eventSelectorOffset, new Field(eventSelector));
+      context.machineState.memory.set(logSizeOffset, new Uint32(values.length));
 
       await new EmitUnencryptedLog(
         /*indirect=*/ 0,
         eventSelectorOffset,
         /*offset=*/ startOffset,
-        values.length,
+        logSizeOffset,
       ).execute(context);
 
       const journalState = context.persistableState.flush();
@@ -472,11 +474,12 @@ describe('Accrued Substate', () => {
 
   it('All substate emission instructions should fail within a static call', async () => {
     context = initContext({ env: initExecutionEnvironment({ isStaticCall: true }) });
+    context.machineState.memory.set(0, new Field(2020n));
 
     const instructions = [
       new EmitNoteHash(/*indirect=*/ 0, /*offset=*/ 0),
       new EmitNullifier(/*indirect=*/ 0, /*offset=*/ 0),
-      new EmitUnencryptedLog(/*indirect=*/ 0, /*eventSelector=*/ 0, /*offset=*/ 0, /*logSize=*/ 1),
+      new EmitUnencryptedLog(/*indirect=*/ 0, /*eventSelector=*/ 0, /*offset=*/ 0, /*logSizeOffset=*/ 0),
       new SendL2ToL1Message(/*indirect=*/ 0, /*recipientOffset=*/ 0, /*contentOffset=*/ 1),
     ];