Skip to content

Commit

Permalink
Address comments from review
Browse files Browse the repository at this point in the history
  • Loading branch information
spalladino committed Apr 3, 2024
1 parent 2dcd0bc commit 602b3e5
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 25 deletions.
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/avm/avm_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('Avm Context', () => {

const newAddress = AztecAddress.random();
const newCalldata = [new Fr(1), new Fr(2)];
const allocatedGas = { l1Gas: 1, l2Gas: 2, daGas: 3 };
const allocatedGas = { l1Gas: 1, 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');

expect(newContext.environment).toEqual(
Expand Down
19 changes: 11 additions & 8 deletions yarn-project/simulator/src/avm/avm_gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,20 @@ export function makeGasCost(gasCost: Partial<Gas>) {
return { ...EmptyGas, ...gasCost };
}

/** Adds multiple instances of Gas. */
export function addGas(...gases: Partial<Gas>[]) {
return {
l1Gas: gases.reduce((acc, gas) => acc + (gas.l1Gas ?? 0), 0),
l2Gas: gases.reduce((acc, gas) => acc + (gas.l2Gas ?? 0), 0),
daGas: gases.reduce((acc, gas) => acc + (gas.daGas ?? 0), 0),
};
/** Sums together multiple instances of Gas. */
export function sumGas(...gases: Partial<Gas>[]) {
return gases.reduce(
(acc: Gas, gas) => ({
l1Gas: acc.l1Gas + (gas.l1Gas ?? 0),
l2Gas: acc.l2Gas + (gas.l2Gas ?? 0),
daGas: acc.daGas + (gas.daGas ?? 0),
}),
EmptyGas,
);
}

/** Zero gas across all gas dimensions. */
export const EmptyGas = {
export const EmptyGas: Gas = {
l1Gas: 0,
l2Gas: 0,
daGas: 0,
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/simulator/src/avm/opcodes/arithmetic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import type { AvmContext } from '../avm_context.js';
import {
type Gas,
GasCostConstants,
addGas,
getCostFromIndirectAccess,
getGasCostMultiplierFromTypeTag,
sumGas,
} from '../avm_gas.js';
import { type Field, type MemoryValue, TypeTag } from '../avm_memory_types.js';
import { Opcode, OperandType } from '../serialization/instruction_serialization.js';
Expand All @@ -29,7 +29,7 @@ export abstract class ThreeOperandArithmeticInstruction extends ThreeOperandInst
l2Gas: getGasCostMultiplierFromTypeTag(this.inTag) * GasCostConstants.ARITHMETIC_COST_PER_BYTE,
};
const indirectCost = getCostFromIndirectAccess(this.indirect);
return addGas(arithmeticCost, indirectCost);
return sumGas(arithmeticCost, indirectCost);
}

protected abstract compute(a: MemoryValue, b: MemoryValue): MemoryValue;
Expand Down
32 changes: 18 additions & 14 deletions yarn-project/simulator/src/avm/opcodes/external_calls.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FunctionSelector } from '@aztec/circuits.js';

import type { AvmContext } from '../avm_context.js';
import { type Gas, addGas, gasLeftToGas, getCostFromIndirectAccess, getFixedGasCost } from '../avm_gas.js';
import { type Gas, gasLeftToGas, getCostFromIndirectAccess, getFixedGasCost, sumGas } from '../avm_gas.js';
import { Field, Uint8 } from '../avm_memory_types.js';
import { AvmSimulator } from '../avm_simulator.js';
import { Opcode, OperandType } from '../serialization/instruction_serialization.js';
Expand Down Expand Up @@ -49,7 +49,7 @@ abstract class ExternalCall extends Instruction {

const callAddress = context.machineState.memory.getAs<Field>(addrOffset);
const calldata = context.machineState.memory.getSlice(argsOffset, this.argsSize).map(f => f.toFr());
const l1Gas = context.machineState.memory.getAs<Field>(gasOffset).toNumber();
const l1Gas = context.machineState.memory.get(gasOffset).toNumber();
const l2Gas = context.machineState.memory.getAs<Field>(gasOffset + 1).toNumber();
const daGas = context.machineState.memory.getAs<Field>(gasOffset + 2).toNumber();
const functionSelector = context.machineState.memory.getAs<Field>(this.temporaryFunctionSelectorOffset).toFr();
Expand All @@ -58,7 +58,7 @@ abstract class ExternalCall extends Instruction {
const baseGas = getFixedGasCost(this.opcode);
const addressingGasCost = getCostFromIndirectAccess(this.indirect);
const allocatedGas = { l1Gas, l2Gas, daGas };
context.machineState.consumeGas(addGas(baseGas, addressingGasCost, allocatedGas));
context.machineState.consumeGas(sumGas(baseGas, addressingGasCost, allocatedGas));

const nestedContext = context.createNestedContractCallContext(
callAddress.toFr(),
Expand Down Expand Up @@ -92,31 +92,35 @@ abstract class ExternalCall extends Instruction {
context.machineState.incrementPc();
}

public get type(): 'CALL' | 'STATICCALL' {
const type = super.type;
if (type !== 'CALL' && type !== 'STATICCALL') {
throw new Error(`Invalid type for ExternalCall instruction: ${type}`);
}
return type;
}
public abstract get type(): 'CALL' | 'STATICCALL';

protected execute(_context: AvmContext): Promise<void> {
throw new Error(`Unimplemented`);
throw new Error(
`Instructions with dynamic gas calculation run all logic on the main execute function and do not override the internal execute.`,
);
}

protected gasCost(): Gas {
throw new Error(`Unimplemented`);
throw new Error(`Instructions with dynamic gas calculation compute gas as part of the main execute function.`);
}
}

export class Call extends ExternalCall {
static type: string = 'CALL';
static type = 'CALL' as const;
static readonly opcode: Opcode = Opcode.CALL;

public get type() {
return Call.type;
}
}

export class StaticCall extends ExternalCall {
static type: string = 'STATICCALL';
static type = 'STATICCALL' as const;
static readonly opcode: Opcode = Opcode.STATICCALL;

public get type() {
return StaticCall.type;
}
}

export class Return extends Instruction {
Expand Down

0 comments on commit 602b3e5

Please sign in to comment.