From 25a7ea76effa98b09051cde383fdcce95e314166 Mon Sep 17 00:00:00 2001
From: Santiago Palladino <santiago@aztecprotocol.com>
Date: Fri, 2 Feb 2024 13:56:22 -0300
Subject: [PATCH] feat!: Unencrypted logs are not strings (#4392)

The private execution oracle was assuming unencrypted log payloads were
strings, where each character was encoded as a field. This means that
emitting a field array did not work, since all bytes but the least
significant one for each field were thrown out.

Given we are not emitting strings from anywhere across our sample
contracts, this commit changes the oracle so it does not throw away the
fields contents, and instead pushes everything into the log payload.
---
 .../acir-simulator/src/acvm/oracle/oracle.ts  |  2 +-
 .../src/client/private_execution.test.ts      | 22 +++++++++++++++++++
 .../contracts/test_contract/src/main.nr       |  5 +++++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/yarn-project/acir-simulator/src/acvm/oracle/oracle.ts b/yarn-project/acir-simulator/src/acvm/oracle/oracle.ts
index 8c97954961a..e73e6a09853 100644
--- a/yarn-project/acir-simulator/src/acvm/oracle/oracle.ts
+++ b/yarn-project/acir-simulator/src/acvm/oracle/oracle.ts
@@ -258,7 +258,7 @@ export class Oracle {
   }
 
   emitUnencryptedLog([contractAddress]: ACVMField[], [eventSelector]: ACVMField[], message: ACVMField[]): ACVMField {
-    const logPayload = Buffer.concat(message.map(charBuffer => Fr.fromString(charBuffer).toBuffer().subarray(-1)));
+    const logPayload = Buffer.concat(message.map(fromACVMField).map(f => f.toBuffer()));
     const log = new UnencryptedL2Log(
       AztecAddress.fromString(contractAddress),
       EventSelector.fromField(fromACVMField(eventSelector)),
diff --git a/yarn-project/acir-simulator/src/client/private_execution.test.ts b/yarn-project/acir-simulator/src/client/private_execution.test.ts
index c199faa8cb5..f7a66112551 100644
--- a/yarn-project/acir-simulator/src/client/private_execution.test.ts
+++ b/yarn-project/acir-simulator/src/client/private_execution.test.ts
@@ -35,6 +35,7 @@ import {
 } from '@aztec/foundation/abi';
 import { asyncMap } from '@aztec/foundation/async-map';
 import { AztecAddress } from '@aztec/foundation/aztec-address';
+import { times } from '@aztec/foundation/collection';
 import { pedersenHash } from '@aztec/foundation/crypto';
 import { EthAddress } from '@aztec/foundation/eth-address';
 import { Fr, GrumpkinScalar } from '@aztec/foundation/fields';
@@ -60,6 +61,7 @@ import { KeyPair, MessageLoadOracleInputs } from '../acvm/index.js';
 import { buildL1ToL2Message } from '../test/utils.js';
 import { computeSlotForMapping } from '../utils.js';
 import { DBOracle } from './db_oracle.js';
+import { collectUnencryptedLogs } from './execution_result.js';
 import { AcirSimulator } from './simulator.js';
 
 jest.setTimeout(60_000);
@@ -232,6 +234,26 @@ describe('Private Execution test suite', () => {
       expect(sideEffectArrayToValueArray(result.callStackItem.publicInputs.newCommitments)).toEqual(emptyCommitments);
       expect(result.callStackItem.publicInputs.contractDeploymentData).toEqual(contractDeploymentData);
     });
+
+    it('emits a field as an unencrypted log', async () => {
+      const artifact = getFunctionArtifact(TestContractArtifact, 'emit_msg_sender');
+      const result = await runSimulator({ artifact, msgSender: owner });
+      const [functionLogs] = collectUnencryptedLogs(result);
+      expect(functionLogs.logs).toHaveLength(1);
+      // Test that the log payload (ie ignoring address, selector, and header) matches what we emitted
+      expect(functionLogs.logs[0].subarray(-32).toString('hex')).toEqual(owner.toBuffer().toString('hex'));
+    });
+
+    it('emits a field array as an unencrypted log', async () => {
+      const artifact = getFunctionArtifact(TestContractArtifact, 'emit_array_as_unencrypted_log');
+      const args = [times(5, () => Fr.random())];
+      const result = await runSimulator({ artifact, msgSender: owner, args });
+      const [functionLogs] = collectUnencryptedLogs(result);
+      expect(functionLogs.logs).toHaveLength(1);
+      // Test that the log payload (ie ignoring address, selector, and header) matches what we emitted
+      const expected = Buffer.concat(args[0].map(arg => arg.toBuffer())).toString('hex');
+      expect(functionLogs.logs[0].subarray(-32 * 5).toString('hex')).toEqual(expected);
+    });
   });
 
   describe('stateful test contract', () => {
diff --git a/yarn-project/noir-contracts/contracts/test_contract/src/main.nr b/yarn-project/noir-contracts/contracts/test_contract/src/main.nr
index d1a18eea77b..c8670731e28 100644
--- a/yarn-project/noir-contracts/contracts/test_contract/src/main.nr
+++ b/yarn-project/noir-contracts/contracts/test_contract/src/main.nr
@@ -233,6 +233,11 @@ contract Test {
         emit_unencrypted_log_from_private(&mut context, context.msg_sender());
     }
 
+    #[aztec(private)]
+    fn emit_array_as_unencrypted_log(fields: [Field; 5]) {
+        emit_unencrypted_log_from_private(&mut context, fields);
+    }
+
     // docs:start:is-time-equal
     #[aztec(public)]
     fn is_time_equal(time: Field) -> Field {