fix execution result incorrectly collecting logs when building tx

circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp

@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <array>
 #include <cassert>
+#include <cstddef>
 #include <cstdint>
 #include <tuple>
 #include <vector>
@@ -111,8 +112,9 @@ std::array<fr, NUM_FIELDS_PER_SHA256> compute_kernels_calldata_hash(
     // 2 unencrypted logs hashes (1 per kernel) -> 4 fields --> 2 sha256 hashes --> 64 bytes
     auto const number_of_inputs =
         (KERNEL_NEW_COMMITMENTS_LENGTH + KERNEL_NEW_NULLIFIERS_LENGTH + KERNEL_PUBLIC_DATA_UPDATE_REQUESTS_LENGTH * 2 +
-         KERNEL_NEW_L2_TO_L1_MSGS_LENGTH + KERNEL_NEW_CONTRACTS_LENGTH * 3 + KERNEL_NUM_ENCRYPTED_LOGS_HASHES * 2 +
-         KERNEL_NUM_UNENCRYPTED_LOGS_HASHES * 2) *
+         KERNEL_NEW_L2_TO_L1_MSGS_LENGTH + KERNEL_NEW_CONTRACTS_LENGTH * 3 +
+         KERNEL_NUM_ENCRYPTED_LOGS_HASHES * NUM_FIELDS_PER_SHA256 +
+         KERNEL_NUM_UNENCRYPTED_LOGS_HASHES * NUM_FIELDS_PER_SHA256) *
         2;
     std::array<NT::fr, number_of_inputs> calldata_hash_inputs{};
 
@@ -160,32 +162,44 @@ std::array<fr, NUM_FIELDS_PER_SHA256> compute_kernels_calldata_hash(
 
         offset += KERNEL_NEW_CONTRACTS_LENGTH * 2 * 2;
 
-        calldata_hash_inputs[offset + i * 2] = encryptedLogsHash[0];
-        calldata_hash_inputs[offset + i * 2 + 1] = encryptedLogsHash[1];
+        for (size_t j = 0; j < NUM_FIELDS_PER_SHA256; j++) {
+            calldata_hash_inputs[offset + i * 2 + j] = encryptedLogsHash[j];
+        }
 
-        offset += KERNEL_NUM_ENCRYPTED_LOGS_HASHES * 2 * 2;
+        offset += KERNEL_NUM_ENCRYPTED_LOGS_HASHES * NUM_FIELDS_PER_SHA256 * 2;
 
-        calldata_hash_inputs[offset + i * 2] = unencryptedLogsHash[0];
-        calldata_hash_inputs[offset + i * 2 + 1] = unencryptedLogsHash[1];
+        for (size_t j = 0; j < NUM_FIELDS_PER_SHA256; j++) {
+            calldata_hash_inputs[offset + i * 2 + j] = unencryptedLogsHash[j];
+        }
     }
 
