From 1ad2fb06e7e219c1889b647646b5e7beeb416fb6 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Thu, 2 May 2024 20:02:29 +0000 Subject: [PATCH] chore(avm-simulator): avm's nested calls now stay and properly track PublicExecutionResult --- .../end-to-end/src/e2e_avm_simulator.test.ts | 9 +- .../simulator/src/avm/avm_simulator.ts | 2 + .../src/avm/opcodes/external_calls.ts | 35 ++--- yarn-project/simulator/src/public/executor.ts | 25 ++-- .../src/public/public_execution_context.ts | 3 +- .../src/public/transitional_adaptors.ts | 125 ++++++------------ 6 files changed, 86 insertions(+), 113 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts index 12cd2508c2f7..435bc3e8be0e 100644 --- a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts +++ b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts @@ -103,7 +103,7 @@ describe('e2e_avm_simulator', () => { expect(await avmContract.methods.call_avm_from_acvm().simulate()).toEqual(123456n); }); - it('Can call ACVM function from AVM', async () => { + it.skip('Can call ACVM function from AVM', async () => { expect(await avmContract.methods.call_acvm_from_avm().simulate()).toEqual(123456n); }); @@ -113,7 +113,7 @@ describe('e2e_avm_simulator', () => { await avmContract.methods.assert_unsiloed_nullifier_acvm(nullifier).send().wait(); }); - it('AVM nested call to ACVM sees settled nullifiers', async () => { + it.skip('AVM nested call to ACVM sees settled nullifiers', async () => { const nullifier = new Fr(123456); await avmContract.methods.new_nullifier(nullifier).send().wait(); await avmContract.methods @@ -122,7 +122,7 @@ describe('e2e_avm_simulator', () => { .wait(); }); - describe('Authwit', () => { + describe.skip('Authwit', () => { it('Works if authwit provided', async () => { const recipient = AztecAddress.random(); const action = avmContract.methods.test_authwit_send_money( @@ -194,8 +194,7 @@ describe('e2e_avm_simulator', () => { expect(tx.status).toEqual(TxStatus.MINED); }); - // TODO(4293): this should work! Fails in public kernel because both nullifiers are incorrectly being siloed by same address - it.skip('Should be able to emit the same unsiloed nullifier from two different contracts', async () => { + it('Should be able to emit the same unsiloed nullifier from two different contracts', async () => { const nullifier = new Fr(1); const tx = await avmContract.methods .create_same_nullifier_in_nested_call(secondAvmContract.address, nullifier) diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 337cd244980d..4750ecdb23a6 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -2,6 +2,7 @@ import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log'; import { strict as assert } from 'assert'; +import { isAvmBytecode } from '../public/transitional_adaptors.js'; import type { AvmContext } from './avm_context.js'; import { AvmContractCallResults } from './avm_message_call_result.js'; import { AvmExecutionError, InvalidProgramCounterError, NoBytecodeForContractError } from './errors.js'; @@ -32,6 +33,7 @@ export class AvmSimulator { if (!bytecode) { throw new NoBytecodeForContractError(this.context.environment.address); } + assert(isAvmBytecode(bytecode), "AVM simulator can't execute non-AVM bytecode"); return await this.executeBytecode(bytecode); } diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index 9d2a8fa68d4b..2fa2f02ddfc5 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -1,16 +1,12 @@ -import { FunctionSelector } from '@aztec/circuits.js'; +import { FunctionSelector, Gas } from '@aztec/circuits.js'; import { padArrayEnd } from '@aztec/foundation/collection'; -import { executePublicFunction } from '../../public/executor.js'; -import { - convertPublicExecutionResult, - createPublicExecutionContext, - updateAvmContextFromPublicExecutionResult, -} from '../../public/transitional_adaptors.js'; +import { convertAvmResultsToPxResult, createPublicExecution } from '../../public/transitional_adaptors.js'; import type { AvmContext } from '../avm_context.js'; import { gasLeftToGas, sumGas } from '../avm_gas.js'; import { Field, Uint8 } from '../avm_memory_types.js'; import { type AvmContractCallResults } from '../avm_message_call_result.js'; +import { AvmSimulator } from '../avm_simulator.js'; import { Opcode, OperandType } from '../serialization/instruction_serialization.js'; import { Addressing } from './addressing_mode.js'; import { Instruction } from './instruction.js'; @@ -69,7 +65,7 @@ abstract class ExternalCall extends Instruction { const totalGas = sumGas(this.gasCost(memoryOperations), allocatedGas); context.machineState.consumeGas(totalGas); - // TRANSITIONAL: This should be removed once the AVM is fully operational and the public executor is gone. + // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit const nestedContext = context.createNestedContractCallContext( callAddress.toFr(), calldata, @@ -77,13 +73,21 @@ abstract class ExternalCall extends Instruction { this.type, FunctionSelector.fromField(functionSelector), ); - const pxContext = createPublicExecutionContext(nestedContext, calldata); - const pxResults = await executePublicFunction(pxContext, /*nested=*/ true); + const startSideEffectCounter = nestedContext.persistableState.trace.accessCounter; + + const oldStyleExecution = createPublicExecution(startSideEffectCounter, nestedContext.environment, calldata); + const nestedCallResults: AvmContractCallResults = await new AvmSimulator(nestedContext).execute(); + const pxResults = convertAvmResultsToPxResult( + nestedCallResults, + startSideEffectCounter, + oldStyleExecution, + Gas.from(allocatedGas), + nestedContext, + ); // store the old PublicExecutionResult object to maintain a recursive data structure for the old kernel context.persistableState.transitionalExecutionResult.nestedExecutions.push(pxResults); - const nestedCallResults: AvmContractCallResults = convertPublicExecutionResult(pxResults); - updateAvmContextFromPublicExecutionResult(nestedContext, pxResults); - const nestedPersistableState = nestedContext.persistableState; + // END TRANSITIONAL + // const nestedContext = context.createNestedContractCallContext( // callAddress.toFr(), // calldata, @@ -92,7 +96,6 @@ abstract class ExternalCall extends Instruction { // FunctionSelector.fromField(functionSelector), // ); // const nestedCallResults: AvmContractCallResults = await new AvmSimulator(nestedContext).execute(); - // const nestedPersistableState = nestedContext.persistableState; const success = !nestedCallResults.reverted; @@ -114,9 +117,9 @@ abstract class ExternalCall extends Instruction { // TODO: Should we merge the changes from a nested call in the case of a STATIC call? if (success) { - context.persistableState.acceptNestedCallState(nestedPersistableState); + context.persistableState.acceptNestedCallState(nestedContext.persistableState); } else { - context.persistableState.rejectNestedCallState(nestedPersistableState); + context.persistableState.rejectNestedCallState(nestedContext.persistableState); } memory.assert(memoryOperations); diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index d4ead2ce03ad..f33a8d064819 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -26,7 +26,7 @@ import { PackedValuesCache } from '../common/packed_values_cache.js'; import { type CommitmentsDB, type PublicContractsDB, type PublicStateDB } from './db.js'; import { type PublicExecution, type PublicExecutionResult, checkValidStaticCall } from './execution.js'; import { PublicExecutionContext } from './public_execution_context.js'; -import { convertAvmResults, createAvmExecutionEnvironment, isAvmBytecode } from './transitional_adaptors.js'; +import { convertAvmResultsToPxResult, createAvmExecutionEnvironment, isAvmBytecode } from './transitional_adaptors.js'; /** * Execute a public function and return the execution result. @@ -55,6 +55,7 @@ export async function executePublicFunction( async function executePublicFunctionAvm(executionContext: PublicExecutionContext): Promise { const address = executionContext.execution.contractAddress; const selector = executionContext.execution.functionData.selector; + const startGas = executionContext.availableGas; const log = createDebugLogger('aztec:simulator:public_execution'); log.verbose(`[AVM] Executing public external function ${address.toString()}:${selector}.`); @@ -65,7 +66,12 @@ async function executePublicFunctionAvm(executionContext: PublicExecutionContext executionContext.contractsDb, executionContext.commitmentsDb, ); + + // TODO: add sideEffectCounter to persistableState construction + // or modify the PersistableStateManager to manage rollbacks across enqueued-calls and transactions. const worldStateJournal = new AvmPersistableStateManager(hostStorage); + const startSideEffectCounter = executionContext.execution.callContext.sideEffectCounter; + worldStateJournal.trace.accessCounter = startSideEffectCounter; const executionEnv = createAvmExecutionEnvironment( executionContext.execution, @@ -75,18 +81,21 @@ async function executePublicFunctionAvm(executionContext: PublicExecutionContext executionContext.transactionFee, ); - const machineState = new AvmMachineState(executionContext.availableGas); - const context = new AvmContext(worldStateJournal, executionEnv, machineState); - const simulator = new AvmSimulator(context); + const machineState = new AvmMachineState(startGas); + const avmContext = new AvmContext(worldStateJournal, executionEnv, machineState); + const simulator = new AvmSimulator(avmContext); - const result = await simulator.execute(); - const newWorldState = context.persistableState.flush(); + const avmResult = await simulator.execute(); log.verbose( - `[AVM] ${address.toString()}:${selector} returned, reverted: ${result.reverted}, reason: ${result.revertReason}.`, + `[AVM] ${address.toString()}:${selector} returned, reverted: ${avmResult.reverted}, reason: ${ + avmResult.revertReason + }.`, ); - return await convertAvmResults(executionContext, newWorldState, result, machineState); + return Promise.resolve( + convertAvmResultsToPxResult(avmResult, startSideEffectCounter, executionContext.execution, startGas, avmContext), + ); } async function executePublicFunctionAcvm( diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index 5998df359979..bcd468aa5681 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -37,7 +37,8 @@ export class PublicExecutionContext extends TypedOracle { public readonly header: Header, public readonly globalVariables: GlobalVariables, private readonly packedValuesCache: PackedValuesCache, - private readonly sideEffectCounter: SideEffectCounter, + // TRANSITIONAL: once AVM-ACVM interoperability is removed (fully functional AVM), sideEffectCounter can be made private + public readonly sideEffectCounter: SideEffectCounter, public readonly stateDb: PublicStateDB, public readonly contractsDb: PublicContractsDB, public readonly commitmentsDb: CommitmentsDB, diff --git a/yarn-project/simulator/src/public/transitional_adaptors.ts b/yarn-project/simulator/src/public/transitional_adaptors.ts index 2bbd079ad895..09b2e2d99fc6 100644 --- a/yarn-project/simulator/src/public/transitional_adaptors.ts +++ b/yarn-project/simulator/src/public/transitional_adaptors.ts @@ -5,7 +5,7 @@ import { ContractStorageRead, ContractStorageUpdateRequest, FunctionData, - Gas, + type Gas, type GasSettings, type GlobalVariables, type Header, @@ -24,9 +24,8 @@ import { AvmContractCallResults } from '../avm/avm_message_call_result.js'; import { type JournalData } from '../avm/journal/journal.js'; import { Mov } from '../avm/opcodes/memory.js'; import { createSimulationError } from '../common/errors.js'; -import { PackedValuesCache, SideEffectCounter } from '../index.js'; import { type PublicExecution, type PublicExecutionResult } from './execution.js'; -import { PublicExecutionContext } from './public_execution_context.js'; +import { type PublicExecutionContext } from './public_execution_context.js'; /** * Convert a PublicExecution(Environment) object to an AvmExecutionEnvironment @@ -60,40 +59,54 @@ export function createAvmExecutionEnvironment( ); } -export function createPublicExecutionContext(avmContext: AvmContext, calldata: Fr[]): PublicExecutionContext { - const sideEffectCounter = avmContext.persistableState.trace.accessCounter; +export function createPublicExecution( + startSideEffectCounter: number, + avmEnvironment: AvmExecutionEnvironment, + calldata: Fr[], +): PublicExecution { const callContext = CallContext.from({ - msgSender: avmContext.environment.sender, - storageContractAddress: avmContext.environment.storageAddress, - functionSelector: avmContext.environment.temporaryFunctionSelector, - isDelegateCall: avmContext.environment.isDelegateCall, - isStaticCall: avmContext.environment.isStaticCall, - sideEffectCounter: sideEffectCounter, + msgSender: avmEnvironment.sender, + storageContractAddress: avmEnvironment.storageAddress, + functionSelector: avmEnvironment.temporaryFunctionSelector, + isDelegateCall: avmEnvironment.isDelegateCall, + isStaticCall: avmEnvironment.isStaticCall, + sideEffectCounter: startSideEffectCounter, }); - const functionData = new FunctionData(avmContext.environment.temporaryFunctionSelector, /*isPrivate=*/ false); + const functionData = new FunctionData(avmEnvironment.temporaryFunctionSelector, /*isPrivate=*/ false); const execution: PublicExecution = { - contractAddress: avmContext.environment.address, + contractAddress: avmEnvironment.address, callContext, args: calldata, functionData, }; - const packedArgs = PackedValuesCache.create([]); - - const context = new PublicExecutionContext( - execution, - avmContext.environment.header, - avmContext.environment.globals, - packedArgs, - new SideEffectCounter(sideEffectCounter), - avmContext.persistableState.hostStorage.publicStateDb, - avmContext.persistableState.hostStorage.contractsDb, - avmContext.persistableState.hostStorage.commitmentsDb, - Gas.from(avmContext.machineState.gasLeft), - avmContext.environment.transactionFee, - avmContext.environment.gasSettings, - ); + return execution; +} - return context; +export function convertAvmResultsToPxResult( + avmResult: AvmContractCallResults, + startSideEffectCounter: number, + fromPx: PublicExecution, + startGas: Gas, + endAvmContext: AvmContext, +): PublicExecutionResult { + const endPersistableState = endAvmContext.persistableState; + const endMachineState = endAvmContext.machineState; + return { + ...endPersistableState.transitionalExecutionResult, // includes nestedExecutions + execution: fromPx, + returnValues: avmResult.output, + startSideEffectCounter: new Fr(startSideEffectCounter), + endSideEffectCounter: new Fr(endPersistableState.trace.accessCounter), + unencryptedLogs: new UnencryptedFunctionL2Logs(endPersistableState.transitionalExecutionResult.unencryptedLogs), + allUnencryptedLogs: new UnencryptedFunctionL2Logs( + endPersistableState.transitionalExecutionResult.allUnencryptedLogs, + ), + reverted: avmResult.reverted, + revertReason: avmResult.revertReason ? createSimulationError(avmResult.revertReason) : undefined, + startGasLeft: startGas, + endGasLeft: endMachineState.gasLeft, + transactionFee: endAvmContext.environment.transactionFee, + }; } /** @@ -187,60 +200,6 @@ export function convertPublicExecutionResult(res: PublicExecutionResult): AvmCon return new AvmContractCallResults(res.reverted, res.returnValues, res.revertReason); } -export function updateAvmContextFromPublicExecutionResult(ctx: AvmContext, result: PublicExecutionResult): void { - // We have to push these manually and not use the trace* functions - // so that we respect the side effect counters. - for (const readRequest of result.contractStorageReads) { - ctx.persistableState.trace.publicStorageReads.push({ - storageAddress: ctx.environment.storageAddress, - exists: true, // FIXME - slot: readRequest.storageSlot, - value: readRequest.currentValue, - counter: new Fr(readRequest.sideEffectCounter ?? Fr.ZERO), - }); - } - - for (const updateRequest of result.contractStorageUpdateRequests) { - ctx.persistableState.trace.publicStorageWrites.push({ - storageAddress: ctx.environment.storageAddress, - slot: updateRequest.storageSlot, - value: updateRequest.newValue, - counter: new Fr(updateRequest.sideEffectCounter ?? Fr.ZERO), - }); - - // We need to manually populate the cache. - ctx.persistableState.publicStorage.write( - ctx.environment.storageAddress, - updateRequest.storageSlot, - updateRequest.newValue, - ); - } - - for (const nullifier of result.newNullifiers) { - ctx.persistableState.trace.newNullifiers.push({ - storageAddress: ctx.environment.storageAddress, - nullifier: nullifier.value, - counter: new Fr(nullifier.counter), - }); - } - - for (const noteHash of result.newNoteHashes) { - ctx.persistableState.trace.newNoteHashes.push({ - storageAddress: ctx.environment.storageAddress, - noteHash: noteHash.value, - counter: new Fr(noteHash.counter), - }); - } - - for (const message of result.newL2ToL1Messages) { - ctx.persistableState.newL1Messages.push(message); - } - - for (const log of result.unencryptedLogs.logs) { - ctx.persistableState.newLogs.push(new UnencryptedL2Log(log.contractAddress, log.selector, log.data)); - } -} - const AVM_MAGIC_SUFFIX = Buffer.from([ Mov.opcode, // opcode 0x00, // indirect