fix(avm): fix unencryptedlog c++ deser (#7194)

Note that I'm removing eventselector from the deserialization, because Esau has a PR open where he's removing it on the TS side. Note also that the constraints/witgen for this opcode are still broken.
AztecProtocol · Jun 26, 2024 · 89a99af · 89a99af
1 parent 9be0ad6
commit 89a99af
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 25 deletions.
diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_deserialization.cpp
@@ -67,9 +67,22 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
 
     { OpCode::GETCONTRACTINSTANCE, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } },
     // TODO: ordering inline with spec
-    { OpCode::EMITNOTEHASH, getter_format },  // TODO: new format for these
-    { OpCode::EMITNULLIFIER, getter_format }, // TODO: new format for these
-    { OpCode::EMITUNENCRYPTEDLOG, getter_format },
+    { OpCode::EMITNOTEHASH,
+      {
+          OperandType::INDIRECT,
+          OperandType::UINT32,
+      } }, // TODO: new format for these
+    { OpCode::EMITNULLIFIER,
+      {
+          OperandType::INDIRECT,
+          OperandType::UINT32,
+      } }, // TODO: new format for these
+    { OpCode::EMITUNENCRYPTEDLOG,
+      {
+          OperandType::INDIRECT,
+          OperandType::UINT32,
+          OperandType::UINT32,
+      } },
     { OpCode::SENDL2TOL1MSG, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32 } },
     { OpCode::SLOAD, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } },
     { OpCode::SSTORE, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } },

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp
@@ -533,7 +533,8 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
             break;
         case OpCode::EMITUNENCRYPTEDLOG:
             trace_builder.op_emit_unencrypted_log(std::get<uint8_t>(inst.operands.at(0)),
-                                                  std::get<uint32_t>(inst.operands.at(1)));
+                                                  std::get<uint32_t>(inst.operands.at(1)),
+                                                  std::get<uint32_t>(inst.operands.at(2)));
             break;
         case OpCode::SENDL2TOL1MSG:
             trace_builder.op_emit_l2_to_l1_msg(std::get<uint8_t>(inst.operands.at(0)),

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp
@@ -1504,10 +1504,14 @@ void AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipient_
     side_effect_counter++;
 }
 
-void AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, uint32_t log_offset)
+void AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect,
+                                              uint32_t log_offset,
+                                              [[maybe_unused]] uint32_t log_size_offset)
 {
     auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
 
+    // FIXME: read (and constrain) log_size_offset
+    // FIXME: we need to constrain the log_size_offset mem read (and tag check), not just one field!
     Row row = create_kernel_output_opcode(indirect, clk, log_offset);
     kernel_trace_builder.op_emit_unencrypted_log(clk, side_effect_counter, row.main_ia);
     row.main_sel_op_emit_unencrypted_log = FF(1);

diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp
@@ -116,7 +116,7 @@ class AvmTraceBuilder {
     // With single output values
     void op_emit_note_hash(uint8_t indirect, uint32_t note_hash_offset);
     void op_emit_nullifier(uint8_t indirect, uint32_t nullifier_offset);
-    void op_emit_unencrypted_log(uint8_t indirect, uint32_t log_offset);
+    void op_emit_unencrypted_log(uint8_t indirect, uint32_t log_offset, uint32_t log_size_offset);
     void op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t msg_offset, uint32_t recipient_offset);
     void op_get_contract_instance(uint8_t indirect, uint32_t address_offset, uint32_t dst_offset);
 

diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp
@@ -1650,6 +1650,7 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes)
                                + to_hex(OpCode::EMITUNENCRYPTEDLOG) + // opcode EMITUNENCRYPTEDLOG
                                "00"                                   // Indirect flag
                                "00000001"                             // src offset 1
+                               "00000002"                             // src size offset
                                + to_hex(OpCode::SENDL2TOL1MSG) +      // opcode SENDL2TOL1MSG
                                "00"                                   // Indirect flag
                                "00000001"                             // src offset 1

diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_kernel.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_kernel.test.cpp
@@ -1022,12 +1022,12 @@ TEST_F(AvmKernelOutputPositiveTests, kernelEmitUnencryptedLog)
     // We write the note hash into memory
     auto direct_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
         trace_builder.op_set(0, 1234, direct_offset, AvmMemoryTag::FF);
-        trace_builder.op_emit_unencrypted_log(/*indirect=*/false, direct_offset);
+        trace_builder.op_emit_unencrypted_log(/*indirect=*/false, direct_offset, /*log_size_offset=*/0);
     };
     auto indirect_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
         trace_builder.op_set(0, 1234, direct_offset, AvmMemoryTag::FF);
         trace_builder.op_set(0, direct_offset, indirect_offset, AvmMemoryTag::U32);
-        trace_builder.op_emit_unencrypted_log(/*indirect=*/true, indirect_offset);
+        trace_builder.op_emit_unencrypted_log(/*indirect=*/true, indirect_offset, /*log_size_offset=*/0);
     };
 
     auto checks = [=](bool indirect, const std::vector<Row>& trace) {

diff --git a/docs/docs/protocol-specs/public-vm/gen/_instruction-set.mdx b/docs/docs/protocol-specs/public-vm/gen/_instruction-set.mdx
@@ -434,8 +434,7 @@ if exists:
 {`context.accruedSubstate.unencryptedLogs.append(
     UnencryptedLog {
         address: context.environment.address,
-        eventSelector: M[eventSelectorOffset],
-        log: M[logOffset:logOffset+logSize],
+        log: M[logOffset:logOffset+M[logSizeOffset]],
     }
 )`}
 	</CodeBlock></td>
@@ -1605,20 +1604,18 @@ Emit an unencrypted log
 - **Flags**: 
 	- **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`.
 - **Args**: 
-	- **eventSelectorOffset**: memory offset of the event selector
 	- **logOffset**: memory offset of the data to log
-	- **logSize**: number of words to log
+	- **logSizeOffset**: memory offset to number of words to log
 - **Expression**: 
 <CodeBlock language="jsx">
 {`context.accruedSubstate.unencryptedLogs.append(
     UnencryptedLog {
         address: context.environment.address,
-        eventSelector: M[eventSelectorOffset],
-        log: M[logOffset:logOffset+logSize],
+        log: M[logOffset:logOffset+M[logSizeOffset]],
     }
 )`}
 </CodeBlock>
-- **Bit-size**: 120
+- **Bit-size**: 88
 
 [![](/img/protocol-specs/public-vm/bit-formats/EMITUNENCRYPTEDLOG.png)](/img/protocol-specs/public-vm/bit-formats/EMITUNENCRYPTEDLOG.png)
 

diff --git a/docs/src/preprocess/InstructionSet/InstructionSet.js b/docs/src/preprocess/InstructionSet/InstructionSet.js
@@ -1343,24 +1343,17 @@ M[dstOffset:dstOffset+CONTRACT_INSTANCE_SIZE+1] = [
     Category: "Accrued Substate - Logging",
     Flags: [{ name: "indirect", description: INDIRECT_FLAG_DESCRIPTION }],
     Args: [
-      {
-        name: "eventSelectorOffset",
-        description: "memory offset of the event selector",
-      },
       { name: "logOffset", description: "memory offset of the data to log" },
       {
-        name: "logSize",
-        description: "number of words to log",
-        mode: "immediate",
-        type: "u32",
+        name: "logSizeOffset",
+        description: "memory offset to number of words to log",
       },
     ],
     Expression: `
 context.accruedSubstate.unencryptedLogs.append(
     UnencryptedLog {
         address: context.environment.address,
-        eventSelector: M[eventSelectorOffset],
-        log: M[logOffset:logOffset+logSize],
+        log: M[logOffset:logOffset+M[logSizeOffset]],
     }
 )
 `,