-    // We subtract 4 from inputs size because 1 logs hash is stored in 2 fields and those 2 fields get converted only
-    // to 256 bits and there are 4 logs hashes in total.
-    constexpr auto num_bytes = (calldata_hash_inputs.size() - 4) * 32;
-    std::array<uint8_t, num_bytes> calldata_hash_inputs_bytes;
-    // Convert all into a buffer, then copy into the array, then hash
-    for (size_t i = 0; i < calldata_hash_inputs.size() - 4; i++) {  // -4 because logs are processed out of the loop
+    // OPTIMIZE DATA OVERHEAD
+    // The data structure calldata_hash_inputs contains 2 * 2 * NUM_FIELDS_PER_SHA256 fr entries for logsHashes. (2
+    // kernels and 2 types of log per kernel). One sha-256 hash value represented in fr array is actually splitted into
+    // 2 fields because fr is 254 bits (smaller than 256 bits). By serializing such a (fr-based) hash value back into
+    // bytes, we only need 32 bytes (256 bits), while the standard fr-array to bytes serialization would return 64
+    // bytes. For the unencryptedlogs and encryptedlogs, we therefore need 2 * NUM_FIELDS_PER_SHA256 * 32 bytes instead
+    // of 2 * 2 * NUM_FIELDS_PER_SHA256 * 32 bytes (half of this is saved).
+
+    // We subtract 2 * NUM_FIELDS_PER_SHA256 * 32 bytes as explained above.
+    constexpr auto num_bytes = (calldata_hash_inputs.size() - NUM_FIELDS_PER_SHA256 * 2) * 32;
+    std::array<uint8_t, num_bytes> calldata_hash_inputs_bytes{};
+
+    // Serialize everything from calldata_hash_inputs except the logs at this stage and copy into
+    // calldata_hash_inputs_bytes;
+    for (size_t i = 0; i < calldata_hash_inputs.size() - NUM_FIELDS_PER_SHA256 * 2 * 2; i++) {
         auto as_bytes = calldata_hash_inputs[i].to_buffer();
-
         auto offset = i * 32;
         std::copy(as_bytes.begin(), as_bytes.end(), calldata_hash_inputs_bytes.begin() + offset);
     }
 
     // Copy the 4 fields of 2 encrypted logs to 64 bytes
     // Modified version of:
     // https://github.com/AztecProtocol/aztec-packages/blob/01080c7f1d2956512b6a9cff0582b43be25b3cc2/circuits/cpp/src/aztec3/circuits/hash.hpp#L350
-    const uint32_t encrypted_logs_start_index = calldata_hash_inputs.size() - 8;
-    const uint32_t first_modified_byte_encrypted = num_bytes - 128;  // 128 = num bytes occupied by all the logs hashes
+    const uint32_t encrypted_logs_start_index = calldata_hash_inputs.size() - NUM_FIELDS_PER_SHA256 * 2 * 2;
+    const uint32_t first_modified_byte_encrypted =
+        num_bytes - NUM_FIELDS_PER_SHA256 * 2 *
+                        32;  // offsetting by number of bytes occupied by all the logs hashes (in the optimized form)
     for (uint8_t i = 0; i < 4; i++) {
         auto half = calldata_hash_inputs[encrypted_logs_start_index + i].to_buffer();
         for (uint8_t j = 0; j < 16; j++) {
@@ -194,9 +208,9 @@ std::array<fr, NUM_FIELDS_PER_SHA256> compute_kernels_calldata_hash(
     }
 
     // Do the same for the unencrypted logs
-    const uint32_t unencrypted_logs_start_index = calldata_hash_inputs.size() - 4;
+    const uint32_t unencrypted_logs_start_index = calldata_hash_inputs.size() - NUM_FIELDS_PER_SHA256 * 2;
     const uint32_t first_modified_byte_unencrypted =
-        num_bytes - 64;  // 64 = num bytes occupied by unencrypted logs hashes
+        num_bytes - NUM_FIELDS_PER_SHA256 * 32;  // offsetting num bytes occupied by unencrypted logs hashes
     for (uint8_t i = 0; i < 4; i++) {
         auto half = calldata_hash_inputs[unencrypted_logs_start_index + i].to_buffer();
         for (uint8_t j = 0; j < 16; j++) {

yarn-project/acir-simulator/src/client/execution_result.ts

@@ -78,7 +78,15 @@ export interface ExecutionResult {
  * @returns All encrypted logs.
  */
 export function collectEncryptedLogs(execResult: ExecutionResult): FunctionL2Logs[] {
-  return [execResult.encryptedLogs, ...execResult.nestedExecutions.flatMap(collectEncryptedLogs)];
+  const logs: FunctionL2Logs[] = [];
+  // traverse through the stack of nested executions
+  const executionStack = [execResult];
+  while (executionStack.length) {
+    const currentExecution = executionStack.pop()!;
+    executionStack.push(...currentExecution.nestedExecutions);
+    logs.push(currentExecution.encryptedLogs);
+  }
+  return logs;
 }
 
 /**
@@ -87,7 +95,15 @@ export function collectEncryptedLogs(execResult: ExecutionResult): FunctionL2Log
  * @returns All unencrypted logs.
  */
 export function collectUnencryptedLogs(execResult: ExecutionResult): FunctionL2Logs[] {
-  return [execResult.unencryptedLogs, ...execResult.nestedExecutions.flatMap(collectUnencryptedLogs)];
+  const logs: FunctionL2Logs[] = [];
+  // traverse through the stack of nested executions
+  const executionStack = [execResult];
+  while (executionStack.length) {
+    const currentExecution = executionStack.pop()!;
+    executionStack.push(...currentExecution.nestedExecutions);
+    logs.push(currentExecution.unencryptedLogs);
+  }
+  return logs;
 }
 
 /**