Skip to content

Commit

Permalink
feat: remove event selector in logs from public context (#7192)
Browse files Browse the repository at this point in the history
Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.
  • Loading branch information
sklppy88 authored Jun 26, 2024
1 parent 431c14c commit 646d45a
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 79 deletions.
19 changes: 6 additions & 13 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ fn handle_foreign_call(
"avmOpcodeStaticCall" => {
handle_external_call(avm_instrs, destinations, inputs, AvmOpcode::STATICCALL);
}
"amvOpcodeEmitUnencryptedLog" => {
"avmOpcodeEmitUnencryptedLog" => {
handle_emit_unencrypted_log(avm_instrs, destinations, inputs);
}
"avmOpcodeNoteHashExists" => handle_note_hash_exists(avm_instrs, destinations, inputs),
Expand Down Expand Up @@ -435,32 +435,25 @@ fn handle_emit_unencrypted_log(
destinations: &Vec<ValueOrArray>,
inputs: &Vec<ValueOrArray>,
) {
if !destinations.is_empty() || inputs.len() != 3 {
if !destinations.is_empty() || inputs.len() != 2 {
panic!(
"Transpiler expects ForeignCall::EMITUNENCRYPTEDLOG to have 0 destinations and 3 inputs, got {} and {}",
"Transpiler expects ForeignCall::EMITUNENCRYPTEDLOG to have 0 destinations and 2 inputs, got {} and {}",
destinations.len(),
inputs.len()
);
}
let event_offset = match &inputs[0] {
ValueOrArray::MemoryAddress(offset) => offset.to_usize() as u32,
_ => panic!(
"Unexpected inputs[0] (event) for ForeignCall::EMITUNENCRYPTEDLOG: {:?}",
inputs[0]
),
};

// 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] {
let (message_offset, message_size_offset) = match &inputs[1] {
ValueOrArray::HeapVector(vec) => (vec.pointer.to_usize() as u32, vec.size.0 as u32),
_ => panic!("Unexpected inputs for ForeignCall::EMITUNENCRYPTEDLOG: {:?}", inputs),
};
avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::EMITUNENCRYPTEDLOG,
// The message array from Brillig is indirect.
indirect: Some(FIRST_OPERAND_INDIRECT),
indirect: Some(ZEROTH_OPERAND_INDIRECT),
operands: vec![
AvmOperand::U32 { value: event_offset },
AvmOperand::U32 { value: message_offset },
AvmOperand::U32 { value: message_size_offset },
],
Expand Down
20 changes: 6 additions & 14 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,13 @@ impl PublicContext {
}
/**
* Emit a log with the given event selector and message.
*
* @param event_selector The event selector for the log.
* @param message The message to emit in the 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);
emit_unencrypted_log(Serialize::serialize(log).as_slice());
}

pub fn note_hash_exists(self, note_hash: Field, leaf_index: Field) -> bool {
note_hash_exists(note_hash, leaf_index) == 1
}
Expand Down Expand Up @@ -241,8 +233,8 @@ unconstrained fn nullifier_exists(nullifier: Field, address: Field) -> u8 {
unconstrained fn emit_nullifier(nullifier: Field) {
emit_nullifier_opcode(nullifier)
}
unconstrained fn emit_unencrypted_log(event_selector: Field, message: [Field]) {
emit_unencrypted_log_opcode(event_selector, message)
unconstrained fn emit_unencrypted_log(message: [Field]) {
emit_unencrypted_log_opcode(message)
}
unconstrained fn l1_to_l2_msg_exists(msg_hash: Field, msg_leaf_index: Field) -> u8 {
l1_to_l2_msg_exists_opcode(msg_hash, msg_leaf_index)
Expand Down Expand Up @@ -325,8 +317,8 @@ unconstrained fn nullifier_exists_opcode(nullifier: Field, address: Field) -> u8
#[oracle(avmOpcodeEmitNullifier)]
unconstrained fn emit_nullifier_opcode(nullifier: Field) {}

#[oracle(amvOpcodeEmitUnencryptedLog)]
unconstrained fn emit_unencrypted_log_opcode(event_selector: Field, message: [Field]) {}
#[oracle(avmOpcodeEmitUnencryptedLog)]
unconstrained fn emit_unencrypted_log_opcode(message: [Field]) {}

#[oracle(avmOpcodeL1ToL2MsgExists)]
unconstrained fn l1_to_l2_msg_exists_opcode(msg_hash: Field, msg_leaf_index: Field) -> u8 {}
Expand Down
7 changes: 3 additions & 4 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const results = await new AvmSimulator(context).executeBytecode(bytecode);
expect(results.reverted).toBe(false);

const eventSelector = new Fr(5);
const expectedFields = [new Fr(10), new Fr(20), new Fr(30)];
const expectedString = 'Hello, world!'.split('').map(c => new Fr(c.charCodeAt(0)));
const expectedCompressedString = [
Expand All @@ -567,9 +566,9 @@ describe('AVM simulator: transpiled Noir contracts', () => {
].map(s => new Fr(Buffer.from(s)));

expect(trace.traceUnencryptedLog).toHaveBeenCalledTimes(3);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, eventSelector, expectedFields);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, eventSelector, expectedString);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, eventSelector, expectedCompressedString);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, expectedFields);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, expectedString);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, expectedCompressedString);
});
});

Expand Down
6 changes: 3 additions & 3 deletions yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ export class AvmPersistableStateManager {
* @param event - log event selector
* @param log - log contents
*/
public writeUnencryptedLog(contractAddress: Fr, event: Fr, log: Fr[]) {
this.log.debug(`UnencryptedL2Log(${contractAddress}) += event ${event} with ${log.length} fields.`);
this.trace.traceUnencryptedLog(contractAddress, event, log);
public writeUnencryptedLog(contractAddress: Fr, log: Fr[]) {
this.log.debug(`UnencryptedL2Log(${contractAddress}) += event with ${log.length} fields.`);
this.trace.traceUnencryptedLog(contractAddress, log);
}

/**
Expand Down
22 changes: 4 additions & 18 deletions yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,44 +305,30 @@ describe('Accrued Substate', () => {
const buf = Buffer.from([
EmitUnencryptedLog.opcode, // opcode
0x01, // indirect
...Buffer.from('02345678', 'hex'), // event selector offset
...Buffer.from('12345678', 'hex'), // log offset
...Buffer.from('a2345678', 'hex'), // length offset
]);
const inst = new EmitUnencryptedLog(
/*indirect=*/ 0x01,
/*eventSelectorOffset=*/ 0x02345678,
/*offset=*/ 0x12345678,
/*lengthOffset=*/ 0xa2345678,
);
const inst = new EmitUnencryptedLog(/*indirect=*/ 0x01, /*offset=*/ 0x12345678, /*lengthOffset=*/ 0xa2345678);

expect(EmitUnencryptedLog.deserialize(buf)).toEqual(inst);
expect(inst.serialize()).toEqual(buf);
});

it('Should append unencrypted logs correctly', async () => {
const startOffset = 0;
const eventSelector = new Fr(5);
const eventSelectorOffset = 10;
const logSizeOffset = 20;

const values = [new Fr(69n), new Fr(420n), new Fr(Fr.MODULUS - 1n)];
context.machineState.memory.setSlice(
startOffset,
values.map(f => new Field(f)),
);
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,
logSizeOffset,
).execute(context);
await new EmitUnencryptedLog(/*indirect=*/ 0, /*offset=*/ startOffset, logSizeOffset).execute(context);

expect(trace.traceUnencryptedLog).toHaveBeenCalledTimes(1);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, eventSelector, values);
expect(trace.traceUnencryptedLog).toHaveBeenCalledWith(address, values);
});
});

