fix: unencryptedlogs witgen (#8669)

Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.

barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp

@@ -1831,15 +1831,14 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithIncorrectNumberOfPublicInputs)
 TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes)
 {
     // Set values into the first register to emit
-    std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode Set
-                               "00"                    // Indirect flag
-                               + to_hex(AvmMemoryTag::U32) +
-                               "01" // value 1
-                               "01" // dst_offset 1
-                               // Cast set to field
-                               + to_hex(OpCode::CAST_8) + // opcode CAST
-                               "00"                       // Indirect flag
-                               + to_hex(AvmMemoryTag::FF) +
+    std::string bytecode_hex = to_hex(OpCode::SET_8) +                // opcode Set
+                               "00"                                   // Indirect flag
+                               + to_hex(AvmMemoryTag::U32) +          // tag U32
+                               "01"                                   // value 1
+                               "01"                                   // dst_offset 1
+                               + to_hex(OpCode::CAST_8) +             // opcode CAST (to field)
+                               "00"                                   // Indirect flag
+                               + to_hex(AvmMemoryTag::FF) +           // tag FF
                                "01"                                   // dst 1
                                "01"                                   // dst 1
                                + to_hex(OpCode::EMITNOTEHASH) +       // opcode EMITNOTEHASH
@@ -1902,13 +1901,15 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes)
     // CHECK EMIT UNENCRYPTED LOG
     auto emit_log_row =
         std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_emit_unencrypted_log == 1; });
-    EXPECT_EQ(emit_log_row->main_ia, 1);
+    // Trust me bro for now, this is the truncated sha output
+    FF expected_hash = FF(std::string("0x006db65fd59fd356f6729140571b5bcd6bb3b83492a16e1bf0a3884442fc3c8a"));
+    EXPECT_EQ(emit_log_row->main_ia, expected_hash);
     EXPECT_EQ(emit_log_row->main_side_effect_counter, 2);
 
     uint32_t emit_log_out_offset = START_EMIT_UNENCRYPTED_LOG_WRITE_OFFSET;
     auto emit_log_kernel_out_row =
         std::ranges::find_if(trace.begin(), trace.end(), [&](Row r) { return r.main_clk == emit_log_out_offset; });
-    EXPECT_EQ(emit_log_kernel_out_row->main_kernel_value_out, 1);
+    EXPECT_EQ(emit_log_kernel_out_row->main_kernel_value_out, expected_hash);
     EXPECT_EQ(emit_log_kernel_out_row->main_kernel_side_effect_out, 2);
     feed_output(emit_log_out_offset, 1, 2, 0);
 

barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp

