Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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()
});
Expand Down Expand Up @@ -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 },
Expand Down
27 changes: 14 additions & 13 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {}
Expand Down Expand Up @@ -367,4 +368,4 @@ impl<N> FunctionReturns<N> {
pub fn deserialize_into<T>(self) -> T where T: Deserialize<N> {
Deserialize::deserialize(self.raw())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grego was ok with this.

Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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),
];

Expand Down
Loading
Loading