From 1d8881e4872b39195ace523432c0e34bc9081f8d Mon Sep 17 00:00:00 2001
From: Santiago Palladino <santiago@aztecprotocol.com>
Date: Wed, 30 Aug 2023 06:54:47 -0300
Subject: [PATCH] fix: Set side effect counter on contract reads (#1870)

The side effect counter was not being collected in
`ContractStorageRead`s due to an error in the `from` static method. This
PR fixes it and reenables a test on public execution.

Fixes #1588
---
 .../acir-simulator/src/public/index.test.ts    | 18 ++++++++----------
 .../structs/public_circuit_public_inputs.ts    |  6 +++++-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/yarn-project/acir-simulator/src/public/index.test.ts b/yarn-project/acir-simulator/src/public/index.test.ts
index c2630961cc3..229708560a0 100644
--- a/yarn-project/acir-simulator/src/public/index.test.ts
+++ b/yarn-project/acir-simulator/src/public/index.test.ts
@@ -1,6 +1,7 @@
 import {
   CallContext,
   CircuitsWasm,
+  ContractStorageRead,
   FunctionData,
   GlobalVariables,
   HistoricBlockData,
@@ -174,23 +175,20 @@ describe('ACIR public execution simulator', () => {
         expect(result.contractStorageReads).toEqual([]);
       });
 
-      // Contract storage reads and update requests are implemented as built-ins, which at the moment Noir does not
-      // now whether they have side-effects or not, so they get run even when their code path
-      // is not picked by a conditional. Once that's fixed, we should re-enable this test.
-      // Task to repair this test: https://github.com/AztecProtocol/aztec-packages/issues/1588
-      it.skip('should run the transfer function without enough sender balance', async () => {
+      it('should fail the transfer function without enough sender balance', async () => {
         const senderBalance = new Fr(10n);
         const recipientBalance = new Fr(20n);
         mockStore(senderBalance, recipientBalance);
 
         const result = await executor.execute(execution, GlobalVariables.empty());
-
         expect(result.returnValues[0]).toEqual(recipientBalance);
 
-        expect(result.contractStorageReads).toEqual([
-          { storageSlot: recipientStorageSlot, value: recipientBalance },
-          { storageSlot: senderStorageSlot, value: senderBalance },
-        ]);
+        expect(result.contractStorageReads).toEqual(
+          [
+            { storageSlot: senderStorageSlot, currentValue: senderBalance, sideEffectCounter: 0 },
+            { storageSlot: recipientStorageSlot, currentValue: recipientBalance, sideEffectCounter: 1 },
+          ].map(ContractStorageRead.from),
+        );
 
         expect(result.contractStorageUpdateRequests).toEqual([]);
       });
diff --git a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts
index 9bbcfe872d3..523b9c22526 100644
--- a/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts
+++ b/yarn-project/circuits.js/src/structs/public_circuit_public_inputs.ts
@@ -49,8 +49,12 @@ export class ContractStorageRead {
      * Value read from the storage slot.
      */
     currentValue: Fr;
+    /**
+     * Optional side effect counter tracking position of this event in tx execution.
+     */
+    sideEffectCounter?: number;
   }) {
-    return new ContractStorageRead(args.storageSlot, args.currentValue);
+    return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter);
   }
 
   toBuffer() {