From 51c640e14c4aeb496655dbfa5baa98889ca9e968 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Tue, 11 Jun 2024 10:41:56 +0000 Subject: [PATCH] wip --- .../vm/avm_trace/avm_execution.cpp | 7 ++- .../vm/avm_trace/avm_kernel_trace.cpp | 2 +- .../vm/avm_trace/avm_kernel_trace.hpp | 2 +- .../barretenberg/vm/avm_trace/avm_trace.cpp | 14 ++++-- .../barretenberg/vm/avm_trace/avm_trace.hpp | 4 +- .../barretenberg/vm/avm_trace/constants.hpp | 6 ++- ...test.ts => abstract_phase_manager.test.ts} | 16 +++--- .../src/public/abstract_phase_manager.ts | 49 ++++++++++++++++--- yarn-project/simulator/src/public/executor.ts | 6 +-- yarn-project/simulator/src/public/utils.ts | 39 --------------- 10 files changed, 74 insertions(+), 71 deletions(-) rename yarn-project/simulator/src/public/{utils.test.ts => abstract_phase_manager.test.ts} (50%) delete mode 100644 yarn-project/simulator/src/public/utils.ts diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp index 27dc5548b79..e3fd29e1e45 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp @@ -116,7 +116,7 @@ std::vector Execution::gen_trace(std::vector const& instructio */ VmPublicInputs Execution::convert_public_inputs(std::vector 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 Execution::gen_trace(std::vector 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(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 diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.cpp index 278d4d4a1a1..95aa16a1709 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.cpp +++ b/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() diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.hpp index 16f59551e8c..068d82517e3 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_kernel_trace.hpp +++ b/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 kernel_input_selector_counter; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp index a45bde206d7..84827f08937 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp +++ b/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(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(row.avm_main_ib)); row.avm_main_sel_op_nullifier_exists = FF(1); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp index 70d45adb3d5..806d9d3b296 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.hpp @@ -28,7 +28,9 @@ using Row = bb::AvmFullRow; 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 finalize(uint32_t min_trace_size = 0, bool range_check_required = false); void reset(); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp index 667fd33244a..4d60a7b0ad9 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/constants.hpp +++ b/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 diff --git a/yarn-project/simulator/src/public/utils.test.ts b/yarn-project/simulator/src/public/abstract_phase_manager.test.ts similarity index 50% rename from yarn-project/simulator/src/public/utils.test.ts rename to yarn-project/simulator/src/public/abstract_phase_manager.test.ts index 0ca629d7ce3..58a2881e537 100644 --- a/yarn-project/simulator/src/public/utils.test.ts +++ b/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); }); }); }); diff --git a/yarn-project/simulator/src/public/abstract_phase_manager.ts b/yarn-project/simulator/src/public/abstract_phase_manager.ts index c8471d742ec..3dd7016612d 100644 --- a/yarn-project/simulator/src/public/abstract_phase_manager.ts +++ b/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.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 diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index 2bf1e11d085..ae32fde1d18 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/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 { 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, diff --git a/yarn-project/simulator/src/public/utils.ts b/yarn-project/simulator/src/public/utils.ts deleted file mode 100644 index 2ca86c7ddb6..00000000000 --- a/yarn-project/simulator/src/public/utils.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { type PublicKernelCircuitPublicInputs } from '@aztec/circuits.js'; - -/** - * 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 - */ -export function lastSideEffectCounter(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; -}