feat(avm): plumb start side effect counter in circuit (#7007)

It still needs to be constrained.

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp

@@ -116,7 +116,7 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
  */
 VmPublicInputs Execution::convert_public_inputs(std::vector<FF> const& public_inputs_vec)
 {
-    VmPublicInputs public_inputs = {};
+    VmPublicInputs public_inputs;
 
     // Case where we pass in empty public inputs - this will be used in tests where they are not required
     if (public_inputs_vec.empty()) {
@@ -306,7 +306,10 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
     // TODO(https://github.com/AztecProtocol/aztec-packages/issues/6718): construction of the public input columns
     // should be done in the kernel - this is stubbed and underconstrained
     VmPublicInputs public_inputs = convert_public_inputs(public_inputs_vec);
-    AvmTraceBuilder trace_builder(public_inputs, execution_hints);
+    uint32_t start_side_effect_counter =
+        !public_inputs_vec.empty() ? static_cast<uint32_t>(public_inputs_vec[PCPI_START_SIDE_EFFECT_COUNTER_OFFSET])
+                                   : 0;
+    AvmTraceBuilder trace_builder(public_inputs, execution_hints, start_side_effect_counter);
 
     // Copied version of pc maintained in trace builder. The value of pc is evolving based
     // on opcode logic and therefore is not maintained here. However, the next opcode in the execution

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.cpp

@@ -12,7 +12,7 @@
 namespace bb::avm_trace {
 
 AvmKernelTraceBuilder::AvmKernelTraceBuilder(VmPublicInputs public_inputs)
-    : public_inputs(public_inputs)
+    : public_inputs(std::move(public_inputs))
 {}
 
 void AvmKernelTraceBuilder::reset()

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.hpp

@@ -59,7 +59,7 @@ class AvmKernelTraceBuilder {
         bool op_sstore = false;
     };
 
-    VmPublicInputs public_inputs{};
+    VmPublicInputs public_inputs;
 
     // Counts the number of accesses into each SELECTOR for the environment selector lookups;
     std::unordered_map<uint32_t, uint32_t> kernel_input_selector_counter;

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp

@@ -26,10 +26,13 @@ namespace bb::avm_trace {
  * @brief Constructor of a trace builder of AVM. Only serves to set the capacity of the
  *        underlying traces and initialize gas values.
  */
-AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs, ExecutionHints execution_hints)
+AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs,
+                                 ExecutionHints execution_hints,
+                                 uint32_t side_effect_counter)
     // NOTE: we initialise the environment builder here as it requires public inputs
-    : kernel_trace_builder(public_inputs)
-    , execution_hints(execution_hints)
+    : kernel_trace_builder(std::move(public_inputs))
+    , side_effect_counter(side_effect_counter)
+    , execution_hints(std::move(execution_hints))
 {
     main_trace.reserve(AVM_TRACE_SIZE);
 
@@ -1643,11 +1646,12 @@ void AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, uint32_t note_offset
     side_effect_counter++;
 }
 
-void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, uint32_t note_offset, uint32_t dest_offset)
+void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, uint32_t nullifier_offset, uint32_t dest_offset)
 {
     auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
 
-    Row row = create_kernel_output_opcode_with_set_metadata_output_from_hint(indirect, clk, note_offset, dest_offset);
+    Row row =
+        create_kernel_output_opcode_with_set_metadata_output_from_hint(indirect, clk, nullifier_offset, dest_offset);
     kernel_trace_builder.op_nullifier_exists(
         clk, side_effect_counter, row.avm_main_ia, /*safe*/ static_cast<uint32_t>(row.avm_main_ib));
     row.avm_main_sel_op_nullifier_exists = FF(1);

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp

@@ -28,7 +28,9 @@ using Row = bb::AvmFullRow<bb::fr>;
 class AvmTraceBuilder {
 
   public:
-    AvmTraceBuilder(VmPublicInputs public_inputs = {}, ExecutionHints execution_hints = {});
+    AvmTraceBuilder(VmPublicInputs public_inputs = {},
+                    ExecutionHints execution_hints = {},
+                    uint32_t side_effect_counter = 0);
 
     std::vector<Row> finalize(uint32_t min_trace_size = 0, bool range_check_required = false);
     void reset();

barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp

@@ -67,8 +67,10 @@ inline const uint32_t PCPI_NEW_NULLIFIERS_OFFSET =
 inline const uint32_t PCPI_NEW_L2_TO_L1_MSGS_OFFSET =
     PCPI_NEW_NULLIFIERS_OFFSET + (MAX_NEW_NULLIFIERS_PER_CALL * NULLIFIER_LENGTH);
 
-inline const uint32_t PCPI_NEW_UNENCRYPTED_LOGS_OFFSET /*2 item gap*/ =
-    PCPI_NEW_L2_TO_L1_MSGS_OFFSET + (MAX_NEW_L2_TO_L1_MSGS_PER_CALL * L2_TO_L1_MESSAGE_LENGTH) + 2;
+inline const uint32_t PCPI_START_SIDE_EFFECT_COUNTER_OFFSET =
+    PCPI_NEW_L2_TO_L1_MSGS_OFFSET + (MAX_NEW_L2_TO_L1_MSGS_PER_CALL * L2_TO_L1_MESSAGE_LENGTH);
+
+inline const uint32_t PCPI_NEW_UNENCRYPTED_LOGS_OFFSET = PCPI_START_SIDE_EFFECT_COUNTER_OFFSET + /*1 item gap*/ 1;
 
 // END INDEXES in the PUBLIC_CIRCUIT_PUBLIC_INPUTS
 

yarn-project/simulator/src/public/utils.test.ts → yarn-project/simulator/src/public/abstract_phase_manager.test.ts

@@ -1,26 +1,26 @@
 import { Fr } from '@aztec/circuits.js';
 import { makePublicKernelCircuitPublicInputs } from '@aztec/circuits.js/testing';
 
-import { lastSideEffectCounter } from './utils.js';
+import { AbstractPhaseManager } from './abstract_phase_manager.js';
 
-describe('sequencer utils', () => {
-  describe('lastSideEffectCounter', () => {
+describe('AbstractPhaseManager utils', () => {
+  describe('getMaxSideEffectCounter', () => {
     it('correctly identifies the highest side effect counter in a transaction', () => {
       const inputs = makePublicKernelCircuitPublicInputs();
 
-      const startingCounter = lastSideEffectCounter(inputs);
+      const startingCounter = AbstractPhaseManager.getMaxSideEffectCounter(inputs);
 
       inputs.endNonRevertibleData.newNoteHashes.at(-1)!.counter = startingCounter + 1;
-      expect(lastSideEffectCounter(inputs)).toBe(startingCounter + 1);
+      expect(AbstractPhaseManager.getMaxSideEffectCounter(inputs)).toBe(startingCounter + 1);
 
       inputs.endNonRevertibleData.publicCallStack.at(-1)!.startSideEffectCounter = new Fr(startingCounter + 2);
-      expect(lastSideEffectCounter(inputs)).toBe(startingCounter + 2);
+      expect(AbstractPhaseManager.getMaxSideEffectCounter(inputs)).toBe(startingCounter + 2);
 
       inputs.end.newNoteHashes.at(-1)!.counter = startingCounter + 3;
-      expect(lastSideEffectCounter(inputs)).toBe(startingCounter + 3);
+      expect(AbstractPhaseManager.getMaxSideEffectCounter(inputs)).toBe(startingCounter + 3);
 
       inputs.end.newNullifiers.at(-1)!.counter = startingCounter + 4;
-      expect(lastSideEffectCounter(inputs)).toBe(startingCounter + 4);
+      expect(AbstractPhaseManager.getMaxSideEffectCounter(inputs)).toBe(startingCounter + 4);
     });
   });
 });

yarn-project/simulator/src/public/abstract_phase_manager.ts

@@ -67,7 +67,6 @@ import { type MerkleTreeOperations } from '@aztec/world-state';
 
 import { HintsBuilder } from './hints_builder.js';
 import { type PublicKernelCircuitSimulator } from './public_kernel_circuit_simulator.js';
-import { lastSideEffectCounter } from './utils.js';
 
 export const PhaseIsRevertible: Record<PublicKernelType, boolean> = {
   [PublicKernelType.NON_PUBLIC]: false,
@@ -263,19 +262,15 @@ export abstract class AbstractPhaseManager {
       while (executionStack.length) {
         const current = executionStack.pop()!;
         const isExecutionRequest = !isPublicExecutionResult(current);
-        const sideEffectCounter = lastSideEffectCounter(kernelPublicOutput) + 1;
-        const availableGas = this.getAvailableGas(tx, kernelPublicOutput);
-        const pendingNullifiers = this.getSiloedPendingNullifiers(kernelPublicOutput);
-
         const result = isExecutionRequest
           ? await this.publicExecutor.simulate(
               current,
               this.globalVariables,
-              availableGas,
+              /*availableGas=*/ this.getAvailableGas(tx, kernelPublicOutput),
               tx.data.constants.txContext,
-              pendingNullifiers,
+              /*pendingNullifiers=*/ this.getSiloedPendingNullifiers(kernelPublicOutput),
               transactionFee,
-              sideEffectCounter,
+              /*startSideEffectCounter=*/ AbstractPhaseManager.getMaxSideEffectCounter(kernelPublicOutput) + 1,
             )
           : current;
 
@@ -493,6 +488,44 @@ export abstract class AbstractPhaseManager {
     return await Promise.all(nested.map(n => this.getPublicCallStackItem(n)));
   }
 
+  /**
+   * Looks at the side effects of a transaction and returns the highest counter
+   * @param tx - A transaction
+   * @returns The highest side effect counter in the transaction so far
+   */
+  static getMaxSideEffectCounter(inputs: PublicKernelCircuitPublicInputs): number {
+    const sideEffectCounters = [
+      ...inputs.endNonRevertibleData.newNoteHashes,
+      ...inputs.endNonRevertibleData.newNullifiers,
+      ...inputs.endNonRevertibleData.noteEncryptedLogsHashes,
+      ...inputs.endNonRevertibleData.encryptedLogsHashes,
+      ...inputs.endNonRevertibleData.unencryptedLogsHashes,
+      ...inputs.endNonRevertibleData.publicCallStack,
+      ...inputs.endNonRevertibleData.publicDataUpdateRequests,
+      ...inputs.end.newNoteHashes,
+      ...inputs.end.newNullifiers,
+      ...inputs.end.noteEncryptedLogsHashes,
+      ...inputs.end.encryptedLogsHashes,
+      ...inputs.end.unencryptedLogsHashes,
+      ...inputs.end.publicCallStack,
+      ...inputs.end.publicDataUpdateRequests,
+    ];
+
+    let max = 0;
+    for (const sideEffect of sideEffectCounters) {
+      if ('startSideEffectCounter' in sideEffect) {
+        // look at both start and end counters because for enqueued public calls start > 0 while end === 0
+        max = Math.max(max, sideEffect.startSideEffectCounter.toNumber(), sideEffect.endSideEffectCounter.toNumber());
+      } else if ('counter' in sideEffect) {
+        max = Math.max(max, sideEffect.counter);
+      } else {
+        throw new Error('Unknown side effect type');
+      }
+    }
+
+    return max;
+  }
+
   protected getBytecodeHash(_result: PublicExecutionResult) {
     // TODO: Determine how to calculate bytecode hash. Circuits just check it isn't zero for now.
     // See https://github.com/AztecProtocol/aztec3-packages/issues/378

yarn-project/simulator/src/public/executor.ts

@@ -38,7 +38,7 @@ export class PublicExecutor {
     txContext: TxContext,
     pendingNullifiers: Nullifier[],
     transactionFee: Fr = Fr.ZERO,
-    sideEffectCounter: number = 0,
+    startSideEffectCounter: number = 0,
   ): Promise<PublicExecutionResult> {
     const address = execution.contractAddress;
     const selector = execution.functionSelector;
@@ -52,13 +52,11 @@ export class PublicExecutor {
     // These data structures will permeate across the simulator when the public executor is phased out
     const hostStorage = new HostStorage(this.stateDb, this.contractsDb, this.commitmentsDb);
 
-    const startSideEffectCounter = sideEffectCounter;
     const worldStateJournal = new AvmPersistableStateManager(hostStorage);
     for (const nullifier of pendingNullifiers) {
       worldStateJournal.nullifiers.cache.appendSiloed(nullifier.value);
     }
-    // All the subsequent side effects will have a counter larger than the call's start counter.
-    worldStateJournal.trace.accessCounter = startSideEffectCounter + 1;
+    worldStateJournal.trace.accessCounter = startSideEffectCounter;
 
     const executionEnv = createAvmExecutionEnvironment(
       execution,