Skip to content

Commit

Permalink
chore(avm): add tag checking and missing indirects
Browse files Browse the repository at this point in the history
UINT32 memory offsets and length checks are commented out until Noir
uses UINT32 for memory.
  • Loading branch information
fcarreiro committed Jun 6, 2024
1 parent 06f03fd commit 2c8451b
Show file tree
Hide file tree
Showing 21 changed files with 227 additions and 109 deletions.
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
18 changes: 18 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,24 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading]

Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them.

## 0.43.0

### [Aztec.nr] Public `emit_unencrypted_log`

The `emit_unencrypted_log` function in the `PublicContext` now takes a slice of fields (`[Field]`). There's also a new `emit_unencrypted_log_string<N>(str<N>)` method for convenience when logging a string.

```diff
#[aztec(public)]
fn emit_unencrypted(value: Field) {
- context.emit_unencrypted_log(/*message=*/ value);
+ context.emit_unencrypted_log(/*message=*/ &[value]);
- context.emit_unencrypted_log(/*message=*/ [10, 20, 30]);
+ context.emit_unencrypted_log(/*message=*/ &[10, 20, 30]);
- context.emit_unencrypted_log(/*message=*/ "Hello, world!");
+ context.emit_unencrypted_log_string(/*message=*/ "Hello, world!");
}
```

## 0.42.0

### [Aztec.nr] Unconstrained Context
Expand Down
28 changes: 20 additions & 8 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,26 @@ impl PublicContext {
* 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) {
pub fn emit_unencrypted_log_with_selector(&mut self, event_selector: Field, log: [Field]) {
emit_unencrypted_log(event_selector, log);
}
pub fn emit_unencrypted_log_string_with_selector<N>(
&mut self,
event_selector: Field,
log: str<N>
) {
// Ideally we'd use CompressedString but we cannot calculate the output size
// without generic arithmetic.
let log_fields: [Field; N] = log.as_bytes().map(|byte: u8| byte as Field);
emit_unencrypted_log(event_selector, log_fields.as_slice());
}
// These two are for compatibility with the selector-less API.
pub fn emit_unencrypted_log(&mut self, log: [Field]) {
self.emit_unencrypted_log_with_selector(/*event_selector=*/ 5, log);
}
pub fn emit_unencrypted_log_string<N>(&mut self, log: str<N>) {
self.emit_unencrypted_log_string_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 +74,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 +246,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 +331,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
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_string(/*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.serialize().as_slice());
}

#[aztec(public)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ contract Benchmarking {
// Emits a public log.
#[aztec(public)]
fn broadcast(owner: AztecAddress) {
context.emit_unencrypted_log(storage.balances.at(owner).read());
context.emit_unencrypted_log(&[storage.balances.at(owner).read()]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract Child {
#[aztec(public)]
fn pub_set_value(new_value: Field) -> Field {
storage.current_value.write(new_value);
context.emit_unencrypted_log(new_value);
context.emit_unencrypted_log(&[new_value]);

new_value
}
Expand Down Expand Up @@ -74,7 +74,7 @@ contract Child {
fn pub_inc_value(new_value: Field) -> Field {
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
context.emit_unencrypted_log(new_value);
context.emit_unencrypted_log(&[new_value]);

new_value
}
Expand All @@ -85,7 +85,7 @@ contract Child {
fn pub_inc_value_internal(new_value: Field) -> Field {
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
context.emit_unencrypted_log(new_value);
context.emit_unencrypted_log(&[new_value]);

new_value
}
Expand All @@ -94,13 +94,13 @@ contract Child {
fn set_value_twice_with_nested_first() {
let _result = Child::at(context.this_address()).pub_set_value(10).call(&mut context);
storage.current_value.write(20);
context.emit_unencrypted_log(20);
context.emit_unencrypted_log(&[20]);
}

#[aztec(public)]
fn set_value_twice_with_nested_last() {
storage.current_value.write(20);
context.emit_unencrypted_log(20);
context.emit_unencrypted_log(&[20]);
let _result = Child::at(context.this_address()).pub_set_value(10).call(&mut context);
}

Expand All @@ -111,6 +111,6 @@ contract Child {
Child::at(context.this_address()).set_value_twice_with_nested_first().call(&mut context);
Child::at(context.this_address()).set_value_twice_with_nested_last().call(&mut context);
storage.current_value.write(20);
context.emit_unencrypted_log(20);
context.emit_unencrypted_log(&[20]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ contract StaticChild {
#[aztec(public)]
fn pub_set_value(new_value: Field) -> Field {
storage.current_value.write(new_value);
context.emit_unencrypted_log(new_value);

context.emit_unencrypted_log(&[new_value]);
new_value
}

Expand Down Expand Up @@ -85,8 +84,7 @@ contract StaticChild {
fn pub_inc_value(new_value: Field) -> Field {
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
context.emit_unencrypted_log(new_value);

context.emit_unencrypted_log(&[new_value]);
new_value
}

Expand All @@ -96,8 +94,7 @@ contract StaticChild {
fn pub_illegal_inc_value(new_value: Field) -> Field {
let old_value = storage.current_value.read();
storage.current_value.write(old_value + new_value);
context.emit_unencrypted_log(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_string(/*message=*/ "Hello, world!");
// docs:end:emit_unencrypted
0
}

#[aztec(public)]
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
15 changes: 9 additions & 6 deletions yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts
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

0 comments on commit 2c8451b

Please sign in to comment.