Expand Down Expand Up @@ -386,7 +372,7 @@ describe('Accrued Substate', () => {
const instructions = [
new EmitNoteHash(/*indirect=*/ 0, /*offset=*/ 0),
new EmitNullifier(/*indirect=*/ 0, /*offset=*/ 0),
new EmitUnencryptedLog(/*indirect=*/ 0, /*eventSelector=*/ 0, /*offset=*/ 0, /*logSizeOffset=*/ 0),
new EmitUnencryptedLog(/*indirect=*/ 0, /*offset=*/ 0, /*logSizeOffset=*/ 0),
new SendL2ToL1Message(/*indirect=*/ 0, /*recipientOffset=*/ 0, /*contentOffset=*/ 1),
];

Expand Down
25 changes: 6 additions & 19 deletions yarn-project/simulator/src/avm/opcodes/accrued_substate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,20 +217,9 @@ export class EmitUnencryptedLog extends Instruction {
static type: string = 'EMITUNENCRYPTEDLOG';
static readonly opcode: Opcode = Opcode.EMITUNENCRYPTEDLOG;
// Informs (de)serialization. See Instruction.deserialize.
static readonly wireFormat = [
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT32,
OperandType.UINT32,
OperandType.UINT32,
];
static readonly wireFormat = [OperandType.UINT8, OperandType.UINT8, OperandType.UINT32, OperandType.UINT32];

constructor(
private indirect: number,
private eventSelectorOffset: number,
private logOffset: number,
private logSizeOffset: number,
) {
constructor(private indirect: number, private logOffset: number, private logSizeOffset: number) {
super();
}

Expand All @@ -241,22 +230,20 @@ export class EmitUnencryptedLog extends Instruction {

const memory = context.machineState.memory.track(this.type);

const [eventSelectorOffset, logOffset, logSizeOffset] = Addressing.fromWire(this.indirect).resolve(
[this.eventSelectorOffset, this.logOffset, this.logSizeOffset],
const [logOffset, logSizeOffset] = Addressing.fromWire(this.indirect).resolve(
[this.logOffset, this.logSizeOffset],
memory,
);
memory.checkTag(TypeTag.FIELD, eventSelectorOffset);
memory.checkTag(TypeTag.UINT32, logSizeOffset);
const logSize = memory.get(logSizeOffset).toNumber();
memory.checkTagsRange(TypeTag.FIELD, logOffset, logSize);

const contractAddress = context.environment.address;
const event = memory.get(eventSelectorOffset).toFr();

const memoryOperations = { reads: 2 + logSize, indirect: this.indirect };
const memoryOperations = { reads: 1 + logSize, indirect: this.indirect };
context.machineState.consumeGas(this.gasCost(memoryOperations));
const log = memory.getSlice(logOffset, logSize).map(f => f.toFr());
context.persistableState.writeUnencryptedLog(contractAddress, event, log);
context.persistableState.writeUnencryptedLog(contractAddress, log);

memory.assert(memoryOperations);
context.machineState.incrementPc();
Expand Down
8 changes: 3 additions & 5 deletions yarn-project/simulator/src/public/side_effect_trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { SerializableContractInstance } from '@aztec/types/contracts';

import { randomBytes, randomInt } from 'crypto';

import { Selector } from '../../../foundation/src/abi/selector.js';
import { AvmContractCallResults } from '../avm/avm_message_call_result.js';
import { initExecutionEnvironment } from '../avm/fixtures/index.js';
import { PublicSideEffectTrace, type TracedContractInstance } from './side_effect_trace.js';
Expand All @@ -25,7 +24,6 @@ describe('Side Effect Trace', () => {
const value = Fr.random();
const recipient = Fr.random();
const content = Fr.random();
const event = new Fr(randomBytes(Selector.SIZE).readUint32BE());
const log = [Fr.random(), Fr.random(), Fr.random()];

const startGasLeft = Gas.fromFields([new Fr(randomInt(10000)), new Fr(randomInt(10000))]);
Expand Down Expand Up @@ -205,13 +203,13 @@ describe('Side Effect Trace', () => {
});

it('Should trace new unencrypted logs', () => {
trace.traceUnencryptedLog(address, event, log);
trace.traceUnencryptedLog(address, log);
expect(trace.getCounter()).toBe(startCounterPlus1);

const pxResult = toPxResult(trace);
const expectLog = new UnencryptedL2Log(
AztecAddress.fromField(address),
EventSelector.fromField(event),
EventSelector.fromField(new Fr(0)),
Buffer.concat(log.map(f => f.toBuffer())),
);
expect(pxResult.unencryptedLogs.logs).toEqual([expectLog]);
Expand Down Expand Up @@ -266,7 +264,7 @@ describe('Side Effect Trace', () => {
testCounter++;
nestedTrace.traceNewL2ToL1Message(recipient, content);
testCounter++;
nestedTrace.traceUnencryptedLog(address, event, log);
nestedTrace.traceUnencryptedLog(address, log);
testCounter++;

trace.traceNestedCall(nestedTrace, avmEnvironment, startGasLeft, endGasLeft, bytecode, avmCallResults);
Expand Down
5 changes: 3 additions & 2 deletions yarn-project/simulator/src/public/side_effect_trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,12 @@ export class PublicSideEffectTrace implements PublicSideEffectTraceInterface {
this.incrementSideEffectCounter();
}

public traceUnencryptedLog(contractAddress: Fr, event: Fr, log: Fr[]) {
public traceUnencryptedLog(contractAddress: Fr, log: Fr[]) {
// TODO(4805): check if some threshold is reached for max logs
const ulog = new UnencryptedL2Log(
AztecAddress.fromField(contractAddress),
EventSelector.fromField(event),
// TODO(#7198): Remove event selector from UnencryptedL2Log
EventSelector.fromField(new Fr(0)),
Buffer.concat(log.map(f => f.toBuffer())),
);
const basicLogHash = Fr.fromBuffer(ulog.hash());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface PublicSideEffectTraceInterface {
traceL1ToL2MessageCheck(contractAddress: Fr, msgHash: Fr, msgLeafIndex: Fr, exists: boolean): void;
// TODO(dbanks12): should new message accept contract address as arg?
traceNewL2ToL1Message(recipient: Fr, content: Fr): void;
traceUnencryptedLog(contractAddress: Fr, event: Fr, log: Fr[]): void;
traceUnencryptedLog(contractAddress: Fr, log: Fr[]): void;
// TODO(dbanks12): odd that getContractInstance is a one-off in that it accepts an entire object instead of components
traceGetContractInstance(instance: TracedContractInstance): void;
traceNestedCall(
Expand Down

0 comments on commit 646d45a

Please sign in to comment.