From 646d45a0cb92634909fb38d0478181c8d1d814af Mon Sep 17 00:00:00 2001 From: esau <152162806+sklppy88@users.noreply.github.com> Date: Wed, 26 Jun 2024 19:49:42 +0200 Subject: [PATCH] feat: remove event selector in logs from public context (#7192) Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. --- avm-transpiler/src/transpile.rs | 19 +++++--------- .../aztec/src/context/public_context.nr | 20 +++++---------- .../simulator/src/avm/avm_simulator.test.ts | 7 +++--- .../simulator/src/avm/journal/journal.ts | 6 ++--- .../src/avm/opcodes/accrued_substate.test.ts | 22 +++------------- .../src/avm/opcodes/accrued_substate.ts | 25 +++++-------------- .../src/public/side_effect_trace.test.ts | 8 +++--- .../simulator/src/public/side_effect_trace.ts | 5 ++-- .../src/public/side_effect_trace_interface.ts | 2 +- 9 files changed, 35 insertions(+), 79 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index f1fdd201b8b..22e5cbcd8ca 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -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), @@ -435,32 +435,25 @@ fn handle_emit_unencrypted_log( destinations: &Vec, inputs: &Vec, ) { - 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 }, ], diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr index 979ddeb2b55..87b17aa92d2 100644 --- a/noir-projects/aztec-nr/aztec/src/context/public_context.nr +++ b/noir-projects/aztec-nr/aztec/src/context/public_context.nr @@ -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( - &mut self, - event_selector: Field, - log: T - ) where T: Serialize { - 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(&mut self, log: T) where T: Serialize { - 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 } @@ -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) @@ -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 {} diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index e28d9d9b8e7..7fb9ef31bb8 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -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 = [ @@ -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); }); }); diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index d1136a718e4..08c3698c581 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -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); } /** diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts index 9f71a34a6f6..54ad83daedc 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts @@ -305,16 +305,10 @@ 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); @@ -322,8 +316,6 @@ describe('Accrued Substate', () => { 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)]; @@ -331,18 +323,12 @@ describe('Accrued Substate', () => { 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); }); }); @@ -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), ]; diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts index 97a21cf1440..6be6d2d8192 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts @@ -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(); } @@ -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(); diff --git a/yarn-project/simulator/src/public/side_effect_trace.test.ts b/yarn-project/simulator/src/public/side_effect_trace.test.ts index fbfb42b2e5f..cce11329fd2 100644 --- a/yarn-project/simulator/src/public/side_effect_trace.test.ts +++ b/yarn-project/simulator/src/public/side_effect_trace.test.ts @@ -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'; @@ -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))]); @@ -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]); @@ -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); diff --git a/yarn-project/simulator/src/public/side_effect_trace.ts b/yarn-project/simulator/src/public/side_effect_trace.ts index 74cdcfe4517..72d253dd5da 100644 --- a/yarn-project/simulator/src/public/side_effect_trace.ts +++ b/yarn-project/simulator/src/public/side_effect_trace.ts @@ -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()); diff --git a/yarn-project/simulator/src/public/side_effect_trace_interface.ts b/yarn-project/simulator/src/public/side_effect_trace_interface.ts index 60dd0b1107d..a44eecbca49 100644 --- a/yarn-project/simulator/src/public/side_effect_trace_interface.ts +++ b/yarn-project/simulator/src/public/side_effect_trace_interface.ts @@ -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(