Skip to content

Commit

Permalink
Remove function selector from AvmExecutionEnvironment
Browse files Browse the repository at this point in the history
  • Loading branch information
jeanmon committed Dec 9, 2024
1 parent 85c0676 commit b296370
Show file tree
Hide file tree
Showing 16 changed files with 65 additions and 81 deletions.
5 changes: 3 additions & 2 deletions yarn-project/circuit-types/src/simulation_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface FailingFunction {
/**
* The selector of the function that failed.
*/
functionSelector: FunctionSelector;
functionSelector?: FunctionSelector;
/**
* The name of the function that failed.
*/
Expand Down Expand Up @@ -160,6 +160,7 @@ export class SimulationError extends Error {
this.functionErrorStack.forEach(failingFunction => {
if (
failingFunction.contractAddress.equals(contractAddress) &&
failingFunction.functionSelector != null &&
failingFunction.functionSelector.equals(functionSelector)
) {
failingFunction.functionName = functionName;
Expand All @@ -175,7 +176,7 @@ export class SimulationError extends Error {
const stackLines: string[] = [
...functionCallStack.map(failingFunction => {
return `at ${failingFunction.contractName ?? failingFunction.contractAddress.toString()}.${
failingFunction.functionName ?? failingFunction.functionSelector.toString()
failingFunction.functionName ?? failingFunction.functionSelector?.toString() ?? ''
}`;
}),
...noirCallStack.map(errorLocation =>
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/pxe/src/pxe_service/error_enriching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas
if (!mentionedFunctions.has(contractAddress.toString())) {
mentionedFunctions.set(contractAddress.toString(), new Set());
}
mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString());
mentionedFunctions.get(contractAddress.toString())!.add(functionSelector?.toString() ?? '');
});

await Promise.all(
Expand Down Expand Up @@ -89,7 +89,9 @@ export async function enrichPublicSimulationError(
err.setNoirCallStack(parsedCallStack);
} catch (err) {
logger.warn(
`Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${originalFailingFunction.functionSelector.toString()}: ${err}`,
`Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${
originalFailingFunction.functionName?.toString() ?? ''
}: ${err}`,
);
}
}
Expand Down
16 changes: 14 additions & 2 deletions yarn-project/simulator/src/avm/avm_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ 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');
const newContext = context.createNestedContractCallContext(
newAddress,
newCalldata,
allocatedGas,
'CALL',
'top level function',
);

expect(newContext.environment).toEqual(
allSameExcept(context.environment, {
Expand Down Expand Up @@ -39,7 +45,13 @@ 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');
const newContext = context.createNestedContractCallContext(
newAddress,
newCalldata,
allocatedGas,
'STATICCALL',
'static func',
);

expect(newContext.environment).toEqual(
allSameExcept(context.environment, {
Expand Down
7 changes: 4 additions & 3 deletions yarn-project/simulator/src/avm/avm_context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type AztecAddress, FunctionSelector } from '@aztec/circuits.js';
import { type AztecAddress } from '@aztec/circuits.js';
import { type Fr } from '@aztec/foundation/fields';

import { type AvmExecutionEnvironment } from './avm_execution_environment.js';
Expand All @@ -16,6 +16,7 @@ export class AvmContext {
* @param persistableState - Manages world state and accrued substate during execution - (caching, fetching, tracing)
* @param environment - Contains constant variables provided by the kernel
* @param machineState - VM state that is modified on an instruction-by-instruction basis
* @param fnName - The function name which initiated this context.
* @returns new AvmContext instance
*/
constructor(
Expand Down Expand Up @@ -43,13 +44,13 @@ export class AvmContext {
calldata: Fr[],
allocatedGas: Gas,
callType: 'CALL' | 'STATICCALL',
functionSelector: FunctionSelector = FunctionSelector.empty(),
fnName: string,
): AvmContext {
const deriveFn =
callType === 'CALL'
? this.environment.deriveEnvironmentForNestedCall
: this.environment.deriveEnvironmentForNestedStaticCall;
const newExecutionEnvironment = deriveFn.call(this.environment, address, calldata, functionSelector);
const newExecutionEnvironment = deriveFn.call(this.environment, address, calldata, fnName);
const forkedWorldState = this.persistableState.fork();
const machineState = AvmMachineState.fromState(gasToGasLeft(allocatedGas));
return new AvmContext(forkedWorldState, newExecutionEnvironment, machineState);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import { AztecAddress, FunctionSelector } from '@aztec/circuits.js';
import { AztecAddress } from '@aztec/circuits.js';
import { Fr } from '@aztec/foundation/fields';

import { allSameExcept, initExecutionEnvironment } from './fixtures/index.js';

describe('Execution Environment', () => {
const newAddress = AztecAddress.fromNumber(123456);
const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)];
const selector = FunctionSelector.empty();

it('New call should fork execution environment correctly', () => {
const executionEnvironment = initExecutionEnvironment();
const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedCall(newAddress, calldata, selector);
const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedCall(newAddress, calldata, 'func');

expect(newExecutionEnvironment).toEqual(
allSameExcept(executionEnvironment, {
Expand All @@ -26,7 +25,7 @@ describe('Execution Environment', () => {
const newExecutionEnvironment = executionEnvironment.deriveEnvironmentForNestedStaticCall(
newAddress,
calldata,
selector,
'static func',
);

expect(newExecutionEnvironment).toEqual(
Expand Down
26 changes: 8 additions & 18 deletions yarn-project/simulator/src/avm/avm_execution_environment.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FunctionSelector, type GlobalVariables } from '@aztec/circuits.js';
import { type GlobalVariables } from '@aztec/circuits.js';
import { type AztecAddress } from '@aztec/foundation/aztec-address';
import { Fr } from '@aztec/foundation/fields';

Expand All @@ -10,7 +10,7 @@ export class AvmExecutionEnvironment {
constructor(
public readonly address: AztecAddress,
public readonly sender: AztecAddress,
public readonly functionSelector: FunctionSelector, // may be temporary (#7224)
public readonly fnName: string,
public readonly contractCallDepth: Fr,
public readonly transactionFee: Fr,
public readonly globals: GlobalVariables,
Expand All @@ -21,13 +21,13 @@ export class AvmExecutionEnvironment {
private deriveEnvironmentForNestedCallInternal(
targetAddress: AztecAddress,
calldata: Fr[],
functionSelector: FunctionSelector,
fnName: string,
isStaticCall: boolean,
) {
return new AvmExecutionEnvironment(
/*address=*/ targetAddress,
/*sender=*/ this.address,
functionSelector,
fnName,
this.contractCallDepth.add(Fr.ONE),
this.transactionFee,
this.globals,
Expand All @@ -39,26 +39,16 @@ export class AvmExecutionEnvironment {
public deriveEnvironmentForNestedCall(
targetAddress: AztecAddress,
calldata: Fr[],
functionSelector: FunctionSelector = FunctionSelector.empty(),
fnName: string,
): AvmExecutionEnvironment {
return this.deriveEnvironmentForNestedCallInternal(
targetAddress,
calldata,
functionSelector,
/*isStaticCall=*/ false,
);
return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, fnName, /*isStaticCall=*/ false);
}

public deriveEnvironmentForNestedStaticCall(
targetAddress: AztecAddress,
calldata: Fr[],
functionSelector: FunctionSelector,
fnName: string,
): AvmExecutionEnvironment {
return this.deriveEnvironmentForNestedCallInternal(
targetAddress,
calldata,
functionSelector,
/*isStaticCall=*/ true,
);
return this.deriveEnvironmentForNestedCallInternal(targetAddress, calldata, fnName, /*isStaticCall=*/ true);
}
}
3 changes: 0 additions & 3 deletions yarn-project/simulator/src/avm/avm_simulator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const ephemeralTrees = await AvmEphemeralForest.create(worldStateDB.getMerkleInterface());
const persistableState = initPersistableStateManager({ worldStateDB, trace, merkleTrees: ephemeralTrees });
const environment = initExecutionEnvironment({
functionSelector,
calldata,
globals,
address: contractInstance.address,
Expand Down Expand Up @@ -434,7 +433,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
describe('Environment getters', () => {
const address = AztecAddress.random();
const sender = AztecAddress.random();
const functionSelector = FunctionSelector.random();
const transactionFee = Fr.random();
const chainId = Fr.random();
const version = Fr.random();
Expand All @@ -453,7 +451,6 @@ describe('AVM simulator: transpiled Noir contracts', () => {
const env = initExecutionEnvironment({
address,
sender,
functionSelector,
transactionFee,
globals,
});
Expand Down
16 changes: 5 additions & 11 deletions yarn-project/simulator/src/avm/avm_simulator.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import {
type AztecAddress,
Fr,
type FunctionSelector,
type GlobalVariables,
MAX_L2_GAS_PER_ENQUEUED_CALL,
} from '@aztec/circuits.js';
import { type AztecAddress, Fr, type GlobalVariables, MAX_L2_GAS_PER_ENQUEUED_CALL } from '@aztec/circuits.js';
import { type DebugLogger, createDebugLogger } from '@aztec/foundation/log';

import { strict as assert } from 'assert';
Expand Down Expand Up @@ -54,7 +48,7 @@ export class AvmSimulator {
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 = createDebugLogger(`aztec:avm_simulator:core(f:${context.environment.functionSelector.toString()})`);
this.log = createDebugLogger(`aztec:avm_simulator:core(f:${this.context.environment.fnName})`);
// TODO(palla/log): Should tallies be printed on debug, or only on trace?
if (this.log.isLevelEnabled('debug')) {
this.tallyPrintFunction = this.printOpcodeTallies;
Expand All @@ -65,8 +59,8 @@ export class AvmSimulator {
public static create(
stateManager: AvmPersistableStateManager,
address: AztecAddress,
fnName: string,
sender: AztecAddress,
functionSelector: FunctionSelector, // may be temporary (#7224)
transactionFee: Fr,
globals: GlobalVariables,
isStaticCall: boolean,
Expand All @@ -76,7 +70,7 @@ export class AvmSimulator {
const avmExecutionEnv = new AvmExecutionEnvironment(
address,
sender,
functionSelector,
fnName,
/*contractCallDepth=*/ Fr.zero(),
transactionFee,
globals,
Expand All @@ -102,7 +96,7 @@ export class AvmSimulator {
message,
/*failingFunction=*/ {
contractAddress: this.context.environment.address,
functionSelector: this.context.environment.functionSelector,
functionName: this.context.environment.fnName,
},
/*noirCallStack=*/ [],
);
Expand Down
11 changes: 2 additions & 9 deletions yarn-project/simulator/src/avm/errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type FailingFunction, type NoirCallStack } from '@aztec/circuit-types';
import { type AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js';
import { type AztecAddress, type Fr } from '@aztec/circuits.js';

import { ExecutionError } from '../common/errors.js';
import { type AvmContext } from './avm_context.js';
Expand Down Expand Up @@ -139,15 +139,8 @@ export class AvmRevertReason extends ExecutionError {
}

function createRevertReason(message: string, revertData: Fr[], context: AvmContext): AvmRevertReason {
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this.
// If the function selector is the public dispatch selector, we need to extract the actual function selector from the calldata.
// We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention.
let functionSelector = context.environment.functionSelector;
// We drop the returnPc information.
const internalCallStack = context.machineState.internalCallStack.map(entry => entry.callPc);
if (functionSelector.toField().equals(new Fr(PUBLIC_DISPATCH_SELECTOR)) && context.environment.calldata.length > 0) {
functionSelector = FunctionSelector.fromField(context.environment.calldata[0]);
}

// If we are reverting due to the same error that we have been tracking, we use the nested error as the cause.
let nestedError = undefined;
Expand All @@ -164,7 +157,7 @@ function createRevertReason(message: string, revertData: Fr[], context: AvmConte
message,
/*failingFunction=*/ {
contractAddress: context.environment.address,
functionSelector: functionSelector,
functionName: context.environment.fnName,
},
/*noirCallStack=*/ [...internalCallStack, context.machineState.pc].map(pc => `0.${pc}`),
/*options=*/ { cause: nestedError },
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/avm/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function initExecutionEnvironment(overrides?: Partial<AvmExecutionEnviron
return new AvmExecutionEnvironment(
overrides?.address ?? AztecAddress.zero(),
overrides?.sender ?? AztecAddress.zero(),
overrides?.functionSelector ?? FunctionSelector.empty(),
overrides?.fnName ?? '',
overrides?.contractCallDepth ?? Fr.zero(),
overrides?.transactionFee ?? Fr.zero(),
overrides?.globals ?? GlobalVariables.empty(),
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,6 @@ export class AvmPersistableStateManager {
const functionName = await getPublicFunctionDebugName(
this.worldStateDB,
nestedEnvironment.address,
nestedEnvironment.functionSelector,
nestedEnvironment.calldata,
);

Expand All @@ -706,6 +705,10 @@ export class AvmPersistableStateManager {
public traceEnqueuedCall(publicCallRequest: PublicCallRequest, calldata: Fr[], reverted: boolean) {
this.trace.traceEnqueuedCall(publicCallRequest, calldata, reverted);
}

public async getPublicFunctionDebugName(contractAddress: AztecAddress, calldata: Fr[]): Promise<string> {
return await getPublicFunctionDebugName(this.worldStateDB, contractAddress, calldata);
}
}

function contractAddressIsCanonical(contractAddress: AztecAddress): boolean {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { GasFees } from '@aztec/circuits.js';
import { FunctionSelector as FunctionSelectorType } from '@aztec/foundation/abi';
import { AztecAddress } from '@aztec/foundation/aztec-address';
import { Fr } from '@aztec/foundation/fields';

Expand All @@ -14,7 +13,6 @@ import { EnvironmentVariable, GetEnvVar } from './environment_getters.js';
describe('Environment getters', () => {
const address = AztecAddress.random();
const sender = AztecAddress.random();
const functionSelector = FunctionSelectorType.random();
const transactionFee = Fr.random();
const chainId = Fr.random();
const version = Fr.random();
Expand All @@ -34,7 +32,6 @@ describe('Environment getters', () => {
const env = initExecutionEnvironment({
address,
sender,
functionSelector,
transactionFee,
globals,
isStaticCall,
Expand Down
10 changes: 6 additions & 4 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Fr, FunctionSelector, Gas, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js';
import { Gas } from '@aztec/circuits.js';

import type { AvmContext } from '../avm_context.js';
import { type AvmContractCallResult } from '../avm_contract_call_result.js';
Expand Down Expand Up @@ -45,7 +45,6 @@ abstract class ExternalCall extends Instruction {

const callAddress = memory.getAs<Field>(addrOffset);
const calldata = memory.getSlice(argsOffset, calldataSize).map(f => f.toFr());
const functionSelector = new Fr(PUBLIC_DISPATCH_SELECTOR);
// If we are already in a static call, we propagate the environment.
const callType = context.environment.isStaticCall ? 'STATICCALL' : this.type;

Expand All @@ -62,12 +61,15 @@ abstract class ExternalCall extends Instruction {
const allocatedGas = { l2Gas: allocatedL2Gas, daGas: allocatedDaGas };
context.machineState.consumeGas(allocatedGas);

const aztecAddress = callAddress.toAztecAddress();
const fnName = await context.persistableState.getPublicFunctionDebugName(aztecAddress, calldata);

const nestedContext = context.createNestedContractCallContext(
callAddress.toAztecAddress(),
aztecAddress,
calldata,
allocatedGas,
callType,
FunctionSelector.fromField(functionSelector),
fnName,
);

const simulator = new AvmSimulator(nestedContext);
Expand Down
Loading

0 comments on commit b296370

Please sign in to comment.