From 7803841cbb28bf84c32dc349f54de671ad029730 Mon Sep 17 00:00:00 2001 From: Rahul Kothari Date: Thu, 6 Jul 2023 13:18:41 +0000 Subject: [PATCH 1/2] fix call stack incorrectly collecting logs --- .../circuits/rollup/components/components.cpp | 2 +- .../src/client/execution_result.ts | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp index fb4937d6a5da..955fe32a5f52 100644 --- a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp +++ b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp @@ -174,7 +174,7 @@ std::array compute_kernels_calldata_hash( constexpr auto num_bytes = (calldata_hash_inputs.size() - 4) * 32; std::array 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 + for (size_t i = 0; i < calldata_hash_inputs.size() - 8; i++) { // -8 because logs are processed out of the loop auto as_bytes = calldata_hash_inputs[i].to_buffer(); auto offset = i * 32; diff --git a/yarn-project/acir-simulator/src/client/execution_result.ts b/yarn-project/acir-simulator/src/client/execution_result.ts index 15dd620b6e08..e516cdde5226 100644 --- a/yarn-project/acir-simulator/src/client/execution_result.ts +++ b/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; } /** From 2076458a8cc0b1f3895605c4c2287fed2209a695 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 7 Jul 2023 09:03:41 +0000 Subject: [PATCH 2/2] Add comments and use NUM_FIELDS_PER_SHA256 consistently in components.cpp --- .../circuits/rollup/components/components.cpp | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp index 955fe32a5f52..2f706bde379f 100644 --- a/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp +++ b/circuits/cpp/src/aztec3/circuits/rollup/components/components.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -111,8 +112,9 @@ std::array 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 calldata_hash_inputs{}; @@ -160,23 +162,33 @@ std::array 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 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() - 8; i++) { // -8 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 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); } @@ -184,8 +196,10 @@ std::array compute_kernels_calldata_hash( // 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 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++) {