From 3c777a6d7aa7e5f3420cec80b73f89943ea9dea6 Mon Sep 17 00:00:00 2001 From: Facundo Date: Tue, 23 Jan 2024 17:10:59 +0000 Subject: [PATCH] feat(avm): add tests for memory and bitwise instructions (#4184) I have to implement memory tagging first for these to make sense, so I'm merging this and then going back to it. Advances #4120 and #4121. --- .../src/avm/interpreter/interpreter.ts | 1 + .../src/avm/opcodes/bitwise.test.ts | 166 ++++++++++++++++ .../acir-simulator/src/avm/opcodes/bitwise.ts | 3 + .../src/avm/opcodes/control_flow.test.ts | 3 +- .../src/avm/opcodes/instruction_set.ts | 27 ++- .../src/avm/opcodes/memory.test.ts | 182 ++++++++++++++++++ .../acir-simulator/src/avm/opcodes/memory.ts | 30 ++- .../acir-simulator/src/avm/opcodes/opcodes.ts | 118 ++++++------ 8 files changed, 454 insertions(+), 76 deletions(-) create mode 100644 yarn-project/acir-simulator/src/avm/opcodes/bitwise.test.ts create mode 100644 yarn-project/acir-simulator/src/avm/opcodes/memory.test.ts diff --git a/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts b/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts index 8b4dfad3a6b..f3970ade43f 100644 --- a/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts +++ b/yarn-project/acir-simulator/src/avm/interpreter/interpreter.ts @@ -84,5 +84,6 @@ export abstract class AvmInterpreterError extends Error { export class InvalidProgramCounterError extends AvmInterpreterError { constructor(pc: number, max: number) { super(`Invalid program counter ${pc}, max is ${max}`); + this.name = 'InvalidProgramCounterError'; } } diff --git a/yarn-project/acir-simulator/src/avm/opcodes/bitwise.test.ts b/yarn-project/acir-simulator/src/avm/opcodes/bitwise.test.ts new file mode 100644 index 00000000000..e1de7356041 --- /dev/null +++ b/yarn-project/acir-simulator/src/avm/opcodes/bitwise.test.ts @@ -0,0 +1,166 @@ +import { Fr } from '@aztec/foundation/fields'; + +import { mock } from 'jest-mock-extended'; + +import { AvmMachineState } from '../avm_machine_state.js'; +import { AvmStateManager } from '../avm_state_manager.js'; +import { + And, + /*Not,*/ + Or, + Shl, + Shr, + Xor, +} from './bitwise.js'; + +describe('Bitwise instructions', () => { + let machineState: AvmMachineState; + let stateManager = mock(); + + beforeEach(() => { + machineState = new AvmMachineState([]); + stateManager = mock(); + }); + + it('Should AND correctly over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(0b11100100111001001111n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new And(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b11100100010001000100n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + it('Should OR correctly over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(0b11100100111001001111n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Or(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b11111110111011101111n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + it('Should XOR correctly over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(0b11100100111001001111n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Xor(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b00011010101010101011n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + describe('SHR', () => { + it('Should shift correctly 0 positions over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(0n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Shr(0, 1, 2).execute(machineState, stateManager); + + const expected = a; + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + it('Should shift correctly 2 positions over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(2n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Shr(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b00111111100100111001n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + it('Should shift correctly 19 positions over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(19n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Shr(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b01n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + }); + + describe('SHL', () => { + it('Should shift correctly 0 positions over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(0n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Shl(0, 1, 2).execute(machineState, stateManager); + + const expected = a; + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + it('Should shift correctly 2 positions over Fr type', () => { + const a = new Fr(0b11111110010011100100n); + const b = new Fr(2n); + + machineState.writeMemory(0, a); + machineState.writeMemory(1, b); + + new Shl(0, 1, 2).execute(machineState, stateManager); + + const expected = new Fr(0b1111111001001110010000n); + const actual = machineState.readMemory(2); + expect(actual).toEqual(expected); + }); + + // it('Should shift correctly over bit limit over Fr type', () => { + // const a = new Fr(0b11111110010011100100n); + // const b = new Fr(19n); + + // machineState.writeMemory(0, a); + // machineState.writeMemory(1, b); + + // new Shl(0, 1, 2).execute(machineState, stateManager); + + // const expected = new Fr(0b01n); + // const actual = machineState.readMemory(2); + // expect(actual).toEqual(expected); + // }); + }); + + // it('Should NOT correctly over Fr type', () => { + // const a = new Fr(0b11111110010011100100n); + + // machineState.writeMemory(0, a); + + // new Not(0, 1).execute(machineState, stateManager); + + // const expected = new Fr(0b00000001101100011011n); // high bits! + // const actual = machineState.readMemory(1); + // expect(actual).toEqual(expected); + // }); +}); diff --git a/yarn-project/acir-simulator/src/avm/opcodes/bitwise.ts b/yarn-project/acir-simulator/src/avm/opcodes/bitwise.ts index 4950c771f7c..ff7802aac61 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/bitwise.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/bitwise.ts @@ -120,6 +120,9 @@ export class Shr extends Instruction { const a: Fr = machineState.readMemory(this.aOffset); const b: Fr = machineState.readMemory(this.bOffset); + // Here we are assuming that the field element maps to a positive number. + // The >> operator is *signed* in JS (and it sign extends). + // E.g.: -1n >> 3n == -1n. const dest = new Fr(a.toBigInt() >> b.toBigInt()); machineState.writeMemory(this.destOffset, dest); 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 864ee0d0b42..906584b3493 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 @@ -8,7 +8,7 @@ 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, InternalCallStackEmptyError, InternalReturn, Jump, JumpI } from './control_flow.js'; -import { CalldataCopy, Cast, Mov, Set } from './memory.js'; +import { CMov, CalldataCopy, Cast, Mov, Set } from './memory.js'; describe('Control Flow Opcodes', () => { let stateManager = mock(); @@ -132,6 +132,7 @@ describe('Control Flow Opcodes', () => { new CalldataCopy(0, 1, 2), new Set(0n, 1), new Mov(0, 1), + new CMov(0, 1, 2, 3), new Cast(0, 1), ]; diff --git a/yarn-project/acir-simulator/src/avm/opcodes/instruction_set.ts b/yarn-project/acir-simulator/src/avm/opcodes/instruction_set.ts index 4395ba46d53..664467f887d 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/instruction_set.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/instruction_set.ts @@ -1,12 +1,9 @@ import { Add, Div, Mul, Sub } from './arithmetic.js'; -//import { And, Not, Or, Shl, Shr, Xor } from './bitwise.js'; +import { And, Not, Or, Shl, Shr, Xor } from './bitwise.js'; //import { Eq, Lt, Lte } from './comparators.js'; import { InternalCall, InternalReturn, Jump, JumpI, Return } from './control_flow.js'; import { Instruction } from './instruction.js'; -import { - CalldataCopy, - /*Cast, Mov*/ -} from './memory.js'; +import { CMov, CalldataCopy, Cast, Mov, Set } from './memory.js'; import { Opcode } from './opcodes.js'; /** - */ @@ -30,14 +27,14 @@ export const INSTRUCTION_SET: Map = ne //[Opcode.LT, Lt], //[Opcode.LTE, Lte], //// Compute - Bitwise - //[Opcode.AND, And], - //[Opcode.OR, Or], - //[Opcode.XOR, Xor], - //[Opcode.NOT, Not], - //[Opcode.SHL, Shl], - //[Opcode.SHR, Shr], + [Opcode.AND, And], + [Opcode.OR, Or], + [Opcode.XOR, Xor], + [Opcode.NOT, Not], + [Opcode.SHL, Shl], + [Opcode.SHR, Shr], //// Compute - Type Conversions - //[Opcode.CAST, Cast], + [Opcode.CAST, Cast], //// Execution Environment //[Opcode.ADDRESS, Address], @@ -72,9 +69,9 @@ export const INSTRUCTION_SET: Map = ne [Opcode.INTERNALCALL, InternalCall], [Opcode.INTERNALRETURN, InternalReturn], //// Machine State - Memory - //[Opcode.SET, Set], - //[Opcode.MOV, Mov], - //[Opcode.CMOV, CMov], + [Opcode.SET, Set], + [Opcode.MOV, Mov], + [Opcode.CMOV, CMov], //// World State //[Opcode.BLOCKHEADERBYNUMBER, Blockheaderbynumber], diff --git a/yarn-project/acir-simulator/src/avm/opcodes/memory.test.ts b/yarn-project/acir-simulator/src/avm/opcodes/memory.test.ts new file mode 100644 index 00000000000..3b02ef36c32 --- /dev/null +++ b/yarn-project/acir-simulator/src/avm/opcodes/memory.test.ts @@ -0,0 +1,182 @@ +import { Fr } from '@aztec/foundation/fields'; + +import { mock } from 'jest-mock-extended'; + +import { AvmMachineState } from '../avm_machine_state.js'; +import { AvmStateManager } from '../avm_state_manager.js'; +import { CMov, CalldataCopy, Cast, Mov, Set } from './memory.js'; + +describe('Memory instructions', () => { + let machineState: AvmMachineState; + let stateManager = mock(); + + beforeEach(() => { + machineState = new AvmMachineState([]); + stateManager = mock(); + }); + + it('Should SET memory correctly', () => { + const value = 123456n; + + new Set(value, 1).execute(machineState, stateManager); + + const expected = new Fr(value); + const actual = machineState.readMemory(1); + expect(actual).toEqual(expected); + }); + + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/3987): tags are not implemented yet - this will behave as a mov + describe('CAST', () => { + it('Should work correctly on different memory cells', () => { + const value = new Fr(123456n); + + machineState.writeMemory(0, value); + + new Cast(/*aOffset=*/ 0, /*dstOffset=*/ 1).execute(machineState, stateManager); + + const actual = machineState.readMemory(1); + expect(actual).toEqual(value); + }); + + it('Should work correctly on same memory cell', () => { + const value = new Fr(123456n); + + machineState.writeMemory(0, value); + + new Cast(/*aOffset=*/ 0, /*dstOffset=*/ 0).execute(machineState, stateManager); + + const actual = machineState.readMemory(0); + expect(actual).toEqual(value); + }); + }); + + describe('MOV', () => { + it('Should work correctly on different memory cells', () => { + const value = new Fr(123456n); + + machineState.writeMemory(0, value); + + new Mov(/*aOffset=*/ 0, /*dstOffset=*/ 1).execute(machineState, stateManager); + + const actual = machineState.readMemory(1); + expect(actual).toEqual(value); + }); + + it('Should work correctly on same memory cell', () => { + const value = new Fr(123456n); + + machineState.writeMemory(0, value); + + new Mov(/*aOffset=*/ 0, /*dstOffset=*/ 0).execute(machineState, stateManager); + + const actual = machineState.readMemory(0); + expect(actual).toEqual(value); + }); + }); + + describe('MOV', () => { + it('Should move A if COND is true, on different memory cells', () => { + const valueA = new Fr(123456n); + const valueB = new Fr(80n); + const valueCondition = new Fr(22n); + + machineState.writeMemory(0, valueA); + machineState.writeMemory(1, valueB); + machineState.writeMemory(2, valueCondition); + + new CMov(/*aOffset=*/ 0, /*bOffset=*/ 1, /*condOffset=*/ 2, /*dstOffset=*/ 3).execute(machineState, stateManager); + + const actual = machineState.readMemory(3); + expect(actual).toEqual(valueA); + }); + + it('Should move B if COND is false, on different memory cells', () => { + const valueA = new Fr(123456n); + const valueB = new Fr(80n); + const valueCondition = new Fr(0n); + + machineState.writeMemory(0, valueA); + machineState.writeMemory(1, valueB); + machineState.writeMemory(2, valueCondition); + + new CMov(/*aOffset=*/ 0, /*bOffset=*/ 1, /*condOffset=*/ 2, /*dstOffset=*/ 3).execute(machineState, stateManager); + + const actual = machineState.readMemory(3); + expect(actual).toEqual(valueB); + }); + + it('Should move A if COND is true, on overlapping memory cells', () => { + const valueA = new Fr(123456n); + const valueB = new Fr(80n); + const valueCondition = new Fr(22n); + + machineState.writeMemory(0, valueA); + machineState.writeMemory(1, valueB); + machineState.writeMemory(2, valueCondition); + + new CMov(/*aOffset=*/ 0, /*bOffset=*/ 1, /*condOffset=*/ 2, /*dstOffset=*/ 2).execute(machineState, stateManager); + + const actual = machineState.readMemory(2); + expect(actual).toEqual(valueA); + }); + + it('Should move B if COND is false, on overlapping memory cells', () => { + const valueA = new Fr(123456n); + const valueB = new Fr(80n); + const valueCondition = new Fr(0n); + + machineState.writeMemory(0, valueA); + machineState.writeMemory(1, valueB); + machineState.writeMemory(2, valueCondition); + + new CMov(/*aOffset=*/ 0, /*bOffset=*/ 1, /*condOffset=*/ 2, /*dstOffset=*/ 2).execute(machineState, stateManager); + + const actual = machineState.readMemory(2); + expect(actual).toEqual(valueB); + }); + }); + + describe('CALLDATA', () => { + it('Writes nothing if size is 0', () => { + const previousValue = new Fr(123456n); + const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)]; + + machineState = new AvmMachineState(calldata); + machineState.writeMemory(0, previousValue); + + new CalldataCopy(/*cdOffset=*/ 2, /*copySize=*/ 0, /*dstOffset=*/ 0).execute(machineState, stateManager); + + const actual = machineState.readMemory(0); + expect(actual).toEqual(previousValue); + }); + + it('Copies all calldata', () => { + const previousValue = new Fr(123456n); + const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)]; + + machineState = new AvmMachineState(calldata); + machineState.writeMemory(0, previousValue); + + new CalldataCopy(/*cdOffset=*/ 0, /*copySize=*/ 3, /*dstOffset=*/ 0).execute(machineState, stateManager); + + const actual = machineState.readMemoryChunk(/*offset=*/ 0, /*size=*/ 3); + expect(actual).toEqual(calldata); + }); + + it('Copies slice of calldata', () => { + const previousValue = new Fr(123456n); + const calldata = [new Fr(1n), new Fr(2n), new Fr(3n)]; + + machineState = new AvmMachineState(calldata); + machineState.writeMemory(0, previousValue); + + new CalldataCopy(/*cdOffset=*/ 1, /*copySize=*/ 2, /*dstOffset=*/ 0).execute(machineState, stateManager); + + const expected = calldata.slice(1); + const actual = machineState.readMemoryChunk(/*offset=*/ 0, /*size=*/ 2); + expect(actual).toEqual(expected); + }); + + // TODO: check bad cases (i.e., out of bounds) + }); +}); diff --git a/yarn-project/acir-simulator/src/avm/opcodes/memory.ts b/yarn-project/acir-simulator/src/avm/opcodes/memory.ts index c856645eef9..3e6bbc62afd 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/memory.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/memory.ts @@ -9,13 +9,12 @@ export class Set extends Instruction { static type: string = 'SET'; static numberOfOperands = 2; - constructor(private constt: bigint, private destOffset: number) { + constructor(private value: bigint, private destOffset: number) { super(); } execute(machineState: AvmMachineState, _stateManager: AvmStateManager): void { - const dest = new Fr(this.constt); - machineState.writeMemory(this.destOffset, dest); + machineState.writeMemory(this.destOffset, new Fr(this.value)); this.incrementPc(machineState); } @@ -58,6 +57,31 @@ export class Mov extends Instruction { } } +/** - */ +export class CMov extends Instruction { + static type: string = 'MOV'; + static numberOfOperands = 4; + + constructor( + private aOffset: number, + private bOffset: number, + private condOffset: number, + private destOffset: number, + ) { + super(); + } + + execute(machineState: AvmMachineState, _stateManager: AvmStateManager): void { + const a = machineState.readMemory(this.aOffset); + const b = machineState.readMemory(this.bOffset); + const cond = machineState.readMemory(this.condOffset); + + machineState.writeMemory(this.destOffset, cond.toBigInt() ? a : b); + + this.incrementPc(machineState); + } +} + /** - */ export class CalldataCopy extends Instruction { static type: string = 'CALLDATACOPY'; diff --git a/yarn-project/acir-simulator/src/avm/opcodes/opcodes.ts b/yarn-project/acir-simulator/src/avm/opcodes/opcodes.ts index e0760e10213..6f3f3709007 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/opcodes.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/opcodes.ts @@ -1,83 +1,87 @@ /** - * All AVM opcodes + * All AVM opcodes. + * Source: https://yp-aztec.netlify.app/docs/public-vm/instruction-set */ export enum Opcode { // Compute // Compute - Arithmetic - ADD, - SUB, - MUL, - DIV, + ADD = 0x00, + SUB = 0x01, + MUL = 0x02, + DIV = 0x03, // Compute - Comparators - EQ, - LT, - LTE, + EQ = 0x04, + LT = 0x05, + LTE = 0x06, // Compute - Bitwise - AND, - OR, - XOR, - NOT, - SHL, - SHR, + AND = 0x07, + OR = 0x08, + XOR = 0x09, + NOT = 0x0a, + SHL = 0x0b, + SHR = 0x0c, // Compute - Type Conversions - CAST, + CAST = 0x0d, // Execution Environment - ADDRESS, - STORAGEADDRESS, - ORIGIN, - SENDER, - PORTAL, - FEEPERL1GAS, - FEEPERL2GAS, - FEEPERDAGAS, - CONTRACTCALLDEPTH, + ADDRESS = 0x0e, + STORAGEADDRESS = 0x0f, + ORIGIN = 0x10, + SENDER = 0x11, + PORTAL = 0x12, + FEEPERL1GAS = 0x13, + FEEPERL2GAS = 0x14, + FEEPERDAGAS = 0x15, + CONTRACTCALLDEPTH = 0x16, // Execution Environment - Globals - CHAINID, - VERSION, - BLOCKNUMBER, - TIMESTAMP, - COINBASE, - BLOCKL1GASLIMIT, - BLOCKL2GASLIMIT, - BLOCKDAGASLIMIT, + CHAINID = 0x17, + VERSION = 0x18, + BLOCKNUMBER = 0x19, + TIMESTAMP = 0x1a, + COINBASE = 0x1b, + BLOCKL1GASLIMIT = 0x1c, + BLOCKL2GASLIMIT = 0x1d, + BLOCKDAGASLIMIT = 0x1e, // Execution Environment - Calldata - CALLDATACOPY, + CALLDATACOPY = 0x1f, // Machine State // Machine State - Gas - L1GASLEFT, - L2GASLEFT, - DAGASLEFT, + L1GASLEFT = 0x20, + L2GASLEFT = 0x21, + DAGASLEFT = 0x22, // Machine State - Internal Control Flow - JUMP, - JUMPI, - INTERNALCALL, - INTERNALRETURN, + JUMP = 0x23, + JUMPI = 0x24, + INTERNALCALL = 0x25, + INTERNALRETURN = 0x26, // Machine State - Memory - SET, - MOV, - CMOV, + SET = 0x27, + MOV = 0x28, + CMOV = 0x29, // World State - BLOCKHEADERBYNUMBER, - SLOAD, // Public Storage - SSTORE, // Public Storage - READL1TOL2MSG, // Messages - SENDL2TOL1MSG, // Messages - EMITNOTEHASH, // Notes & Nullifiers - EMITNULLIFIER, // Notes & Nullifiers + BLOCKHEADERBYNUMBER = 0x2a, + SLOAD = 0x2b, // Public Storage + SSTORE = 0x2c, // Public Storage + READL1TOL2MSG = 0x2d, // Messages + SENDL2TOL1MSG = 0x2e, // Messages + EMITNOTEHASH = 0x2f, // Notes & Nullifiers + EMITNULLIFIER = 0x30, // Notes & Nullifiers // Accrued Substate - EMITUNENCRYPTEDLOG, + EMITUNENCRYPTEDLOG = 0x31, // Control Flow - Contract Calls - CALL, - STATICCALL, - RETURN, - REVERT, + CALL = 0x32, + STATICCALL = 0x33, + RETURN = 0x34, + REVERT = 0x35, // Gadgets - KECCAK, - POSEIDON, + KECCAK = 0x36, + POSEIDON = 0x37, + + // Add new opcodes before this + TOTAL_OPCODES_NUMBER, }