Addressing review feedback

yarn-project/simulator/src/avm/avm_context.test.ts

@@ -10,18 +10,11 @@ describe('Avm Context', () => {
     const newAddress = AztecAddress.random();
     const newCalldata = [new Fr(1), new Fr(2)];
     const allocatedGas = { l2Gas: 2, daGas: 3 }; // How much of the current call gas we pass to the nested call
-    const newContext = context.createNestedContractCallContext(
-      newAddress,
-      newCalldata,
-      allocatedGas,
-      'CALL',
-      'top level function',
-    );
+    const newContext = context.createNestedContractCallContext(newAddress, newCalldata, allocatedGas, 'CALL');
 
     expect(newContext.environment).toEqual(
       allSameExcept(context.environment, {
         address: newAddress,
-        fnName: 'top level function',
         contractCallDepth: Fr.ONE,
         calldata: newCalldata,
         isStaticCall: false,
@@ -46,18 +39,11 @@ describe('Avm Context', () => {
     const newAddress = AztecAddress.random();
     const newCalldata = [new Fr(1), new Fr(2)];
     const allocatedGas = { l2Gas: 2, daGas: 3 };
-    const newContext = context.createNestedContractCallContext(
-      newAddress,
-      newCalldata,
-      allocatedGas,
-      'STATICCALL',
-      'static function',
-    );
+    const newContext = context.createNestedContractCallContext(newAddress, newCalldata, allocatedGas, 'STATICCALL');
 
     expect(newContext.environment).toEqual(
       allSameExcept(context.environment, {
         address: newAddress,
-        fnName: 'static function',
         contractCallDepth: Fr.ONE,
         calldata: newCalldata,
         isStaticCall: true,

yarn-project/simulator/src/avm/avm_context.ts

@@ -44,13 +44,12 @@ export class AvmContext {
     calldata: Fr[],
     allocatedGas: Gas,
     callType: 'CALL' | 'STATICCALL',
-    fnName: string,
   ): AvmContext {
     const deriveFn =
       callType === 'CALL'
         ? this.environment.deriveEnvironmentForNestedCall
         : this.environment.deriveEnvironmentForNestedStaticCall;
-    const newExecutionEnvironment = deriveFn.call(this.environment, address, calldata, fnName);
+    const newExecutionEnvironment = deriveFn.call(this.environment, address, calldata);
     const forkedWorldState = this.persistableState.fork();
     const machineState = AvmMachineState.fromState(gasToGasLeft(allocatedGas));
     return new AvmContext(forkedWorldState, newExecutionEnvironment, machineState);

yarn-project/simulator/src/avm/avm_execution_environment.test.ts

@@ -9,30 +9,24 @@ describe('Execution Environment', () => {
 
   it('New call should fork execution environment correctly', () => {
     const executionEnvironment = initExecutionEnvironment();
-    const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedCall(newAddress, calldata, 'func');
+    const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedCall(newAddress, calldata);
 
     expect(newExecutionEnvironment).toEqual(
       allSameExcept(executionEnvironment, {
         address: newAddress,
-        fnName: 'func',
         contractCallDepth: Fr.ONE,
         calldata: calldata,
       }),
     );
   });
 
-  it('New static call call should fork execution environment correctly', () => {
+  it('New static call should fork execution environment correctly', () => {
     const executionEnvironment = initExecutionEnvironment();
-    const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedStaticCall(
-      newAddress,
-      calldata,
-      'static func',
-    );
+    const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedStaticCall(newAddress, calldata);
 
     expect(newExecutionEnvironment).toEqual(
       allSameExcept(executionEnvironment, {
         address: newAddress,
-        fnName: 'static func',
         contractCallDepth: Fr.ONE,
         isStaticCall: true,
         calldata: calldata,

yarn-project/simulator/src/avm/avm_execution_environment.ts

@@ -10,24 +10,17 @@ export class AvmExecutionEnvironment {
   constructor(
     public readonly address: AztecAddress,
     public readonly sender: AztecAddress,
-    public readonly fnName: string,
     public readonly contractCallDepth: Fr,
     public readonly transactionFee: Fr,
     public readonly globals: GlobalVariables,
     public readonly isStaticCall: boolean,
     public readonly calldata: Fr[],
   ) {}
 
-  private deriveEnvironmentForNestedCallInternal(
-    targetAddress: AztecAddress,
-    calldata: Fr[],
-    fnName: string,
-    isStaticCall: boolean,
-  ) {
+  private deriveEnvironmentForNestedCallInternal(targetAddress: AztecAddress, calldata: Fr[], isStaticCall: boolean) {
     return new AvmExecutionEnvironment(
       /*address=*/ targetAddress,
       /*sender=*/ this.address,
-      fnName,
       this.contractCallDepth.add(Fr.ONE),
       this.transactionFee,
       this.globals,
@@ -36,19 +29,11 @@ export class AvmExecutionEnvironment {
     );
   }
 
-  public deriveEnvironmentForNestedCall(
-    targetAddress: AztecAddress,
-    calldata: Fr[],
-    fnName: string,
-  ): AvmExecutionEnvironment {
-    return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, fnName, /*isStaticCall=*/ false);
+  public deriveEnvironmentForNestedCall(targetAddress: AztecAddress, calldata: Fr[]): AvmExecutionEnvironment {
+    return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, /*isStaticCall=*/ false);
   }
 
-  public deriveEnvironmentForNestedStaticCall(
-    targetAddress: AztecAddress,
-    calldata: Fr[],
-    fnName: string,
-  ): AvmExecutionEnvironment {
-    return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, fnName, /*isStaticCall=*/ true);
+  public deriveEnvironmentForNestedStaticCall(targetAddress: AztecAddress, calldata: Fr[]): AvmExecutionEnvironment {
+    return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, /*isStaticCall=*/ true);
   }
 }

yarn-project/simulator/src/avm/avm_simulator.ts

@@ -43,23 +43,34 @@ export class AvmSimulator {
   private tallyPrintFunction = () => {};
   private tallyInstructionFunction = (_a: number, _b: string, _c: Gas) => {};
 
+  // Test Purposes only: Logger will not have the proper function name. Use this constructor for testing purposes
+  // only. Otherwise, use build() below.
   constructor(private context: AvmContext, private instructionSet: InstructionSet = INSTRUCTION_SET()) {
     assert(
       context.machineState.gasLeft.l2Gas <= MAX_L2_GAS_PER_ENQUEUED_CALL,
       `Cannot allocate more than ${MAX_L2_GAS_PER_ENQUEUED_CALL} to the AVM for execution of an enqueued call`,
     );
-    this.log = createLogger(`simulator:avm:core(f:${this.context.environment.fnName})`);
+    this.log = createLogger(`aztec:avm_simulator:core(calldata[0]: ${context.environment.calldata[0]})`);
     // TODO(palla/log): Should tallies be printed on debug, or only on trace?
     if (this.log.isLevelEnabled('debug')) {
       this.tallyPrintFunction = this.printOpcodeTallies;
       this.tallyInstructionFunction = this.tallyInstruction;
     }
   }
 
-  public static create(
+  // Factory to have a proper function name in the logger. Retrieving the name is asynchronous and
+  // cannot be done as part of the constructor.
+  public static async build(context: AvmContext): Promise<AvmSimulator> {
+    const simulator = new AvmSimulator(context);
+    const fnName = await context.persistableState.getPublicFunctionDebugName(context.environment);
+    simulator.log = createLogger(`aztec:avm_simulator:core(f:${fnName})`);
+
+    return simulator;
+  }
+
+  public static async create(
     stateManager: AvmPersistableStateManager,
     address: AztecAddress,
-    fnName: string,
     sender: AztecAddress,
     transactionFee: Fr,
     globals: GlobalVariables,
@@ -70,7 +81,6 @@ export class AvmSimulator {
     const avmExecutionEnv = new AvmExecutionEnvironment(
       address,
       sender,
-      fnName,
       /*contractCallDepth=*/ Fr.zero(),
       transactionFee,
       globals,
@@ -80,8 +90,7 @@ export class AvmSimulator {
 
     const avmMachineState = new AvmMachineState(allocatedGas);
     const avmContext = new AvmContext(stateManager, avmExecutionEnv, avmMachineState);
-    const instructionSet = INSTRUCTION_SET();
-    return new AvmSimulator(avmContext, instructionSet);
+    return await AvmSimulator.build(avmContext);
   }
 
   /**
@@ -92,11 +101,12 @@ export class AvmSimulator {
     if (!bytecode) {
       // revert, consuming all gas
       const message = `No bytecode found at: ${this.context.environment.address}. Reverting...`;
+      const fnName = await this.context.persistableState.getPublicFunctionDebugName(this.context.environment);
       const revertReason = new AvmRevertReason(
         message,
         /*failingFunction=*/ {
           contractAddress: this.context.environment.address,
-          functionName: this.context.environment.fnName,
+          functionName: fnName,
         },
         /*noirCallStack=*/ [],
       );
@@ -170,7 +180,7 @@ export class AvmSimulator {
 
       const output = machineState.getOutput();
       const reverted = machineState.getReverted();
-      const revertReason = reverted ? revertReasonFromExplicitRevert(output, this.context) : undefined;
+      const revertReason = reverted ? await revertReasonFromExplicitRevert(output, this.context) : undefined;
       const results = new AvmContractCallResult(reverted, output, machineState.gasLeft, revertReason);
       this.log.debug(`Context execution results: ${results.toString()}`);
 
@@ -184,7 +194,7 @@ export class AvmSimulator {
         throw err;
       }
 
-      const revertReason = revertReasonFromExceptionalHalt(err, this.context);
+      const revertReason = await revertReasonFromExceptionalHalt(err, this.context);
       // Note: "exceptional halts" cannot return data, hence [].
       const results = new AvmContractCallResult(/*reverted=*/ true, /*output=*/ [], machineState.gasLeft, revertReason);
       this.log.debug(`Context execution results: ${results.toString()}`);

yarn-project/simulator/src/avm/errors.ts

@@ -138,7 +138,7 @@ export class AvmRevertReason extends ExecutionError {
   }
 }
 
-function createRevertReason(message: string, revertData: Fr[], context: AvmContext): AvmRevertReason {
+async function createRevertReason(message: string, revertData: Fr[], context: AvmContext): Promise<AvmRevertReason> {
   // We drop the returnPc information.
   const internalCallStack = context.machineState.internalCallStack.map(entry => entry.callPc);
 
@@ -153,11 +153,13 @@ function createRevertReason(message: string, revertData: Fr[], context: AvmConte
     message = context.machineState.collectedRevertInfo.recursiveRevertReason.message;
   }
 
+  const fnName = await context.persistableState.getPublicFunctionDebugName(context.environment);
+
   return new AvmRevertReason(
     message,
     /*failingFunction=*/ {
       contractAddress: context.environment.address,
-      functionName: context.environment.fnName,
+      functionName: fnName,
     },
     /*noirCallStack=*/ [...internalCallStack, context.machineState.pc].map(pc => `0.${pc}`),
     /*options=*/ { cause: nestedError },
@@ -170,8 +172,11 @@ function createRevertReason(message: string, revertData: Fr[], context: AvmConte
  * @param haltingError - the lower-level error causing the exceptional halt
  * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack
  */
-export function revertReasonFromExceptionalHalt(haltingError: AvmExecutionError, context: AvmContext): AvmRevertReason {
-  return createRevertReason(haltingError.message, [], context);
+export async function revertReasonFromExceptionalHalt(
+  haltingError: AvmExecutionError,
+  context: AvmContext,
+): Promise<AvmRevertReason> {
+  return await createRevertReason(haltingError.message, [], context);
 }
 
 /**
@@ -180,6 +185,6 @@ export function revertReasonFromExceptionalHalt(haltingError: AvmExecutionError,
  * @param revertData - output data of the explicit REVERT instruction
  * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack
  */
-export function revertReasonFromExplicitRevert(revertData: Fr[], context: AvmContext): AvmRevertReason {
-  return createRevertReason('Assertion failed: ', revertData, context);
+export async function revertReasonFromExplicitRevert(revertData: Fr[], context: AvmContext): Promise<AvmRevertReason> {
+  return await createRevertReason('Assertion failed: ', revertData, context);
 }

yarn-project/simulator/src/avm/fixtures/index.ts

@@ -64,7 +64,6 @@ export function initExecutionEnvironment(overrides?: Partial<AvmExecutionEnviron
   return new AvmExecutionEnvironment(
     overrides?.address ?? AztecAddress.zero(),
     overrides?.sender ?? AztecAddress.zero(),
-    overrides?.fnName ?? '',
     overrides?.contractCallDepth ?? Fr.zero(),
     overrides?.transactionFee ?? Fr.zero(),
     overrides?.globals ?? GlobalVariables.empty(),

yarn-project/simulator/src/avm/journal/journal.ts

@@ -706,8 +706,8 @@ export class AvmPersistableStateManager {
     this.trace.traceEnqueuedCall(publicCallRequest, calldata, reverted);
   }
 
-  public async getPublicFunctionDebugName(contractAddress: AztecAddress, calldata: Fr[]): Promise<string> {
-    return await getPublicFunctionDebugName(this.worldStateDB, contractAddress, calldata);
+  public async getPublicFunctionDebugName(avmEnvironment: AvmExecutionEnvironment): Promise<string> {
+    return await getPublicFunctionDebugName(this.worldStateDB, avmEnvironment.address, avmEnvironment.calldata);
   }
 }
 

yarn-project/simulator/src/avm/opcodes/external_calls.ts

@@ -62,17 +62,9 @@ abstract class ExternalCall extends Instruction {
     context.machineState.consumeGas(allocatedGas);
 
     const aztecAddress = callAddress.toAztecAddress();
-    const fnName = await context.persistableState.getPublicFunctionDebugName(aztecAddress, calldata);
-
-    const nestedContext = context.createNestedContractCallContext(
-      aztecAddress,
-      calldata,
-      allocatedGas,
-      callType,
-      fnName,
-    );
+    const nestedContext = context.createNestedContractCallContext(aztecAddress, calldata, allocatedGas, callType);
 
-    const simulator = new AvmSimulator(nestedContext);
+    const simulator = await AvmSimulator.build(nestedContext);
     const nestedCallResults: AvmContractCallResult = await simulator.execute();
     const success = !nestedCallResults.reverted;
 

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

@@ -333,10 +333,9 @@ export class PublicTxSimulator {
     );
     const timer = new Timer();
 
-    const simulator = AvmSimulator.create(
+    const simulator = await AvmSimulator.create(
       stateManager,
       address,
-      fnName,
       sender,
       transactionFee,
       this.globalVariables,

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

@@ -536,7 +536,7 @@ function createPublicExecutionRequest(avmEnvironment: AvmExecutionEnvironment):
   const callContext = CallContext.from({
     msgSender: avmEnvironment.sender,
     contractAddress: avmEnvironment.address,
-    functionSelector: FunctionSelector.empty(),
+    functionSelector: FunctionSelector.empty(), // TODO(#10563): Remove functionSelector in public call context
     isStaticCall: avmEnvironment.isStaticCall,
   });
   return new PublicExecutionRequest(callContext, avmEnvironment.calldata);