From e30e7dd39ffa57ec198971ec56ac4f183a2df172 Mon Sep 17 00:00:00 2001
From: Santiago Palladino <santiago@aztecprotocol.com>
Date: Fri, 2 Feb 2024 12:37:13 -0300
Subject: [PATCH] feat!: Unencrypted logs are not strings

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 8c97954961a1..e73e6a098536 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 c199faa8cb50..f7a661125519 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 d1a18eea77b7..c8670731e28d 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 {