@@ -367,10 +367,12 @@ VmPublicInputs Execution::convert_public_inputs(std::vector<FF> const& public_in
     // For EMITUNENCRYPTEDLOG
     for (size_t i = 0; i < MAX_UNENCRYPTED_LOGS_PER_CALL; i++) {
         size_t dest_offset = START_EMIT_UNENCRYPTED_LOG_WRITE_OFFSET + i;
-        size_t pcpi_offset = PCPI_NEW_UNENCRYPTED_LOGS_OFFSET + (i * 2);
+        size_t pcpi_offset =
+            PCPI_NEW_UNENCRYPTED_LOGS_OFFSET + (i * 3); // 3 because we have metadata, this is the window size
 
         ko_values[dest_offset] = public_inputs_vec[pcpi_offset];
         ko_side_effect[dest_offset] = public_inputs_vec[pcpi_offset + 1];
+        ko_metadata[dest_offset] = public_inputs_vec[pcpi_offset + 2];
     }
 
     return public_inputs;

barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.cpp

@@ -265,10 +265,13 @@ void AvmKernelTraceBuilder::op_l1_to_l2_msg_exists(uint32_t clk,
     kernel_trace.push_back(entry);
 }
 
-void AvmKernelTraceBuilder::op_emit_unencrypted_log(uint32_t clk, uint32_t side_effect_counter, const FF& log_hash)
+void AvmKernelTraceBuilder::op_emit_unencrypted_log(uint32_t clk,
+                                                    uint32_t side_effect_counter,
+                                                    const FF& log_hash,
+                                                    const FF& log_length)
 {
     uint32_t offset = START_EMIT_UNENCRYPTED_LOG_WRITE_OFFSET + emit_unencrypted_log_offset;
-    perform_kernel_output_lookup(offset, side_effect_counter, log_hash, FF(0));
+    perform_kernel_output_lookup(offset, side_effect_counter, log_hash, log_length);
     emit_unencrypted_log_offset++;
 
     KernelTraceEntry entry = {

barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.hpp

@@ -57,8 +57,8 @@ class AvmKernelTraceBuilder {
     std::unordered_map<uint32_t, uint32_t> kernel_output_selector_counter;
 
     AvmKernelTraceBuilder(uint32_t initial_side_effect_counter, VmPublicInputs public_inputs, ExecutionHints hints)
-        : initial_side_effect_counter(initial_side_effect_counter)
-        , public_inputs(std::move(public_inputs))
+        : public_inputs(std::move(public_inputs))
+        , initial_side_effect_counter(initial_side_effect_counter)
         , hints(std::move(hints))
     {}
 
@@ -92,14 +92,16 @@ class AvmKernelTraceBuilder {
     void op_nullifier_exists(uint32_t clk, uint32_t side_effect_counter, const FF& nullifier, uint32_t result);
     void op_emit_nullifier(uint32_t clk, uint32_t side_effect_counter, const FF& nullifier);
     void op_l1_to_l2_msg_exists(uint32_t clk, uint32_t side_effect_counter, const FF& message, uint32_t result);
-    void op_emit_unencrypted_log(uint32_t clk, uint32_t side_effect_counter, const FF& log_hash);
+    void op_emit_unencrypted_log(uint32_t clk, uint32_t side_effect_counter, const FF& log_hash, const FF& log_length);
     void op_emit_l2_to_l1_msg(uint32_t clk, uint32_t side_effect_counter, const FF& l2_to_l1_msg, const FF& recipient);
 
+    // This is temporarily made public so we can access PIs
+    VmPublicInputs public_inputs;
+
   private:
     std::vector<KernelTraceEntry> kernel_trace;
 
     uint32_t initial_side_effect_counter;
-    VmPublicInputs public_inputs;
     ExecutionHints hints;
 
     // Output index counters

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "barretenberg/common/assert.hpp"
+#include "barretenberg/common/serialize.hpp"
 #include "barretenberg/common/throw_or_abort.hpp"
 #include "barretenberg/crypto/pedersen_commitment/pedersen.hpp"
 #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp"
@@ -2493,18 +2494,67 @@ void AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect,
                                               uint32_t log_offset,
                                               [[maybe_unused]] uint32_t log_size_offset)
 {
+    std::vector<uint8_t> bytes_to_hash;
+
     auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
 
     // FIXME: read (and constrain) log_size_offset
     auto [resolved_log_offset, resolved_log_size_offset] =
         unpack_indirects<2>(indirect, { log_offset, log_size_offset });
-    auto log_size = unconstrained_read_from_memory(resolved_log_size_offset);
 
-    // FIXME: we need to constrain the log_size_offset mem read (and tag check), not just one field!
-    // FIXME: we shouldn't pass resolved_log_offset.offset; we should modify create_kernel_output_opcode to take an
-    // addresswithmode.
-    Row row = create_kernel_output_opcode(indirect, clk, resolved_log_offset.offset);
-    kernel_trace_builder.op_emit_unencrypted_log(clk, side_effect_counter, row.main_ia);
+    FF contract_address = std::get<0>(kernel_trace_builder.public_inputs)[ADDRESS_SELECTOR];
+    std::vector<uint8_t> contract_address_bytes = contract_address.to_buffer();
+
+    // Unencrypted logs are hashed with sha256 and truncated to 31 bytes - and then padded back to 32 bytes
+    bytes_to_hash.insert(bytes_to_hash.end(),
+                         std::make_move_iterator(contract_address_bytes.begin()),
+                         std::make_move_iterator(contract_address_bytes.end()));
+
+    uint32_t log_size = static_cast<uint32_t>(unconstrained_read_from_memory(resolved_log_size_offset));
+    // The size is in fields of 32 bytes, the length used for the hash is in terms of bytes
+    uint32_t num_bytes = log_size * 32;
+    std::vector<uint8_t> log_size_bytes = to_buffer(num_bytes);
+    // Add the log size to the hash to bytes
+    bytes_to_hash.insert(bytes_to_hash.end(),
+                         std::make_move_iterator(log_size_bytes.begin()),
+                         std::make_move_iterator(log_size_bytes.end()));
+
+    // Load the log values from memory
+    // This first read might be indirect which subsequent reads should not be
+    FF initial_direct_addr = resolved_log_offset.mode == AddressingMode::DIRECT
+                                 ? resolved_log_offset.offset
+                                 : unconstrained_read_from_memory(resolved_log_offset.offset);
+
+    AddressWithMode direct_field_addr = AddressWithMode(static_cast<uint32_t>(initial_direct_addr));
+    // We need to read the rest of the log_size number of elements
+    std::vector<uint8_t> log_value_bytes;
+    for (uint32_t i = 0; i < log_size; i++) {
+        FF log_value = unconstrained_read_from_memory(direct_field_addr + i);
+        std::vector<uint8_t> log_value_byte = log_value.to_buffer();
+        bytes_to_hash.insert(bytes_to_hash.end(),
+                             std::make_move_iterator(log_value_byte.begin()),
+                             std::make_move_iterator(log_value_byte.end()));
+    }
+
+    std::array<uint8_t, 32> output = crypto::sha256(bytes_to_hash);
+    // Truncate the hash to 31 bytes so it will be a valid field element
+    FF trunc_hash = FF(from_buffer<uint256_t>(output.data()) >> 8);
+
+    // The + 32 here is for the contract_address in bytes, the +4 is for the extra 4 bytes that contain log_size and is
+    // prefixed to message see toBuffer in unencrypted_l2_log.ts
+    FF length_of_preimage = num_bytes + 32 + 4;
+    // The + 4 is because the kernels store the length of the
+    // processed log as 4 bytes; thus for this length value to match the log length stored in the kernels, we need to
+    // add four to the length here. [Copied from unencrypted_l2_log.ts]
+    FF metadata_log_length = length_of_preimage + 4;
+    Row row = Row{
+        .main_clk = clk,
+        .main_ia = trunc_hash,
+        .main_ib = metadata_log_length,
+        .main_internal_return_ptr = internal_return_ptr,
+        .main_pc = pc++,
+    };
+    kernel_trace_builder.op_emit_unencrypted_log(clk, side_effect_counter, trunc_hash, metadata_log_length);
     row.main_sel_op_emit_unencrypted_log = FF(1);
 
     // Constrain gas cost

barretenberg/cpp/src/barretenberg/vm/constants.hpp

@@ -71,7 +71,11 @@ inline const uint32_t PCPI_NEW_L2_TO_L1_MSGS_OFFSET =
 inline const uint32_t PCPI_START_SIDE_EFFECT_COUNTER_OFFSET =
     PCPI_NEW_L2_TO_L1_MSGS_OFFSET + (MAX_L2_TO_L1_MSGS_PER_CALL * L2_TO_L1_MESSAGE_LENGTH);
 
-inline const uint32_t PCPI_NEW_UNENCRYPTED_LOGS_OFFSET = PCPI_START_SIDE_EFFECT_COUNTER_OFFSET + /*1 item gap*/ 1;
+// The +2 here is because the order is
+// START_SIDE_EFFECT_COUNTER
+// END_SIDE_EFFECT_COUNTER -> + 1
+// NEW_UNENCRYPTED_LOGS -> + 2
+inline const uint32_t PCPI_NEW_UNENCRYPTED_LOGS_OFFSET = PCPI_START_SIDE_EFFECT_COUNTER_OFFSET + 2;
 
 // END INDEXES in the PUBLIC_CIRCUIT_PUBLIC_INPUTS
 

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -544,7 +544,7 @@ contract AvmTest {
         dep::aztec::oracle::debug_log::debug_log("get_da_gas_left");
         let _ = get_da_gas_left(inputs);
         dep::aztec::oracle::debug_log::debug_log("emit_unencrypted_log");
-        // let _ = emit_unencrypted_log(inputs);
+        let _ = emit_unencrypted_log(inputs);
         dep::aztec::oracle::debug_log::debug_log("note_hash_exists");
         let _ = note_hash_exists(inputs, 1, 2);
         dep::aztec::oracle::debug_log::debug_log("new_note_hash");

yarn-project/circuit-types/src/logs/unencrypted_l2_log.ts

@@ -21,10 +21,7 @@ export class UnencryptedL2Log {
   ) {}
 
   get length(): number {
-    // We want the length of the buffer output from function_l2_logs -> toBuffer to equal the stored log length in the kernels.
-    // The kernels store the length of the processed log as 4 bytes; thus for this length value to match the log length stored in the kernels,
-    // we need to add four to the length here.
-    // https://github.com/AztecProtocol/aztec-packages/issues/6578#issuecomment-2125003435
+    // This +4 is because we prefix the log length - see toBuffer below
     return this.data.length + AztecAddress.SIZE_IN_BYTES + 4;
   }
 

yarn-project/simulator/src/public/side_effect_trace.ts

@@ -183,10 +183,8 @@ export class PublicSideEffectTrace implements PublicSideEffectTraceInterface {
     const basicLogHash = Fr.fromBuffer(ulog.hash());
     this.unencryptedLogs.push(ulog);
     this.allUnencryptedLogs.push(ulog);
-    // We want the length of the buffer output from function_l2_logs -> toBuffer to equal the stored log length in the kernels.
-    // The kernels store the length of the processed log as 4 bytes; thus for this length value to match the log length stored in the kernels,
-    // we need to add four to the length here.
-    // https://github.com/AztecProtocol/aztec-packages/issues/6578#issuecomment-2125003435
+    // This length is for charging DA and is checked on-chain - has to be length of log preimage + 4 bytes.
+    // The .length call also has a +4 but that is unrelated
     this.unencryptedLogsHashes.push(new LogHash(basicLogHash, this.sideEffectCounter, new Fr(ulog.length + 4)));
     this.logger.debug(`NEW_UNENCRYPTED_LOG cnt: ${this.sideEffectCounter}`);
     this.incrementSideEffectCounter();