From bd6e79727bb207978634d70df3bb213a222a4bb7 Mon Sep 17 00:00:00 2001 From: Maddiaa <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 00:30:16 +0000 Subject: [PATCH] feat(avm): revert instruction (#4206) fixes: https://github.com/AztecProtocol/aztec-packages/issues/4123 --- .../src/avm/avm_machine_state.ts | 5 + .../acir-simulator/src/avm/fixtures/index.ts | 47 +--- .../src/avm/interpreter/interpreter.ts | 4 + .../src/avm/opcodes/control_flow.test.ts | 254 ++++++++++-------- .../src/avm/opcodes/control_flow.ts | 24 +- .../src/avm/opcodes/instruction.ts | 5 + 6 files changed, 181 insertions(+), 158 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/avm_machine_state.ts b/yarn-project/acir-simulator/src/avm/avm_machine_state.ts index 4d9fa6bb7fe..75da070f6b5 100644 --- a/yarn-project/acir-simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/acir-simulator/src/avm/avm_machine_state.ts @@ -31,6 +31,10 @@ export class AvmMachineState { * If an instruction triggers a halt, then it ends execution of the VM */ public halted: boolean; + /** + * Signifies if the execution has reverted ( due to a revert instruction ) + */ + public reverted: boolean; /** * Create a new avm context @@ -45,6 +49,7 @@ export class AvmMachineState { this.callStack = []; this.halted = false; + this.reverted = false; this.executionEnvironment = executionEnvironment; } diff --git a/yarn-project/acir-simulator/src/avm/fixtures/index.ts b/yarn-project/acir-simulator/src/avm/fixtures/index.ts index 2eb4ac73854..4c0d889063c 100644 --- a/yarn-project/acir-simulator/src/avm/fixtures/index.ts +++ b/yarn-project/acir-simulator/src/avm/fixtures/index.ts @@ -6,41 +6,10 @@ import { Fr } from '@aztec/foundation/fields'; import { AvmExecutionEnvironment } from '../avm_execution_environment.js'; -/** - * An interface that allows to override the default values of the AvmExecutionEnvironment - */ -export interface AvmExecutionEnvironmentOverrides { - address?: AztecAddress; - - storageAddress?: AztecAddress; - - origin?: AztecAddress; - - sender?: AztecAddress; - - portal?: EthAddress; - - feePerL1Gas?: Fr; - - feePerL2Gas?: Fr; - - feePerDaGas?: Fr; - - contractCallDepth?: Fr; - - globals?: GlobalVariables; - - isStaticCall?: boolean; - - isDelegateCall?: boolean; - - calldata?: Fr[]; -} - /** * Create an empty instance of the Execution Environment where all values are zero, unless overriden in the overrides object */ -export function initExecutionEnvironment(overrides?: AvmExecutionEnvironmentOverrides): AvmExecutionEnvironment { +export function initExecutionEnvironment(overrides?: Partial): AvmExecutionEnvironment { return new AvmExecutionEnvironment( overrides?.address ?? AztecAddress.zero(), overrides?.storageAddress ?? AztecAddress.zero(), @@ -59,19 +28,9 @@ export function initExecutionEnvironment(overrides?: AvmExecutionEnvironmentOver } /** - * An interface that allows to override the default values of the GlobalVariables - */ -export interface GlobalVariablesOverrides { - chainId?: Fr; - version?: Fr; - blockNumber?: Fr; - timestamp?: Fr; -} - -/** - * Create an empty instance of the Global Variables where all values are zero, unless overriden in the overrides object + * Create an empty instance of the Execution Environment where all values are zero, unless overriden in the overrides object */ -export function initGlobalVariables(overrides?: GlobalVariablesOverrides): GlobalVariables { +export function initGlobalVariables(overrides?: Partial): GlobalVariables { return new GlobalVariables( overrides?.chainId ?? Fr.zero(), overrides?.version ?? Fr.zero(), diff --git a/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts b/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts index 75b90eb5d0c..9073a0f0d6d 100644 --- a/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts +++ b/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts @@ -45,6 +45,10 @@ export class AvmInterpreter { } const returnData = this.machineState.getReturnData(); + if (this.machineState.reverted) { + return AvmMessageCallResult.revert(returnData); + } + return AvmMessageCallResult.success(returnData); } catch (_e) { if (!(_e instanceof AvmInterpreterError)) { diff --git a/yarn-project/acir-simulator/src/avm/opcodes/control_flow.test.ts b/yarn-project/acir-simulator/src/avm/opcodes/control_flow.test.ts index 59e24adb042..7c04e08f246 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/control_flow.test.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/control_flow.test.ts @@ -1,13 +1,15 @@ +import { Fr } from '@aztec/foundation/fields'; + import { MockProxy, mock } from 'jest-mock-extended'; import { AvmMachineState } from '../avm_machine_state.js'; -import { TypeTag, Uint16 } from '../avm_memory_types.js'; +import { Field, TypeTag, Uint16 } from '../avm_memory_types.js'; import { initExecutionEnvironment } from '../fixtures/index.js'; import { AvmJournal } from '../journal/journal.js'; import { Add, Mul, Sub } from './arithmetic.js'; import { And, Not, Or, Shl, Shr, Xor } from './bitwise.js'; import { Eq, Lt, Lte } from './comparators.js'; -import { InternalCall, InternalReturn, Jump, JumpI } from './control_flow.js'; +import { InternalCall, InternalReturn, Jump, JumpI, Return, Revert } from './control_flow.js'; import { InstructionExecutionError } from './instruction.js'; import { CMov, CalldataCopy, Cast, Mov, Set } from './memory.js'; @@ -20,132 +22,166 @@ describe('Control Flow Opcodes', () => { machineState = new AvmMachineState(initExecutionEnvironment()); }); - it('Should implement JUMP', async () => { - const jumpLocation = 22; + describe('Jumps', () => { + it('Should implement JUMP', async () => { + const jumpLocation = 22; - expect(machineState.pc).toBe(0); + expect(machineState.pc).toBe(0); - const instruction = new Jump(jumpLocation); - await instruction.execute(machineState, journal); - expect(machineState.pc).toBe(jumpLocation); - }); + const instruction = new Jump(jumpLocation); + await instruction.execute(machineState, journal); + expect(machineState.pc).toBe(jumpLocation); + }); - it('Should implement JUMPI - truthy', async () => { - const jumpLocation = 22; - const jumpLocation1 = 69; + it('Should implement JUMPI - truthy', async () => { + const jumpLocation = 22; + const jumpLocation1 = 69; - expect(machineState.pc).toBe(0); + expect(machineState.pc).toBe(0); - machineState.memory.set(0, new Uint16(1n)); - machineState.memory.set(1, new Uint16(2n)); + machineState.memory.set(0, new Uint16(1n)); + machineState.memory.set(1, new Uint16(2n)); - const instruction = new JumpI(jumpLocation, 0); - await instruction.execute(machineState, journal); - expect(machineState.pc).toBe(jumpLocation); + const instruction = new JumpI(jumpLocation, 0); + await instruction.execute(machineState, journal); + expect(machineState.pc).toBe(jumpLocation); - // Truthy can be greater than 1 - const instruction1 = new JumpI(jumpLocation1, 1); - await instruction1.execute(machineState, journal); - expect(machineState.pc).toBe(jumpLocation1); - }); + // Truthy can be greater than 1 + const instruction1 = new JumpI(jumpLocation1, 1); + await instruction1.execute(machineState, journal); + expect(machineState.pc).toBe(jumpLocation1); + }); + + it('Should implement JUMPI - falsy', async () => { + const jumpLocation = 22; + + expect(machineState.pc).toBe(0); + + machineState.memory.set(0, new Uint16(0n)); - it('Should implement JUMPI - falsy', async () => { - const jumpLocation = 22; + const instruction = new JumpI(jumpLocation, 0); + await instruction.execute(machineState, journal); + expect(machineState.pc).toBe(1); + }); - expect(machineState.pc).toBe(0); + it('Should implement Internal Call and Return', async () => { + const jumpLocation = 22; - machineState.memory.set(0, new Uint16(0n)); + expect(machineState.pc).toBe(0); - const instruction = new JumpI(jumpLocation, 0); - await instruction.execute(machineState, journal); - expect(machineState.pc).toBe(1); + const instruction = new InternalCall(jumpLocation); + const returnInstruction = new InternalReturn(); + + await instruction.execute(machineState, journal); + expect(machineState.pc).toBe(jumpLocation); + + await returnInstruction.execute(machineState, journal); + expect(machineState.pc).toBe(1); + }); + + it('Should chain series of control flow instructions', async () => { + const jumpLocation0 = 22; + const jumpLocation1 = 69; + const jumpLocation2 = 1337; + + const aloneJumpLocation = 420; + + const instructions = [ + // pc | internal call stack + new InternalCall(jumpLocation0), // 22 | [1] + new InternalCall(jumpLocation1), // 69 | [1, 23] + new InternalReturn(), // 23 | [1] + new Jump(aloneJumpLocation), // 420 | [1] + new InternalCall(jumpLocation2), // 1337| [1, 421] + new InternalReturn(), // 421 | [1] + new InternalReturn(), // 1 | [] + ]; + + // The expected program counter after each instruction is invoked + const expectedPcs = [ + jumpLocation0, + jumpLocation1, + jumpLocation0 + 1, + aloneJumpLocation, + jumpLocation2, + aloneJumpLocation + 1, + 1, + ]; + + for (let i = 0; i < instructions.length; i++) { + await instructions[i].execute(machineState, journal); + expect(machineState.pc).toBe(expectedPcs[i]); + } + }); + + it('Should error if Internal Return is called without a corresponding Internal Call', async () => { + const returnInstruction = () => new InternalReturn().execute(machineState, journal); + await expect(returnInstruction()).rejects.toThrow(InstructionExecutionError); + }); + + it('Should increment PC on All other Instructions', async () => { + const instructions = [ + new Add(0, 1, 2), + new Sub(0, 1, 2), + new Mul(0, 1, 2), + new Lt(TypeTag.UINT16, 0, 1, 2), + new Lte(TypeTag.UINT16, 0, 1, 2), + new Eq(TypeTag.UINT16, 0, 1, 2), + new Xor(TypeTag.UINT16, 0, 1, 2), + new And(TypeTag.UINT16, 0, 1, 2), + new Or(TypeTag.UINT16, 0, 1, 2), + new Shl(TypeTag.UINT16, 0, 1, 2), + new Shr(TypeTag.UINT16, 0, 1, 2), + new Not(TypeTag.UINT16, 0, 2), + new CalldataCopy(0, 1, 2), + new Set(TypeTag.UINT16, 0n, 1), + new Mov(0, 1), + new CMov(0, 1, 2, 3), + new Cast(TypeTag.UINT16, 0, 1), + ]; + + for (const instruction of instructions) { + // Use a fresh machine state each run + const innerMachineState = new AvmMachineState(initExecutionEnvironment()); + innerMachineState.memory.set(0, new Uint16(4n)); + innerMachineState.memory.set(1, new Uint16(8n)); + innerMachineState.memory.set(2, new Uint16(12n)); + expect(innerMachineState.pc).toBe(0); + + await instruction.execute(innerMachineState, journal); + } + }); }); - it('Should implement Internal Call and Return', async () => { - const jumpLocation = 22; + describe('Halting Opcodes', () => { + it('Should return data from the return opcode', async () => { + const returnData = [new Fr(1n), new Fr(2n), new Fr(3n)]; - expect(machineState.pc).toBe(0); + machineState.memory.set(0, new Field(1n)); + machineState.memory.set(1, new Field(2n)); + machineState.memory.set(2, new Field(3n)); - const instruction = new InternalCall(jumpLocation); - const returnInstruction = new InternalReturn(); + const instruction = new Return(0, returnData.length); + await instruction.execute(machineState, journal); - await instruction.execute(machineState, journal); - expect(machineState.pc).toBe(jumpLocation); + expect(machineState.getReturnData()).toEqual(returnData); + expect(machineState.halted).toBe(true); + expect(machineState.reverted).toBe(false); + }); - await returnInstruction.execute(machineState, journal); - expect(machineState.pc).toBe(1); - }); + it('Should return data and revert from the revert opcode', async () => { + const returnData = [new Fr(1n), new Fr(2n), new Fr(3n)]; - it('Should chain series of control flow instructions', async () => { - const jumpLocation0 = 22; - const jumpLocation1 = 69; - const jumpLocation2 = 1337; - - const aloneJumpLocation = 420; - - const instructions = [ - // pc | internal call stack - new InternalCall(jumpLocation0), // 22 | [1] - new InternalCall(jumpLocation1), // 69 | [1, 23] - new InternalReturn(), // 23 | [1] - new Jump(aloneJumpLocation), // 420 | [1] - new InternalCall(jumpLocation2), // 1337| [1, 421] - new InternalReturn(), // 421 | [1] - new InternalReturn(), // 1 | [] - ]; - - // The expected program counter after each instruction is invoked - const expectedPcs = [ - jumpLocation0, - jumpLocation1, - jumpLocation0 + 1, - aloneJumpLocation, - jumpLocation2, - aloneJumpLocation + 1, - 1, - ]; - - for (let i = 0; i < instructions.length; i++) { - await instructions[i].execute(machineState, journal); - expect(machineState.pc).toBe(expectedPcs[i]); - } - }); + machineState.memory.set(0, new Field(1n)); + machineState.memory.set(1, new Field(2n)); + machineState.memory.set(2, new Field(3n)); - it('Should error if Internal Return is called without a corresponding Internal Call', async () => { - const returnInstruction = () => new InternalReturn().execute(machineState, journal); - await expect(returnInstruction()).rejects.toThrow(InstructionExecutionError); - }); + const instruction = new Revert(0, returnData.length); + await instruction.execute(machineState, journal); - it('Should increment PC on All other Instructions', async () => { - const instructions = [ - new Add(0, 1, 2), - new Sub(0, 1, 2), - new Mul(0, 1, 2), - new Lt(TypeTag.UINT16, 0, 1, 2), - new Lte(TypeTag.UINT16, 0, 1, 2), - new Eq(TypeTag.UINT16, 0, 1, 2), - new Xor(TypeTag.UINT16, 0, 1, 2), - new And(TypeTag.UINT16, 0, 1, 2), - new Or(TypeTag.UINT16, 0, 1, 2), - new Shl(TypeTag.UINT16, 0, 1, 2), - new Shr(TypeTag.UINT16, 0, 1, 2), - new Not(TypeTag.UINT16, 0, 2), - new CalldataCopy(0, 1, 2), - new Set(TypeTag.UINT16, 0n, 1), - new Mov(0, 1), - new CMov(0, 1, 2, 3), - new Cast(TypeTag.UINT16, 0, 1), - ]; - - for (const instruction of instructions) { - // Use a fresh machine state each run - const innerMachineState = new AvmMachineState(initExecutionEnvironment()); - innerMachineState.memory.set(0, new Uint16(4n)); - innerMachineState.memory.set(1, new Uint16(8n)); - innerMachineState.memory.set(2, new Uint16(12n)); - expect(machineState.pc).toBe(0); - await instruction.execute(innerMachineState, journal); - expect(innerMachineState.pc).toBe(1); - } + expect(machineState.getReturnData()).toEqual(returnData); + expect(machineState.halted).toBe(true); + expect(machineState.reverted).toBe(true); + }); }); }); diff --git a/yarn-project/acir-simulator/src/avm/opcodes/control_flow.ts b/yarn-project/acir-simulator/src/avm/opcodes/control_flow.ts index 6bbee244cbb..c890fc700e3 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/control_flow.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/control_flow.ts @@ -1,5 +1,3 @@ -import { Fr } from '@aztec/foundation/fields'; - import { AvmMachineState } from '../avm_machine_state.js'; import { IntegralValue } from '../avm_memory_types.js'; import { AvmJournal } from '../journal/journal.js'; @@ -14,9 +12,7 @@ export class Return extends Instruction { } async execute(machineState: AvmMachineState, _journal: AvmJournal): Promise { - const returnData = machineState.memory - .getSlice(this.returnOffset, this.copySize) - .map(word => new Fr(word.toBigInt())); + const returnData = machineState.memory.getSlice(this.returnOffset, this.copySize).map(word => word.toFr()); machineState.setReturnData(returnData); @@ -24,6 +20,24 @@ export class Return extends Instruction { } } +export class Revert extends Instruction { + static type: string = 'RETURN'; + static numberOfOperands = 2; + + constructor(private returnOffset: number, private retSize: number) { + super(); + } + + async execute(machineState: AvmMachineState, _journal: AvmJournal): Promise { + const returnData = machineState.memory + .getSlice(this.returnOffset, this.returnOffset + this.retSize) + .map(word => word.toFr()); + machineState.setReturnData(returnData); + + this.revert(machineState); + } +} + export class Jump extends Instruction { static type: string = 'JUMP'; static numberOfOperands = 1; diff --git a/yarn-project/acir-simulator/src/avm/opcodes/instruction.ts b/yarn-project/acir-simulator/src/avm/opcodes/instruction.ts index 434bfc257db..8eb73212fd2 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/instruction.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/instruction.ts @@ -19,6 +19,11 @@ export abstract class Instruction { machineState.halted = true; } + revert(machineState: AvmMachineState): void { + machineState.halted = true; + machineState.reverted = true; + } + static checkTags(machineState: AvmMachineState, tag: TypeTag, ...offsets: number[]) { for (const offset of offsets) { checkTag(machineState, tag, offset);