From 2daae46171f098025cfe057535e0423a37d28424 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 30 Oct 2024 10:51:06 +0000 Subject: [PATCH 01/13] feat(avm)!: byte indexed PC --- .../simulator/src/avm/avm_machine_state.ts | 9 +- .../simulator/src/avm/avm_simulator.ts | 22 ++--- .../src/avm/opcodes/accrued_substate.ts | 7 -- .../simulator/src/avm/opcodes/arithmetic.ts | 1 - .../simulator/src/avm/opcodes/bitwise.ts | 2 - .../simulator/src/avm/opcodes/comparators.ts | 1 - .../simulator/src/avm/opcodes/contract.ts | 1 - .../simulator/src/avm/opcodes/control_flow.ts | 22 ++++- .../simulator/src/avm/opcodes/conversion.ts | 1 - .../simulator/src/avm/opcodes/ec_add.ts | 1 - .../src/avm/opcodes/environment_getters.ts | 1 - .../src/avm/opcodes/external_calls.ts | 1 - .../simulator/src/avm/opcodes/hashing.ts | 3 - .../simulator/src/avm/opcodes/instruction.ts | 7 ++ .../simulator/src/avm/opcodes/memory.ts | 6 -- .../simulator/src/avm/opcodes/misc.ts | 1 - .../src/avm/opcodes/multi_scalar_mul.ts | 1 - .../simulator/src/avm/opcodes/storage.ts | 2 - .../bytecode_serialization.test.ts | 90 +++++++++---------- .../serialization/bytecode_serialization.ts | 42 ++++----- 20 files changed, 101 insertions(+), 120 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index a46f0028d63..83a22a5966f 100644 --- a/yarn-project/simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/simulator/src/avm/avm_machine_state.ts @@ -19,7 +19,7 @@ export class AvmMachineState { /** gas remaining of the gas allocated for a contract call */ public l2GasLeft: number; public daGasLeft: number; - /** program counter */ + /** program counter, byte based */ public pc: number = 0; /** return/revertdata of the last nested call. */ public nestedReturndata: Fr[] = []; @@ -91,13 +91,6 @@ export class AvmMachineState { } } - /** - * Most instructions just increment PC before they complete - */ - public incrementPc() { - this.pc++; - } - /** * Halt as successful * Output data must NOT be modified once it is set diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 582912a91e6..1fcca013a48 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -15,8 +15,7 @@ import { revertReasonFromExceptionalHalt, revertReasonFromExplicitRevert, } from './errors.js'; -import type { Instruction } from './opcodes/index.js'; -import { decodeFromBytecode } from './serialization/bytecode_serialization.js'; +import { decodeInstructionFromBytecode } from './serialization/bytecode_serialization.js'; type OpcodeTally = { count: number; @@ -70,24 +69,17 @@ export class AvmSimulator { */ public async executeBytecode(bytecode: Buffer): Promise { assert(isAvmBytecode(bytecode), "AVM simulator can't execute non-AVM bytecode"); + assert(bytecode.length > 0, "AVM simulator can't execute empty bytecode"); this.bytecode = bytecode; - return await this.executeInstructions(decodeFromBytecode(bytecode)); - } - /** - * Executes the provided instructions in the current context. - * This method is useful for testing and debugging. - */ - public async executeInstructions(instructions: Instruction[]): Promise { - assert(instructions.length > 0); const { machineState } = this.context; try { // Execute instruction pointed to by the current program counter // continuing until the machine state signifies a halt let instrCounter = 0; while (!machineState.getHalted()) { - const instruction = instructions[machineState.pc]; + const [instruction, bytesRead] = decodeInstructionFromBytecode(bytecode, machineState.pc); assert( !!instruction, 'AVM attempted to execute non-existent instruction. This should never happen (invalid bytecode or AVM simulator bug)!', @@ -105,6 +97,10 @@ export class AvmSimulator { // Normal returns and reverts will return normally here. // "Exceptional halts" will throw. await instruction.execute(this.context); + if (!instruction.handlesPC()) { + // Increment PC if the instruction doesn't handle it itself + machineState.pc += bytesRead; + } // gas used by this instruction - used for profiling/tallying const gasUsed: Gas = { @@ -113,9 +109,9 @@ export class AvmSimulator { }; this.tallyInstruction(instrPc, instruction.constructor.name, gasUsed); - if (machineState.pc >= instructions.length) { + if (machineState.pc >= bytecode.length) { this.log.warn('Passed end of program'); - throw new InvalidProgramCounterError(machineState.pc, /*max=*/ instructions.length); + throw new InvalidProgramCounterError(machineState.pc, /*max=*/ bytecode.length); } } diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts index a7701468dd1..5ba30a80aeb 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.ts @@ -43,7 +43,6 @@ export class NoteHashExists extends Instruction { memory.set(existsOffset, exists ? new Uint1(1) : new Uint1(0)); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -74,7 +73,6 @@ export class EmitNoteHash extends Instruction { context.persistableState.writeNoteHash(context.environment.address, noteHash); memory.assert({ reads: 1, addressing }); - context.machineState.incrementPc(); } } @@ -115,7 +113,6 @@ export class NullifierExists extends Instruction { memory.set(existsOffset, exists ? new Uint1(1) : new Uint1(0)); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -157,7 +154,6 @@ export class EmitNullifier extends Instruction { } memory.assert({ reads: 1, addressing }); - context.machineState.incrementPc(); } } @@ -201,7 +197,6 @@ export class L1ToL2MessageExists extends Instruction { memory.set(existsOffset, exists ? new Uint1(1) : new Uint1(0)); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -236,7 +231,6 @@ export class EmitUnencryptedLog extends Instruction { context.persistableState.writeUnencryptedLog(contractAddress, log); memory.assert({ reads: 1 + logSize, addressing }); - context.machineState.incrementPc(); } } @@ -267,6 +261,5 @@ export class SendL2ToL1Message extends Instruction { context.persistableState.writeL2ToL1Message(context.environment.address, recipient, content); memory.assert({ reads: 2, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/arithmetic.ts b/yarn-project/simulator/src/avm/opcodes/arithmetic.ts index 958933bd1d1..571c1b64ed5 100644 --- a/yarn-project/simulator/src/avm/opcodes/arithmetic.ts +++ b/yarn-project/simulator/src/avm/opcodes/arithmetic.ts @@ -21,7 +21,6 @@ export abstract class ThreeOperandArithmeticInstruction extends ThreeOperandInst memory.set(dstOffset, dest); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } protected abstract compute(a: MemoryValue, b: MemoryValue): MemoryValue; diff --git a/yarn-project/simulator/src/avm/opcodes/bitwise.ts b/yarn-project/simulator/src/avm/opcodes/bitwise.ts index 65fb14519d4..5e36c158212 100644 --- a/yarn-project/simulator/src/avm/opcodes/bitwise.ts +++ b/yarn-project/simulator/src/avm/opcodes/bitwise.ts @@ -22,7 +22,6 @@ abstract class ThreeOperandBitwiseInstruction extends ThreeOperandInstruction { memory.set(dstOffset, res); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } protected abstract compute(a: IntegralValue, b: IntegralValue): IntegralValue; @@ -110,6 +109,5 @@ export class Not extends Instruction { memory.set(dstOffset, res); memory.assert({ reads: 1, writes: 1, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/comparators.ts b/yarn-project/simulator/src/avm/opcodes/comparators.ts index f77035caa5c..7dd54f9b663 100644 --- a/yarn-project/simulator/src/avm/opcodes/comparators.ts +++ b/yarn-project/simulator/src/avm/opcodes/comparators.ts @@ -21,7 +21,6 @@ abstract class ComparatorInstruction extends ThreeOperandInstruction { memory.set(dstOffset, dest); memory.assert({ reads: 2, writes: 1, addressing }); - context.machineState.incrementPc(); } protected abstract compare(a: MemoryValue, b: MemoryValue): boolean; diff --git a/yarn-project/simulator/src/avm/opcodes/contract.ts b/yarn-project/simulator/src/avm/opcodes/contract.ts index 76f821755cb..a9912fd38fc 100644 --- a/yarn-project/simulator/src/avm/opcodes/contract.ts +++ b/yarn-project/simulator/src/avm/opcodes/contract.ts @@ -70,6 +70,5 @@ export class GetContractInstance extends Instruction { memory.set(dstOffset, memberValue); memory.assert({ reads: 1, writes: 2, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/control_flow.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.ts index a83af756468..4fe1485f69b 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.ts @@ -22,12 +22,18 @@ export class Jump extends Instruction { context.machineState.memory.assert({}); } + + public override handlesPC(): boolean { + return true; + } } export class JumpI extends Instruction { static type: string = 'JUMPI'; static readonly opcode: Opcode = Opcode.JUMPI_16; + private shouldJump: boolean = false; + // Instruction wire format with opcode. static readonly wireFormat: OperandType[] = [ OperandType.UINT8, @@ -50,13 +56,19 @@ export class JumpI extends Instruction { const condition = memory.getAs(condOffset); if (condition.toBigInt() == 0n) { - context.machineState.incrementPc(); + this.shouldJump = false; + // PC handling is done in the fetch-decode loop. } else { + this.shouldJump = true; context.machineState.pc = this.loc; } memory.assert({ reads: 1, addressing }); } + + public override handlesPC(): boolean { + return this.shouldJump === true; + } } export class InternalCall extends Instruction { @@ -77,6 +89,10 @@ export class InternalCall extends Instruction { context.machineState.memory.assert({}); } + + public override handlesPC(): boolean { + return true; + } } export class InternalReturn extends Instruction { @@ -100,4 +116,8 @@ export class InternalReturn extends Instruction { context.machineState.memory.assert({}); } + + public override handlesPC(): boolean { + return true; + } } diff --git a/yarn-project/simulator/src/avm/opcodes/conversion.ts b/yarn-project/simulator/src/avm/opcodes/conversion.ts index 38156debcc0..f7a954d5e6c 100644 --- a/yarn-project/simulator/src/avm/opcodes/conversion.ts +++ b/yarn-project/simulator/src/avm/opcodes/conversion.ts @@ -64,6 +64,5 @@ export class ToRadixBE extends Instruction { memory.setSlice(dstOffset, res); memory.assert({ reads: 2, writes: this.numLimbs, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/ec_add.ts b/yarn-project/simulator/src/avm/opcodes/ec_add.ts index 5f17b690a3c..c3d8015af9a 100644 --- a/yarn-project/simulator/src/avm/opcodes/ec_add.ts +++ b/yarn-project/simulator/src/avm/opcodes/ec_add.ts @@ -92,6 +92,5 @@ export class EcAdd extends Instruction { memory.set(dstOffset + 2, new Field(dest.equals(Point.ZERO) ? 1 : 0)); memory.assert({ reads: 6, writes: 3, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/environment_getters.ts b/yarn-project/simulator/src/avm/opcodes/environment_getters.ts index 4f9205adff7..92e9de1b8e8 100644 --- a/yarn-project/simulator/src/avm/opcodes/environment_getters.ts +++ b/yarn-project/simulator/src/avm/opcodes/environment_getters.ts @@ -83,6 +83,5 @@ export class GetEnvVar extends Instruction { memory.set(dstOffset, getValue(this.varEnum as EnvironmentVariable, context)); memory.assert({ writes: 1, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index db6b658dade..0a9d77c90f9 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -107,7 +107,6 @@ abstract class ExternalCall extends Instruction { ); memory.assert({ reads: calldataSize + 4, writes: 1, addressing }); - context.machineState.incrementPc(); } public abstract override get type(): 'CALL' | 'STATICCALL'; diff --git a/yarn-project/simulator/src/avm/opcodes/hashing.ts b/yarn-project/simulator/src/avm/opcodes/hashing.ts index 84344e1f10e..2f817814b0e 100644 --- a/yarn-project/simulator/src/avm/opcodes/hashing.ts +++ b/yarn-project/simulator/src/avm/opcodes/hashing.ts @@ -40,7 +40,6 @@ export class Poseidon2 extends Instruction { ); memory.assert({ reads: Poseidon2.stateSize, writes: Poseidon2.stateSize, addressing }); - context.machineState.incrementPc(); } } @@ -78,7 +77,6 @@ export class KeccakF1600 extends Instruction { memory.setSlice(dstOffset, res); memory.assert({ reads: inputSize, writes: inputSize, addressing }); - context.machineState.incrementPc(); } } @@ -127,6 +125,5 @@ export class Sha256Compression extends Instruction { memory.setSlice(outputOffset, res); memory.assert({ reads: STATE_SIZE + INPUTS_SIZE, writes: STATE_SIZE, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/instruction.ts b/yarn-project/simulator/src/avm/opcodes/instruction.ts index 4896ae206a9..c7ae85a5d84 100644 --- a/yarn-project/simulator/src/avm/opcodes/instruction.ts +++ b/yarn-project/simulator/src/avm/opcodes/instruction.ts @@ -22,6 +22,13 @@ export abstract class Instruction { */ public abstract execute(context: AvmContext): Promise; + /** + * Whether the instruction will modify the PC itself. + */ + public handlesPC(): boolean { + return false; + } + /** * Generate a string representation of the instruction including * the instruction sub-class name all of its flags and operands. diff --git a/yarn-project/simulator/src/avm/opcodes/memory.ts b/yarn-project/simulator/src/avm/opcodes/memory.ts index cde6056f688..89706285d1d 100644 --- a/yarn-project/simulator/src/avm/opcodes/memory.ts +++ b/yarn-project/simulator/src/avm/opcodes/memory.ts @@ -72,7 +72,6 @@ export class Set extends Instruction { memory.set(dstOffset, res); memory.assert({ writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -113,7 +112,6 @@ export class Cast extends Instruction { memory.set(dstOffset, casted); memory.assert({ reads: 1, writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -152,7 +150,6 @@ export class Mov extends Instruction { memory.set(dstOffset, a); memory.assert({ reads: 1, writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -193,7 +190,6 @@ export class CalldataCopy extends Instruction { memory.setSlice(dstOffset, transformedData); memory.assert({ reads: 2, writes: copySize, addressing }); - context.machineState.incrementPc(); } } @@ -217,7 +213,6 @@ export class ReturndataSize extends Instruction { memory.set(dstOffset, new Uint32(context.machineState.nestedReturndata.length)); memory.assert({ writes: 1, addressing }); - context.machineState.incrementPc(); } } @@ -260,6 +255,5 @@ export class ReturndataCopy extends Instruction { memory.setSlice(dstOffset, transformedData); memory.assert({ reads: 2, writes: copySize, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/misc.ts b/yarn-project/simulator/src/avm/opcodes/misc.ts index 11bfe55234d..88f1f4a9581 100644 --- a/yarn-project/simulator/src/avm/opcodes/misc.ts +++ b/yarn-project/simulator/src/avm/opcodes/misc.ts @@ -56,6 +56,5 @@ export class DebugLog extends Instruction { DebugLog.logger.verbose(formattedStr); memory.assert({ reads: 1 + fieldsSize + this.messageSize, addressing }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts b/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts index 26b3686818a..d0f536e8c3e 100644 --- a/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts +++ b/yarn-project/simulator/src/avm/opcodes/multi_scalar_mul.ts @@ -114,6 +114,5 @@ export class MultiScalarMul extends Instruction { writes: 3 /* output triplet */, addressing, }); - context.machineState.incrementPc(); } } diff --git a/yarn-project/simulator/src/avm/opcodes/storage.ts b/yarn-project/simulator/src/avm/opcodes/storage.ts index 77d161030e7..f01c573c93c 100644 --- a/yarn-project/simulator/src/avm/opcodes/storage.ts +++ b/yarn-project/simulator/src/avm/opcodes/storage.ts @@ -46,7 +46,6 @@ export class SStore extends BaseStorageInstruction { context.persistableState.writeStorage(context.environment.address, slot, value); memory.assert({ reads: 2, addressing }); - context.machineState.incrementPc(); } } @@ -71,7 +70,6 @@ export class SLoad extends BaseStorageInstruction { const value = await context.persistableState.readStorage(context.environment.address, slot); memory.set(dstOffset, new Field(value)); - context.machineState.incrementPc(); memory.assert({ writes: 1, reads: 1, addressing }); } } diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts index 7662b5300c6..1bcafba5ae4 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'assert'; import { Add, Call, EnvironmentVariable, GetEnvVar, StaticCall, Sub } from '../opcodes/index.js'; import { type BufferCursor } from './buffer_cursor.js'; -import { type InstructionSet, decodeFromBytecode, encodeToBytecode } from './bytecode_serialization.js'; +import { type InstructionSet, encodeToBytecode } from './bytecode_serialization.js'; import { Opcode } from './instruction_serialization.js'; class InstA { @@ -46,19 +46,19 @@ class InstB { } describe('Bytecode Serialization', () => { - it('Should deserialize using instruction set', () => { - const instructionSet: InstructionSet = new Map([ - [InstA.opcode, InstA.deserialize], - [InstB.opcode, InstB.deserialize], - ]); - const a = new InstA(0x1234); - const b = new InstB(0x5678n); - const bytecode = Buffer.concat([a.serialize(), b.serialize()]); - - const actual = decodeFromBytecode(bytecode, instructionSet); - - expect(actual).toEqual([a, b]); - }); + // it('Should deserialize using instruction set', () => { + // const instructionSet: InstructionSet = new Map([ + // [InstA.opcode, InstA.deserialize], + // [InstB.opcode, InstB.deserialize], + // ]); + // const a = new InstA(0x1234); + // const b = new InstB(0x5678n); + // const bytecode = Buffer.concat([a.serialize(), b.serialize()]); + + // const actual = decodeFromBytecode(bytecode, instructionSet); + + // expect(actual).toEqual([a, b]); + // }); it('Should serialize using instruction.serialize()', () => { const a = new InstA(1234); @@ -70,37 +70,37 @@ describe('Bytecode Serialization', () => { expect(actual).toEqual(expected); }); - it('Should deserialize real instructions', () => { - const instructions = [ - new Add(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.ADD_8, Add.wireFormat8), - new Sub(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.SUB_8, Sub.wireFormat8), - new GetEnvVar(/*indirect=*/ 0, EnvironmentVariable.ADDRESS, /*dstOffset=*/ 1).as( - Opcode.GETENVVAR_16, - GetEnvVar.wireFormat16, - ), - new Call( - /*indirect=*/ 0x01, - /*gasOffset=*/ 0x1234, - /*addrOffset=*/ 0xa234, - /*argsOffset=*/ 0xb234, - /*argsSize=*/ 0xc234, - /*successOffset=*/ 0xf234, - ), - new StaticCall( - /*indirect=*/ 0x01, - /*gasOffset=*/ 0x1234, - /*addrOffset=*/ 0xa234, - /*argsOffset=*/ 0xb234, - /*argsSize=*/ 0xc234, - /*successOffset=*/ 0xf234, - ), - ]; - const bytecode = Buffer.concat(instructions.map(i => i.serialize())); - - const actual = decodeFromBytecode(bytecode); - - expect(actual).toEqual(instructions); - }); + // it('Should deserialize real instructions', () => { + // const instructions = [ + // new Add(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.ADD_8, Add.wireFormat8), + // new Sub(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.SUB_8, Sub.wireFormat8), + // new GetEnvVar(/*indirect=*/ 0, EnvironmentVariable.ADDRESS, /*dstOffset=*/ 1).as( + // Opcode.GETENVVAR_16, + // GetEnvVar.wireFormat16, + // ), + // new Call( + // /*indirect=*/ 0x01, + // /*gasOffset=*/ 0x1234, + // /*addrOffset=*/ 0xa234, + // /*argsOffset=*/ 0xb234, + // /*argsSize=*/ 0xc234, + // /*successOffset=*/ 0xf234, + // ), + // new StaticCall( + // /*indirect=*/ 0x01, + // /*gasOffset=*/ 0x1234, + // /*addrOffset=*/ 0xa234, + // /*argsOffset=*/ 0xb234, + // /*argsSize=*/ 0xc234, + // /*successOffset=*/ 0xf234, + // ), + // ]; + // const bytecode = Buffer.concat(instructions.map(i => i.serialize())); + + // const actual = decodeFromBytecode(bytecode); + + // expect(actual).toEqual(instructions); + // }); it('Should serialize real instructions', () => { const instructions = [ diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 88f9ef16de5..9be75ce0515 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts @@ -143,12 +143,11 @@ const INSTRUCTION_SET = () => [EcAdd.opcode, Instruction.deserialize.bind(EcAdd)], [Poseidon2.opcode, Instruction.deserialize.bind(Poseidon2)], [Sha256Compression.opcode, Instruction.deserialize.bind(Sha256Compression)], + [KeccakF1600.opcode, Instruction.deserialize.bind(KeccakF1600)], [MultiScalarMul.opcode, Instruction.deserialize.bind(MultiScalarMul)], + // Conversions [ToRadixBE.opcode, Instruction.deserialize.bind(ToRadixBE)], - // Future Gadgets -- pending changes in noir - // SHA256COMPRESSION, - [KeccakF1600.opcode, Instruction.deserialize.bind(KeccakF1600)], ]); /** @@ -158,30 +157,25 @@ export function encodeToBytecode(instructions: Serializable[]): Buffer { return Buffer.concat(instructions.map(i => i.serialize())); } -/** - * Convert a buffer of bytecode into an array of instructions. - * @param bytecode Buffer of bytecode. - * @param instructionSet Optional {@code InstructionSet} to be used for deserialization. - * @returns Bytecode decoded into an ordered array of Instructions - */ -export function decodeFromBytecode( +// Returns the instruction and the number of bytes consumed. +export function decodeInstructionFromBytecode( bytecode: Buffer, + pc: number, instructionSet: InstructionSet = INSTRUCTION_SET(), -): Instruction[] { - const instructions: Instruction[] = []; - const cursor = new BufferCursor(bytecode); - - while (!cursor.eof()) { - const opcode: Opcode = cursor.bufferAtPosition().readUint8(); // peek. - const instructionDeserializerOrUndef = instructionSet.get(opcode); - if (instructionDeserializerOrUndef === undefined) { - throw new Error(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); - } +): [Instruction, number] { + if (pc >= bytecode.length) { + throw new Error(`pc ${pc} is out of bounds for bytecode of length ${bytecode.length}`); + } + const cursor = new BufferCursor(bytecode, pc); + const startingPosition = cursor.position(); + const opcode: Opcode = cursor.bufferAtPosition().readUint8(); // peek. - const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; - const i: Instruction = instructionDeserializer(cursor); - instructions.push(i); + const instructionDeserializerOrUndef = instructionSet.get(opcode); + if (instructionDeserializerOrUndef === undefined) { + throw new Error(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); } - return instructions; + const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; + const instruction = instructionDeserializer(cursor); + return [instruction, cursor.position() - startingPosition]; } From 3f98b0dc28c0a0a1e177f987b258f08adcecc9b1 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 30 Oct 2024 13:14:03 +0000 Subject: [PATCH 02/13] transpiler + fixes --- avm-transpiler/src/instructions.rs | 11 +++ avm-transpiler/src/transpile.rs | 87 ++++++++++--------- avm-transpiler/src/transpile_contract.rs | 18 ++-- avm-transpiler/src/utils.rs | 4 +- .../src/protocol_contract_data.ts | 8 +- .../simulator/src/avm/avm_machine_state.ts | 2 + .../simulator/src/avm/avm_simulator.test.ts | 21 ++--- .../simulator/src/avm/avm_simulator.ts | 1 + .../src/avm/opcodes/control_flow.test.ts | 6 +- .../simulator/src/avm/opcodes/control_flow.ts | 10 +-- .../src/avm/opcodes/external_calls.ts | 6 ++ 11 files changed, 97 insertions(+), 77 deletions(-) diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs index 74318b0b5ac..5cb5ad37a38 100644 --- a/avm-transpiler/src/instructions.rs +++ b/avm-transpiler/src/instructions.rs @@ -64,6 +64,10 @@ impl AvmInstruction { } bytes } + + pub fn size(&self) -> usize { + self.to_bytes().len() + } } impl Debug for AvmInstruction { @@ -101,6 +105,7 @@ pub enum AvmTypeTag { /// Operands are usually 32 bits (offsets or jump destinations) /// Constants (as used by the SET instruction) can have size /// different from 32 bits +#[allow(non_camel_case_types)] pub enum AvmOperand { U8 { value: u8 }, U16 { value: u16 }, @@ -108,6 +113,8 @@ pub enum AvmOperand { U64 { value: u64 }, U128 { value: u128 }, FF { value: FieldElement }, + // Unresolved brillig pc that needs translation to a 16 bit byte-indexed PC. + BRILLIG_LOCATION { brillig_pc: u16 }, } impl Display for AvmOperand { @@ -119,6 +126,9 @@ impl Display for AvmOperand { AvmOperand::U64 { value } => write!(f, " U64:{}", value), AvmOperand::U128 { value } => write!(f, " U128:{}", value), AvmOperand::FF { value } => write!(f, " FF:{}", value), + AvmOperand::BRILLIG_LOCATION { brillig_pc } => { + write!(f, " BRILLIG_LOCATION:{}", brillig_pc) + } } } } @@ -132,6 +142,7 @@ impl AvmOperand { AvmOperand::U64 { value } => value.to_be_bytes().to_vec(), AvmOperand::U128 { value } => value.to_be_bytes().to_vec(), AvmOperand::FF { value } => value.to_be_bytes(), + AvmOperand::BRILLIG_LOCATION { brillig_pc } => brillig_pc.to_be_bytes().to_vec(), } } } diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 799d0017b6c..35eb4fb5416 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -9,22 +9,24 @@ use acvm::brillig_vm::brillig::{ use acvm::FieldElement; use noirc_errors::debug_info::DebugInfo; -use crate::bit_traits::bits_needed_for; +use crate::bit_traits::{bits_needed_for, BitsQueryable}; use crate::instructions::{AddressingModeBuilder, AvmInstruction, AvmOperand, AvmTypeTag}; use crate::opcodes::AvmOpcode; use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program, make_operand}; /// Transpile a Brillig program to AVM bytecode -pub fn brillig_to_avm( - brillig_bytecode: &[BrilligOpcode], - brillig_pcs_to_avm_pcs: &[usize], -) -> Vec { +/// Returns the bytecode and a mapping from Brillig program counter to AVM program counter. +pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec, Vec) { dbg_print_brillig_program(brillig_bytecode); let mut avm_instrs: Vec = Vec::new(); + let mut brillig_pcs_to_avm_pcs: Vec = [0_usize].to_vec(); + let mut current_avm_pc: usize = 0; // Transpile a Brillig instruction to one or more AVM instructions for brillig_instr in brillig_bytecode { + let current_avm_instr_index = avm_instrs.len(); + match brillig_instr { BrilligOpcode::BinaryFieldOp { destination, op, lhs, rhs } => { let bits_needed = @@ -231,22 +233,23 @@ pub fn brillig_to_avm( }); } BrilligOpcode::Jump { location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 16); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMP_16, - operands: vec![make_operand(16, &avm_loc)], + // operands: vec![make_operand(16, &avm_loc)], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], ..Default::default() }); } BrilligOpcode::JumpIf { condition, location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 16); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMPI_16, indirect: Some( AddressingModeBuilder::default().direct_operand(condition).build(), ), operands: vec![ - make_operand(16, &avm_loc), + AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }, make_operand(16, &condition.to_usize()), ], ..Default::default() @@ -295,10 +298,10 @@ pub fn brillig_to_avm( )); } BrilligOpcode::Call { location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 16); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::INTERNALCALL, - operands: vec![AvmOperand::U16 { value: avm_loc as u16 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], ..Default::default() }); } @@ -342,8 +345,37 @@ pub fn brillig_to_avm( brillig_instr ), } + + // Increment the AVM program counter. + current_avm_pc += + avm_instrs.iter().skip(current_avm_instr_index).map(|i| i.size()).sum::(); + brillig_pcs_to_avm_pcs.push(current_avm_pc); } + // Now that we have the general structure of the AVM program, we need to resolve the + // Brillig jump locations. + let mut avm_instrs = avm_instrs + .into_iter() + .map(|i| match i.opcode { + AvmOpcode::JUMP_16 | AvmOpcode::JUMPI_16 | AvmOpcode::INTERNALCALL => { + let new_operands = i + .operands + .into_iter() + .map(|o| match o { + AvmOperand::BRILLIG_LOCATION { brillig_pc } => { + let avm_pc = brillig_pcs_to_avm_pcs[brillig_pc as usize]; + assert!(avm_pc.num_bits() <= 16, "Oops! AVM PC is too large!"); + AvmOperand::U16 { value: avm_pc as u16 } + } + _ => o, + }) + .collect::>(); + AvmInstruction { operands: new_operands, ..i } + } + _ => i, + }) + .collect::>(); + // TEMPORARY: Add a "magic number" instruction to the end of the program. // This makes it possible to know that the bytecode corresponds to the AVM. // We are adding a MOV instruction that moves a value to itself. @@ -362,7 +394,8 @@ pub fn brillig_to_avm( for instruction in avm_instrs { bytecode.extend_from_slice(&instruction.to_bytes()); } - bytecode + + (bytecode, brillig_pcs_to_avm_pcs) } /// Handle brillig foreign calls @@ -1508,36 +1541,6 @@ pub fn patch_assert_message_pcs( .collect() } -/// Compute an array that maps each Brillig pc to an AVM pc. -/// This must be done before transpiling to properly transpile jump destinations. -/// This is necessary for two reasons: -/// 1. The transpiler injects `initial_offset` instructions at the beginning of the program. -/// 2. Some brillig instructions map to multiple AVM instructions -/// args: -/// initial_offset: how many AVM instructions were inserted at the start of the program -/// brillig: the Brillig program -/// returns: an array where each index is a Brillig pc, -/// and each value is the corresponding AVM pc. -pub fn map_brillig_pcs_to_avm_pcs(brillig_bytecode: &[BrilligOpcode]) -> Vec { - let mut pc_map = vec![0; brillig_bytecode.len()]; - - pc_map[0] = 0; // first PC is always 0 as there are no instructions inserted by AVM at start - for i in 0..brillig_bytecode.len() - 1 { - let num_avm_instrs_for_this_brillig_instr = match &brillig_bytecode[i] { - BrilligOpcode::ForeignCall { function, .. } - if function == "avmOpcodeReturndataCopy" => - { - 2 - } - _ => 1, - }; - // next Brillig pc will map to an AVM pc offset by the - // number of AVM instructions generated for this Brillig one - pc_map[i + 1] = pc_map[i] + num_avm_instrs_for_this_brillig_instr; - } - pc_map -} - fn tag_from_bit_size(bit_size: BitSize) -> AvmTypeTag { match bit_size { BitSize::Integer(IntegerBitSize::U1) => AvmTypeTag::UINT1, diff --git a/avm-transpiler/src/transpile_contract.rs b/avm-transpiler/src/transpile_contract.rs index 10b9367a4ab..ecb9ca4738f 100644 --- a/avm-transpiler/src/transpile_contract.rs +++ b/avm-transpiler/src/transpile_contract.rs @@ -6,9 +6,7 @@ use serde::{Deserialize, Serialize}; use acvm::acir::circuit::Program; use noirc_errors::debug_info::ProgramDebugInfo; -use crate::transpile::{ - brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_assert_message_pcs, patch_debug_info_pcs, -}; +use crate::transpile::{brillig_to_avm, patch_assert_message_pcs, patch_debug_info_pcs}; use crate::utils::{extract_brillig_from_acir_program, extract_static_assert_messages}; use fxhash::FxHashMap as HashMap; @@ -92,6 +90,7 @@ impl From for TranspiledContractArtifact { for function in contract.functions { if function.custom_attributes.contains(&"public".to_string()) { + // if function.name == "public_dispatch" { info!("Transpiling AVM function {} on contract {}", function.name, contract.name); // Extract Brillig Opcodes from acir let acir_program = function.bytecode; @@ -99,15 +98,8 @@ impl From for TranspiledContractArtifact { let assert_messages = extract_static_assert_messages(&acir_program); info!("Extracted Brillig program has {} instructions", brillig_bytecode.len()); - // Map Brillig pcs to AVM pcs (index is Brillig PC, value is AVM PC) - let brillig_pcs_to_avm_pcs = map_brillig_pcs_to_avm_pcs(brillig_bytecode); - - // Patch the assert messages with updated PCs - let assert_messages = - patch_assert_message_pcs(assert_messages, &brillig_pcs_to_avm_pcs); - // Transpile to AVM - let avm_bytecode = brillig_to_avm(brillig_bytecode, &brillig_pcs_to_avm_pcs); + let (avm_bytecode, brillig_pcs_to_avm_pcs) = brillig_to_avm(brillig_bytecode); log::info!( "{}::{}: bytecode is {} bytes", @@ -116,6 +108,10 @@ impl From for TranspiledContractArtifact { avm_bytecode.len(), ); + // Patch the assert messages with updated PCs + let assert_messages = + patch_assert_message_pcs(assert_messages, &brillig_pcs_to_avm_pcs); + // Patch the debug infos with updated PCs let debug_infos = patch_debug_info_pcs( function.debug_symbols.debug_infos, diff --git a/avm-transpiler/src/utils.rs b/avm-transpiler/src/utils.rs index 3f5269f5daa..4a92a895a60 100644 --- a/avm-transpiler/src/utils.rs +++ b/avm-transpiler/src/utils.rs @@ -79,9 +79,11 @@ pub fn dbg_print_avm_program(avm_program: &[AvmInstruction]) { info!("Transpiled AVM program has {} instructions", avm_program.len()); trace!("Printing AVM program..."); let mut counts = std::collections::HashMap::::new(); + let mut avm_pc = 0; for (i, instruction) in avm_program.iter().enumerate() { - trace!("\tPC:{0}: {1}", i, &instruction.to_string()); + trace!("\tIDX:{0} AVMPC:{1} - {2}", i, avm_pc, &instruction.to_string()); *counts.entry(instruction.opcode).or_insert(0) += 1; + avm_pc += instruction.size(); } debug!("AVM opcode counts:"); let mut sorted_counts: Vec<_> = counts.into_iter().collect(); diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index 699341e49a1..3e47bbd78ba 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x13794ed6c957a68bc852fe4c2a161019a53011b08331d8eb0287483a7845d334'), + AuthRegistry: Fr.fromString('0x07f087b50e979ca68413e45642ec987ce9793daf533bb2cd972af7366efe00cd'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x146cb9b7cda808b4d5e066773e2fe0131d4ac4f7238bc0f093dce70f7c0f3421'), - Router: Fr.fromString('0x19e9ec99aedfe3ea69ba91b862b815df7d1796fa802985a154159cd739fe4817'), + FeeJuice: Fr.fromString('0x0f81bdfc641bb478475cbcb6d10c05b19f4916f78c07c3132c05136f31878a36'), + Router: Fr.fromString('0x06c0bf942c1e8c25f57ca29b935af303ba54278a72e803b01c0f42256b74f01e'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x149eb7ca5c1f23580f2449edfbb65ec70160e5d4b6f8313f061b8a8d73d014ab', + '0x130b2286d22d028aaff576b4d446626bac34664e0f345f9f26e7531a82b8b5ab', ); diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index 83a22a5966f..202e4ff23c3 100644 --- a/yarn-project/simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/simulator/src/avm/avm_machine_state.ts @@ -21,6 +21,8 @@ export class AvmMachineState { public daGasLeft: number; /** program counter, byte based */ public pc: number = 0; + /** program counter of the next instruction, byte based */ + public nextPc: number = 0; /** return/revertdata of the last nested call. */ public nestedReturndata: Fr[] = []; diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index eb4beb17b6e..9bd70541067 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -417,19 +417,20 @@ describe('AVM simulator: transpiled Noir contracts', () => { }); }); - it('conversions', async () => { - const calldata: Fr[] = [new Fr(0b1011101010100)]; - const context = initContext({ env: initExecutionEnvironment({ calldata }) }); + // it('conversions', async () => { + // const calldata: Fr[] = [new Fr(0b1011101010100)]; + // const context = initContext({ env: initExecutionEnvironment({ calldata }) }); - const bytecode = getAvmTestContractBytecode('to_radix_le'); - const results = await new AvmSimulator(context).executeBytecode(bytecode); + // const bytecode = getAvmTestContractBytecode('to_radix_le'); + // const results = await new AvmSimulator(context).executeBytecode(bytecode); - expect(results.reverted).toBe(false); - const expectedResults = Buffer.concat('0010101011'.split('').map(c => new Fr(Number(c)).toBuffer())); - const resultBuffer = Buffer.concat(results.output.map(f => f.toBuffer())); + // expect(results.reverted).toBe(false); + // const expectedResults = Buffer.concat('0010101011'.split('').map(c => new Fr(Number(c)).toBuffer())); + // const resultBuffer = Buffer.concat(results.output.map(f => f.toBuffer())); - expect(resultBuffer.equals(expectedResults)).toBe(true); - }); + // expect(results.output.map(f => f.toNumber().toString()).join('')).toEqual('0010101011'); + // // expect(resultBuffer).toEqual(expectedResults); + // }); describe('Side effects, world state, nested calls', () => { const address = new Fr(1); diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 1fcca013a48..56c322c1162 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -96,6 +96,7 @@ export class AvmSimulator { // Execute the instruction. // Normal returns and reverts will return normally here. // "Exceptional halts" will throw. + machineState.nextPc = machineState.pc + bytesRead; await instruction.execute(this.context); if (!instruction.handlesPC()) { // Increment PC if the instruction doesn't handle it itself diff --git a/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts index 0dac3c087a8..db53c76deaf 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts @@ -70,13 +70,14 @@ describe('Control Flow Opcodes', () => { it('Should implement JUMPI - falsy', async () => { const jumpLocation = 22; + context.machineState.nextPc = 30; expect(context.machineState.pc).toBe(0); context.machineState.memory.set(0, new Uint16(0n)); const instruction = new JumpI(/*indirect=*/ 0, jumpLocation, /*condOffset=*/ 0); await instruction.execute(context); - expect(context.machineState.pc).toBe(1); + expect(context.machineState.pc).toBe(30); }); }); @@ -96,6 +97,7 @@ describe('Control Flow Opcodes', () => { const jumpLocation = 22; expect(context.machineState.pc).toBe(0); + context.machineState.nextPc = 6; const instruction = new InternalCall(jumpLocation); const returnInstruction = new InternalReturn(); @@ -104,7 +106,7 @@ describe('Control Flow Opcodes', () => { expect(context.machineState.pc).toBe(jumpLocation); await returnInstruction.execute(context); - expect(context.machineState.pc).toBe(1); + expect(context.machineState.pc).toBe(6); }); it('Should error if Internal Return is called without a corresponding Internal Call', async () => { diff --git a/yarn-project/simulator/src/avm/opcodes/control_flow.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.ts index 4fe1485f69b..8fe48736e38 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.ts @@ -32,8 +32,6 @@ export class JumpI extends Instruction { static type: string = 'JUMPI'; static readonly opcode: Opcode = Opcode.JUMPI_16; - private shouldJump: boolean = false; - // Instruction wire format with opcode. static readonly wireFormat: OperandType[] = [ OperandType.UINT8, @@ -56,10 +54,8 @@ export class JumpI extends Instruction { const condition = memory.getAs(condOffset); if (condition.toBigInt() == 0n) { - this.shouldJump = false; - // PC handling is done in the fetch-decode loop. + context.machineState.pc = context.machineState.nextPc; } else { - this.shouldJump = true; context.machineState.pc = this.loc; } @@ -67,7 +63,7 @@ export class JumpI extends Instruction { } public override handlesPC(): boolean { - return this.shouldJump === true; + return true; } } @@ -84,7 +80,7 @@ export class InternalCall extends Instruction { public async execute(context: AvmContext): Promise { context.machineState.consumeGas(this.gasCost()); - context.machineState.internalCallStack.push(context.machineState.pc + 1); + context.machineState.internalCallStack.push(context.machineState.nextPc); context.machineState.pc = this.loc; context.machineState.memory.assert({}); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index 0a9d77c90f9..2ecc1e3f979 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -196,6 +196,12 @@ export class Revert extends Instruction { context.machineState.revert(output); memory.assert({ reads: retSize + 1, addressing }); } + + // We don't want to increase the PC after reverting because it breaks messages. + // Maybe we can remove this once messages don't depend on PCs. + public override handlesPC(): boolean { + return true; + } } /** Returns the smaller of two bigints. */ From 2b434f477725bb2fe94b3830bb9113d403eff29b Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 30 Oct 2024 13:47:28 +0000 Subject: [PATCH 03/13] fixes --- .../simulator/src/avm/avm_simulator.test.ts | 23 ++++++++----------- .../src/avm/opcodes/external_calls.ts | 4 ++++ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 9bd70541067..357b2bf4ab7 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -417,20 +417,16 @@ describe('AVM simulator: transpiled Noir contracts', () => { }); }); - // it('conversions', async () => { - // const calldata: Fr[] = [new Fr(0b1011101010100)]; - // const context = initContext({ env: initExecutionEnvironment({ calldata }) }); - - // const bytecode = getAvmTestContractBytecode('to_radix_le'); - // const results = await new AvmSimulator(context).executeBytecode(bytecode); + it('conversions', async () => { + const calldata: Fr[] = [new Fr(0b1011101010100)]; + const context = initContext({ env: initExecutionEnvironment({ calldata }) }); - // expect(results.reverted).toBe(false); - // const expectedResults = Buffer.concat('0010101011'.split('').map(c => new Fr(Number(c)).toBuffer())); - // const resultBuffer = Buffer.concat(results.output.map(f => f.toBuffer())); + const bytecode = getAvmTestContractBytecode('to_radix_le'); + const results = await new AvmSimulator(context).executeBytecode(bytecode); - // expect(results.output.map(f => f.toNumber().toString()).join('')).toEqual('0010101011'); - // // expect(resultBuffer).toEqual(expectedResults); - // }); + expect(results.reverted).toBe(false); + expect(results.output.map(f => f.toNumber().toString()).join('')).toEqual('0010101011'); + }); describe('Side effects, world state, nested calls', () => { const address = new Fr(1); @@ -1085,7 +1081,8 @@ describe('AVM simulator: transpiled Noir contracts', () => { // infinitely loop back to the tested instruction // infinite loop should break on side effect overrun error, // but otherwise will run out of gas - new Jump(/*jumpOffset*/ 2), + // Note: 15 is the byte index, calucalted as 3*size(Set.wireFormat8) + new Jump(/*jumpOffset*/ 15), ]); const context = initContext({ persistableState }); const results = await new AvmSimulator(context).executeBytecode(markBytecodeAsAvm(bytecode)); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index 2ecc1e3f979..296c2cda9e6 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -158,6 +158,10 @@ export class Return extends Instruction { context.machineState.return(output); memory.assert({ reads: this.copySize, addressing }); } + + public override handlesPC(): boolean { + return true; + } } export class Revert extends Instruction { From c98ed60a2a9c180c7086693abd7f5e4f46880a94 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 30 Oct 2024 13:52:12 +0000 Subject: [PATCH 04/13] more fixes --- .../src/avm/opcodes/control_flow.test.ts | 37 -------- .../bytecode_serialization.test.ts | 90 +++++++++---------- .../serialization/bytecode_serialization.ts | 15 ++++ 3 files changed, 60 insertions(+), 82 deletions(-) diff --git a/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts index db53c76deaf..a0a5ecf6139 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts @@ -114,41 +114,4 @@ describe('Control Flow Opcodes', () => { await expect(returnInstruction()).rejects.toThrow(InstructionExecutionError); }); }); - - describe('General flow', () => { - 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(context); - expect(context.machineState.pc).toBe(expectedPcs[i]); - } - }); - }); }); diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts index 1bcafba5ae4..7662b5300c6 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'assert'; import { Add, Call, EnvironmentVariable, GetEnvVar, StaticCall, Sub } from '../opcodes/index.js'; import { type BufferCursor } from './buffer_cursor.js'; -import { type InstructionSet, encodeToBytecode } from './bytecode_serialization.js'; +import { type InstructionSet, decodeFromBytecode, encodeToBytecode } from './bytecode_serialization.js'; import { Opcode } from './instruction_serialization.js'; class InstA { @@ -46,19 +46,19 @@ class InstB { } describe('Bytecode Serialization', () => { - // it('Should deserialize using instruction set', () => { - // const instructionSet: InstructionSet = new Map([ - // [InstA.opcode, InstA.deserialize], - // [InstB.opcode, InstB.deserialize], - // ]); - // const a = new InstA(0x1234); - // const b = new InstB(0x5678n); - // const bytecode = Buffer.concat([a.serialize(), b.serialize()]); - - // const actual = decodeFromBytecode(bytecode, instructionSet); - - // expect(actual).toEqual([a, b]); - // }); + it('Should deserialize using instruction set', () => { + const instructionSet: InstructionSet = new Map([ + [InstA.opcode, InstA.deserialize], + [InstB.opcode, InstB.deserialize], + ]); + const a = new InstA(0x1234); + const b = new InstB(0x5678n); + const bytecode = Buffer.concat([a.serialize(), b.serialize()]); + + const actual = decodeFromBytecode(bytecode, instructionSet); + + expect(actual).toEqual([a, b]); + }); it('Should serialize using instruction.serialize()', () => { const a = new InstA(1234); @@ -70,37 +70,37 @@ describe('Bytecode Serialization', () => { expect(actual).toEqual(expected); }); - // it('Should deserialize real instructions', () => { - // const instructions = [ - // new Add(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.ADD_8, Add.wireFormat8), - // new Sub(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.SUB_8, Sub.wireFormat8), - // new GetEnvVar(/*indirect=*/ 0, EnvironmentVariable.ADDRESS, /*dstOffset=*/ 1).as( - // Opcode.GETENVVAR_16, - // GetEnvVar.wireFormat16, - // ), - // new Call( - // /*indirect=*/ 0x01, - // /*gasOffset=*/ 0x1234, - // /*addrOffset=*/ 0xa234, - // /*argsOffset=*/ 0xb234, - // /*argsSize=*/ 0xc234, - // /*successOffset=*/ 0xf234, - // ), - // new StaticCall( - // /*indirect=*/ 0x01, - // /*gasOffset=*/ 0x1234, - // /*addrOffset=*/ 0xa234, - // /*argsOffset=*/ 0xb234, - // /*argsSize=*/ 0xc234, - // /*successOffset=*/ 0xf234, - // ), - // ]; - // const bytecode = Buffer.concat(instructions.map(i => i.serialize())); - - // const actual = decodeFromBytecode(bytecode); - - // expect(actual).toEqual(instructions); - // }); + it('Should deserialize real instructions', () => { + const instructions = [ + new Add(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.ADD_8, Add.wireFormat8), + new Sub(/*indirect=*/ 0, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 2).as(Opcode.SUB_8, Sub.wireFormat8), + new GetEnvVar(/*indirect=*/ 0, EnvironmentVariable.ADDRESS, /*dstOffset=*/ 1).as( + Opcode.GETENVVAR_16, + GetEnvVar.wireFormat16, + ), + new Call( + /*indirect=*/ 0x01, + /*gasOffset=*/ 0x1234, + /*addrOffset=*/ 0xa234, + /*argsOffset=*/ 0xb234, + /*argsSize=*/ 0xc234, + /*successOffset=*/ 0xf234, + ), + new StaticCall( + /*indirect=*/ 0x01, + /*gasOffset=*/ 0x1234, + /*addrOffset=*/ 0xa234, + /*argsOffset=*/ 0xb234, + /*argsSize=*/ 0xc234, + /*successOffset=*/ 0xf234, + ), + ]; + const bytecode = Buffer.concat(instructions.map(i => i.serialize())); + + const actual = decodeFromBytecode(bytecode); + + expect(actual).toEqual(instructions); + }); it('Should serialize real instructions', () => { const instructions = [ diff --git a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 9be75ce0515..cebf4a73cfe 100644 --- a/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts +++ b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts @@ -157,6 +157,21 @@ export function encodeToBytecode(instructions: Serializable[]): Buffer { return Buffer.concat(instructions.map(i => i.serialize())); } +// For testing only +export function decodeFromBytecode( + bytecode: Buffer, + instructionSet: InstructionSet = INSTRUCTION_SET(), +): Instruction[] { + const instructions: Instruction[] = []; + let pc = 0; + while (pc < bytecode.length) { + const [instruction, bytesConsumed] = decodeInstructionFromBytecode(bytecode, pc, instructionSet); + instructions.push(instruction); + pc += bytesConsumed; + } + return instructions; +} + // Returns the instruction and the number of bytes consumed. export function decodeInstructionFromBytecode( bytecode: Buffer, From 6dfecb8ebd4e8bfd8937ce1b7021aef300ba1c1a Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 31 Oct 2024 08:58:17 +0000 Subject: [PATCH 05/13] Some fixes after merging with 32-bit pc addressing --- avm-transpiler/src/transpile.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index dc162262de3..487e4652735 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -236,8 +236,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< assert!(location.num_bits() <= 16); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMP_32, - // operands: vec![make_operand(32, &avm_loc)], - operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], ..Default::default() }); } @@ -249,7 +248,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< AddressingModeBuilder::default().direct_operand(condition).build(), ), operands: vec![ - AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }, + AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }, make_operand(16, &condition.to_usize()), ], ..Default::default() @@ -301,7 +300,7 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< assert!(location.num_bits() <= 16); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::INTERNALCALL, - operands: vec![AvmOperand::U16 { value: avm_loc as u32 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], ..Default::default() }); } @@ -357,15 +356,15 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< let mut avm_instrs = avm_instrs .into_iter() .map(|i| match i.opcode { - AvmOpcode::JUMP_16 | AvmOpcode::JUMPI_16 | AvmOpcode::INTERNALCALL => { + AvmOpcode::JUMP_32 | AvmOpcode::JUMPI_32 | AvmOpcode::INTERNALCALL => { let new_operands = i .operands .into_iter() .map(|o| match o { AvmOperand::BRILLIG_LOCATION { brillig_pc } => { let avm_pc = brillig_pcs_to_avm_pcs[brillig_pc as usize]; - assert!(avm_pc.num_bits() <= 16, "Oops! AVM PC is too large!"); - AvmOperand::U16 { value: avm_pc as u16 } + assert!(avm_pc.num_bits() <= 32, "Oops! AVM PC is too large!"); + AvmOperand::U32 { value: avm_pc as u32 } } _ => o, }) From d26cfee4dc32cd72a9fd46ba96c7ea6d9844e306 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 31 Oct 2024 09:55:18 +0000 Subject: [PATCH 06/13] Update protocol contract data --- .../src/protocol_contract_data.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index 32422db8b5a..1978867c801 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x2ace300b02ca5ab0a25052b1e852913a47292096997ca09f758c0e3624e84560'), - ContractInstanceDeployer: Fr.fromString('0x21e432f60f69ac5eb7582c26c03c6c7e4a3eb577720774bc8e1521561ca752a1'), - ContractClassRegisterer: Fr.fromString('0x1b6d5873cef5a35f681ab9468527f356c96e09b3c64603aef404ec2ad80aa3a9'), - MultiCallEntrypoint: Fr.fromString('0x0966ead8d11933bb3c72547bb997898971715f2275acd4c7d5d86fdf614ba1a2'), - FeeJuice: Fr.fromString('0x017f00981c9ac146ed746087a9ff9edc82b6f276e22840597ec2544f124ea4a0'), - Router: Fr.fromString('0x1d8f25db3e8faa6a96cb1ecf57876a2ee04581deb3c4f181488ccd817abcbdb0'), + AuthRegistry: Fr.fromString('0x1b6d9c59d70879bf40b7162dc09636b98b79c71de6da4f6220c3ce68299b32ff'), + ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), + ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), + MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), + FeeJuice: Fr.fromString('0x24f4432454069413e1fd66afb8948401568606bf58aeb177faeb6fa1986d594e'), + Router: Fr.fromString('0x04a71f51bcba229640326146613b63e0f18d5bc95825a6ccea1273825c49b973'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x1aa73a62d09c12a5430b14860f0830123b4fe37a3b61303319fe67448edbe3b6', + '0x25472984a7cd9688418bae074f9443884f00e467ea7ac5d458f3aca5e73f6d22', ); From ff1c43338b9a0e97f9419dd504f6fa890e07cde8 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Thu, 31 Oct 2024 10:15:58 +0000 Subject: [PATCH 07/13] fix brillig location --- avm-transpiler/src/instructions.rs | 2 +- avm-transpiler/src/transpile.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs index 5cb5ad37a38..23394070bd9 100644 --- a/avm-transpiler/src/instructions.rs +++ b/avm-transpiler/src/instructions.rs @@ -114,7 +114,7 @@ pub enum AvmOperand { U128 { value: u128 }, FF { value: FieldElement }, // Unresolved brillig pc that needs translation to a 16 bit byte-indexed PC. - BRILLIG_LOCATION { brillig_pc: u16 }, + BRILLIG_LOCATION { brillig_pc: u32 }, } impl Display for AvmOperand { diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 487e4652735..4a8934601b8 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -233,22 +233,22 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< }); } BrilligOpcode::Jump { location } => { - assert!(location.num_bits() <= 16); + assert!(location.num_bits() <= 32); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMP_32, - operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }], ..Default::default() }); } BrilligOpcode::JumpIf { condition, location } => { - assert!(location.num_bits() <= 16); + assert!(location.num_bits() <= 32); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMPI_32, indirect: Some( AddressingModeBuilder::default().direct_operand(condition).build(), ), operands: vec![ - AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }, + AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }, make_operand(16, &condition.to_usize()), ], ..Default::default() @@ -297,10 +297,10 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> (Vec< )); } BrilligOpcode::Call { location } => { - assert!(location.num_bits() <= 16); + assert!(location.num_bits() <= 32); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::INTERNALCALL, - operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u16 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }], ..Default::default() }); } From 2d2501da8a8f42874c1161df3f070b24e1f7a19c Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 1 Nov 2024 12:31:59 +0000 Subject: [PATCH 08/13] Work in progress - implementation and start unit tests fixing --- .../vm/avm/tests/execution.test.cpp | 108 +++++---- .../vm/avm/trace/deserialization.cpp | 209 +++++++++------- .../vm/avm/trace/deserialization.hpp | 5 +- .../barretenberg/vm/avm/trace/execution.cpp | 126 ++++++---- .../src/barretenberg/vm/avm/trace/trace.cpp | 226 +++++++++++++----- .../src/barretenberg/vm/avm/trace/trace.hpp | 56 +++-- .../simulator/src/avm/avm_simulator.test.ts | 2 +- 7 files changed, 468 insertions(+), 264 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index 4640e94585d..9e1bce08918 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -61,12 +61,12 @@ class AvmExecutionTests : public ::testing::Test { }; /** - * @brief Generate the execution trace pertaining to the supplied instructions. + * @brief Generate the execution trace pertaining to the supplied bytecode. * - * @param instructions A vector of the instructions to be executed. + * @param bytecode * @return The trace as a vector of Row. */ - std::vector gen_trace_from_instr(const std::vector& bytecode) const + std::vector gen_trace_from_bytecode(const std::vector& bytecode) const { std::vector calldata{}; std::vector returndata{}; @@ -144,7 +144,7 @@ TEST_F(AvmExecutionTests, basicAddReturn) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // 2 instructions ASSERT_THAT(instructions, SizeIs(4)); @@ -164,7 +164,7 @@ TEST_F(AvmExecutionTests, basicAddReturn) Field(&Instruction::operands, ElementsAre(VariantWith(0), VariantWith(0), VariantWith(0))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); validate_trace(std::move(trace), public_inputs, {}, {}); } @@ -192,7 +192,7 @@ TEST_F(AvmExecutionTests, setAndSubOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(4)); @@ -223,7 +223,7 @@ TEST_F(AvmExecutionTests, setAndSubOpcodes) VariantWith(51), VariantWith(1))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the subtraction selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_sub == 1; }); @@ -268,7 +268,7 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) bytecode_hex.append(ret_hex); auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(15)); @@ -296,11 +296,15 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) Field(&Instruction::operands, ElementsAre(VariantWith(0), VariantWith(0), VariantWith(0))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); - // Find the first row enabling the multiplication selector and pc = 13 - auto row = std::ranges::find_if( - trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_mul == 1 && r.main_pc == 13; }); + // Find the first row enabling the multiplication selector and pc of last multiplication + const auto last_mul_pc = + 2 * Deserialization::get_pc_increment(OpCode::SET_8) + 11 * Deserialization::get_pc_increment(OpCode::MUL_8); + + auto row = std::ranges::find_if(trace.begin(), trace.end(), [last_mul_pc](Row r) { + return r.main_sel_op_mul == 1 && r.main_pc == last_mul_pc; + }); EXPECT_EQ(row->main_ic, 244140625); // 5^12 = 244140625 validate_trace(std::move(trace), public_inputs); @@ -323,7 +327,7 @@ TEST_F(AvmExecutionTests, simpleInternalCall) "0D3D2518" // val 222111000 = 0xD3D2518 "0004" // dst_offset 4 + to_hex(OpCode::INTERNALCALL) + // opcode INTERNALCALL - "00000004" // jmp_dest + "0000001C" // jmp_dest 28 + to_hex(OpCode::ADD_16) + // opcode ADD "00" // Indirect flag "0004" // addr a 4 @@ -342,7 +346,7 @@ TEST_F(AvmExecutionTests, simpleInternalCall) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); EXPECT_THAT(instructions, SizeIs(6)); @@ -351,12 +355,12 @@ TEST_F(AvmExecutionTests, simpleInternalCall) // INTERNALCALL EXPECT_THAT(instructions.at(1), AllOf(Field(&Instruction::op_code, OpCode::INTERNALCALL), - Field(&Instruction::operands, ElementsAre(VariantWith(4))))); + Field(&Instruction::operands, ElementsAre(VariantWith(28))))); // INTERNALRETURN EXPECT_EQ(instructions.at(5).op_code, OpCode::INTERNALRETURN); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Expected sequence of PCs during execution std::vector pc_sequence{ 0, 1, 4, 5, 2, 3 }; @@ -417,7 +421,7 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) bytecode_g; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(12)); @@ -431,7 +435,7 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) EXPECT_EQ(instructions.at(i).op_code, opcode_sequence.at(i)); } - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Expected sequence of PCs during execution std::vector pc_sequence{ 0, 1, 2, 8, 6, 7, 9, 10, 4, 5, 11, 3 }; @@ -490,7 +494,7 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(7)); @@ -537,7 +541,7 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) // Positive test for JUMPI. // We invoke CALLDATACOPY on a FF array of one value which will serve as the conditional value -// for JUMPI ans set this value at memory offset 10. +// for JUMPI and set this value at memory offset 10. // Then, we set value 20 (UINT16) at memory offset 101. // Then, a JUMPI call is performed. Depending of the conditional value, the next opcode (ADD) is // omitted or not, i.e., we jump to the subsequent opcode MUL. @@ -587,7 +591,7 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) ; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(8)); @@ -641,7 +645,7 @@ TEST_F(AvmExecutionTests, movOpcode) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(3)); @@ -661,7 +665,7 @@ TEST_F(AvmExecutionTests, movOpcode) Field(&Instruction::operands, ElementsAre(VariantWith(0), VariantWith(171), VariantWith(33))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the MOV selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_mov == 1; }); @@ -699,7 +703,7 @@ TEST_F(AvmExecutionTests, indMovOpcode) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(5)); @@ -709,7 +713,7 @@ TEST_F(AvmExecutionTests, indMovOpcode) Field(&Instruction::operands, ElementsAre(VariantWith(1), VariantWith(1), VariantWith(2))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the MOV selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_mov == 1; }); @@ -738,7 +742,7 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(3)); @@ -751,7 +755,7 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes) VariantWith(17), VariantWith(18))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the cast selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_cast == 1; }); @@ -806,7 +810,7 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBytes) "0100"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -875,7 +879,7 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode) "0100"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -947,7 +951,7 @@ TEST_F(AvmExecutionTests, sha256CompressionOpcode) "0008"; // ret size 8 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector calldata = std::vector(); @@ -1009,7 +1013,7 @@ TEST_F(AvmExecutionTests, poseidon2PermutationOpCode) "0004"; // ret size 8 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata = std::vector(); @@ -1082,7 +1086,7 @@ TEST_F(AvmExecutionTests, keccakf1600OpCode) "0019"; // ret size 25 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector calldata = std::vector(); @@ -1150,7 +1154,7 @@ TEST_F(AvmExecutionTests, embeddedCurveAddOpCode) "0003"; // ret size 1 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -1239,7 +1243,7 @@ TEST_F(AvmExecutionTests, msmOpCode) "0003"; // ret size 3 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); // Assign a vector that we will mutate internally in gen_trace to store the return values; std::vector returndata; @@ -1305,7 +1309,7 @@ TEST_F(AvmExecutionTests, getEnvOpcode) "000B"; // ret size 12 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(12)); @@ -1514,7 +1518,7 @@ TEST_F(AvmExecutionTests, getEnvOpcode) // "0001"; // dst_offset // // auto bytecode = hex_to_bytes(bytecode_hex); -// auto instructions = Deserialization::parse(bytecode); +// auto instructions = Deserialization::parse_bytecode_statically(bytecode); // // // Public inputs for the circuit // std::vector calldata; @@ -1548,7 +1552,7 @@ TEST_F(AvmExecutionTests, l2GasLeft) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(3)); @@ -1560,7 +1564,7 @@ TEST_F(AvmExecutionTests, l2GasLeft) VariantWith(static_cast(EnvironmentVariable::L2GASLEFT)), VariantWith(17))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the L2GASLEFT selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_l2gasleft == 1; }); @@ -1592,7 +1596,7 @@ TEST_F(AvmExecutionTests, daGasLeft) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(3)); @@ -1604,7 +1608,7 @@ TEST_F(AvmExecutionTests, daGasLeft) VariantWith(static_cast(EnvironmentVariable::DAGASLEFT)), VariantWith(39))))); - auto trace = gen_trace_from_instr(bytecode); + auto trace = gen_trace_from_bytecode(bytecode); // Find the first row enabling the DAGASLEFT selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_dagasleft == 1; }); @@ -1631,7 +1635,7 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithTooMuchGasAllocated) public_inputs_vec[L2_START_GAS_LEFT_PCPI_OFFSET] = MAX_L2_GAS_PER_ENQUEUED_CALL + 1; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ExecutionHints execution_hints; EXPECT_THROW_WITH_MESSAGE(gen_trace(bytecode, calldata, public_inputs_vec, returndata, execution_hints), @@ -1651,7 +1655,7 @@ TEST_F(AvmExecutionTests, ExecutorThrowsWithIncorrectNumberOfPublicInputs) std::vector public_inputs_vec = { 1 }; auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ExecutionHints execution_hints; EXPECT_THROW_WITH_MESSAGE(gen_trace(bytecode, calldata, public_inputs_vec, returndata, execution_hints), @@ -1691,7 +1695,7 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(7)); @@ -1788,7 +1792,7 @@ TEST_F(AvmExecutionTests, kernelOutputStorageLoadOpcodeSimple) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(4)); @@ -1849,7 +1853,7 @@ TEST_F(AvmExecutionTests, kernelOutputStorageStoreOpcodeSimple) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); std::vector returndata; @@ -1906,7 +1910,7 @@ TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(5)); @@ -1988,7 +1992,7 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(6)); @@ -2125,7 +2129,7 @@ TEST_F(AvmExecutionTests, opCallOpcodes) "0003"; // ret size 3 (extra read is for the success flag) auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); std::vector returndata; @@ -2199,7 +2203,7 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcode) "0006"; // ret size 6 (dst & exists for all 3) auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(5)); @@ -2234,7 +2238,7 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcodeBadEnum) "0011"; // exists offset auto bytecode = hex_to_bytes(bytecode_hex); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); std::vector calldata; std::vector returndata; @@ -2262,7 +2266,7 @@ TEST_F(AvmExecutionTests, invalidOpcode) "0000"; // ret size 0 auto bytecode = hex_to_bytes(bytecode_hex); - EXPECT_THROW_WITH_MESSAGE(Deserialization::parse(bytecode), "Invalid opcode"); + EXPECT_THROW_WITH_MESSAGE(Deserialization::parse_bytecode_statically(bytecode), "Invalid opcode"); } // Negative test detecting an incomplete instruction: instruction tag present but an operand is missing @@ -2279,7 +2283,7 @@ TEST_F(AvmExecutionTests, truncatedInstructionNoOperand) "FF"; // addr b and missing address for c = a-b auto bytecode = hex_to_bytes(bytecode_hex); - EXPECT_THROW_WITH_MESSAGE(Deserialization::parse(bytecode), "Operand is missing"); + EXPECT_THROW_WITH_MESSAGE(Deserialization::parse_bytecode_statically(bytecode), "Operand is missing"); } } // namespace tests_avm diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index a105982e1bd..55b47920127 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -2,6 +2,7 @@ #include "barretenberg/common/throw_or_abort.hpp" #include "barretenberg/vm/avm/trace/common.hpp" #include "barretenberg/vm/avm/trace/helper.hpp" +#include "barretenberg/vm/avm/trace/instructions.hpp" #include "barretenberg/vm/avm/trace/opcode.hpp" #include @@ -194,104 +195,148 @@ const std::unordered_map OPERAND_TYPE_SIZE = { } // Anonymous namespace +// TODO: once opcodes are frozen, this function can be replaced by a table/map of constants +size_t Deserialization::get_pc_increment(OpCode opcode) +{ + auto const iter = OPCODE_WIRE_FORMAT.find(opcode); + + if (iter == OPCODE_WIRE_FORMAT.end()) { + return 0; + } + + // OPCODE_WIRE_FORMAT does not contain the opcode itself which accounts for 1 byte + size_t increment = 1; + + std::vector inst_format = iter->second; + for (const auto& op_type : inst_format) { + increment += OPERAND_TYPE_SIZE.at(op_type); + } + + return increment; +} + /** - * @brief Parsing of the supplied bytecode into a vector of instructions. It essentially - * checks that each opcode value is in the defined range and extracts the operands + * @brief Parsing of an instruction in the supplied bytecode at byte position pos. This + * checks that the opcode value is in the defined range and extracts the operands * for each opcode based on the specification from OPCODE_WIRE_FORMAT. * * @param bytecode The bytecode to be parsed as a vector of bytes/uint8_t - * @throws runtime_error exception when the bytecode is invalid. - * @return Vector of instructions + * @param pos Bytecode position + * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range + * @return The instruction */ -std::vector Deserialization::parse(std::vector const& bytecode) +Instruction Deserialization::parse(std::vector const& bytecode, size_t pos) { - std::vector instructions; - size_t pos = 0; const auto length = bytecode.size(); debug("------- PARSING BYTECODE -------"); - debug("Parsing bytecode of length: " + std::to_string(length)); - while (pos < length) { - const uint8_t opcode_byte = bytecode.at(pos); + debug("Parse bytecode of length: " + std::to_string(length), " at position: ", pos); - if (!Bytecode::is_valid(opcode_byte)) { - throw_or_abort("Invalid opcode byte: " + to_hex(opcode_byte) + " at position: " + std::to_string(pos)); - } - pos++; + if (pos >= length) { + throw_or_abort("Position is out of range. Position: " + std::to_string(pos) + + " Bytecode length: " + std::to_string(length)); + } + + const uint8_t opcode_byte = bytecode.at(pos); + + if (!Bytecode::is_valid(opcode_byte)) { + throw_or_abort("Invalid opcode byte: " + to_hex(opcode_byte) + " at position: " + std::to_string(pos)); + } + pos++; + + auto const opcode = static_cast(opcode_byte); + auto const iter = OPCODE_WIRE_FORMAT.find(opcode); + if (iter == OPCODE_WIRE_FORMAT.end()) { + throw_or_abort("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); + } + std::vector inst_format = iter->second; - auto const opcode = static_cast(opcode_byte); - auto const iter = OPCODE_WIRE_FORMAT.find(opcode); - if (iter == OPCODE_WIRE_FORMAT.end()) { - throw_or_abort("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); + std::vector operands; + for (OperandType const& op_type : inst_format) { + // No underflow as above condition guarantees pos <= length (after pos++) + const auto operand_size = OPERAND_TYPE_SIZE.at(op_type); + if (length - pos < operand_size) { + throw_or_abort("Operand is missing at position " + std::to_string(pos) + " for opcode " + to_hex(opcode) + + " not enough bytes for operand type " + std::to_string(static_cast(op_type))); } - std::vector inst_format = iter->second; - - std::vector operands; - for (OperandType const& opType : inst_format) { - // No underflow as while condition guarantees pos <= length (after pos++) - if (length - pos < OPERAND_TYPE_SIZE.at(opType)) { - throw_or_abort("Operand is missing at position " + std::to_string(pos) + " for opcode " + - to_hex(opcode) + " not enough bytes for operand type " + - std::to_string(static_cast(opType))); - } - switch (opType) { - case OperandType::TAG: { - uint8_t tag_u8 = bytecode.at(pos); - if (tag_u8 > MAX_MEM_TAG) { - throw_or_abort("Instruction tag is invalid at position " + std::to_string(pos) + - " value: " + std::to_string(tag_u8) + " for opcode: " + to_string(opcode)); - } - operands.emplace_back(static_cast(tag_u8)); - break; - } - case OperandType::INDIRECT8: - case OperandType::UINT8: - operands.emplace_back(bytecode.at(pos)); - break; - case OperandType::INDIRECT16: - case OperandType::UINT16: { - uint16_t operand_u16 = 0; - uint8_t const* pos_ptr = &bytecode.at(pos); - serialize::read(pos_ptr, operand_u16); - operands.emplace_back(operand_u16); - break; - } - case OperandType::UINT32: { - uint32_t operand_u32 = 0; - uint8_t const* pos_ptr = &bytecode.at(pos); - serialize::read(pos_ptr, operand_u32); - operands.emplace_back(operand_u32); - break; - } - case OperandType::UINT64: { - uint64_t operand_u64 = 0; - uint8_t const* pos_ptr = &bytecode.at(pos); - serialize::read(pos_ptr, operand_u64); - operands.emplace_back(operand_u64); - break; + switch (op_type) { + case OperandType::TAG: { + uint8_t tag_u8 = bytecode.at(pos); + if (tag_u8 > MAX_MEM_TAG) { + throw_or_abort("Instruction tag is invalid at position " + std::to_string(pos) + + " value: " + std::to_string(tag_u8) + " for opcode: " + to_string(opcode)); } - case OperandType::UINT128: { - uint128_t operand_u128 = 0; - uint8_t const* pos_ptr = &bytecode.at(pos); - serialize::read(pos_ptr, operand_u128); - operands.emplace_back(operand_u128); - break; - } - case OperandType::FF: { - FF operand_ff; - uint8_t const* pos_ptr = &bytecode.at(pos); - read(pos_ptr, operand_ff); - operands.emplace_back(operand_ff); - } - } - pos += OPERAND_TYPE_SIZE.at(opType); + operands.emplace_back(static_cast(tag_u8)); + break; + } + case OperandType::INDIRECT8: + case OperandType::UINT8: + operands.emplace_back(bytecode.at(pos)); + break; + case OperandType::INDIRECT16: + case OperandType::UINT16: { + uint16_t operand_u16 = 0; + uint8_t const* pos_ptr = &bytecode.at(pos); + serialize::read(pos_ptr, operand_u16); + operands.emplace_back(operand_u16); + break; + } + case OperandType::UINT32: { + uint32_t operand_u32 = 0; + uint8_t const* pos_ptr = &bytecode.at(pos); + serialize::read(pos_ptr, operand_u32); + operands.emplace_back(operand_u32); + break; } - auto instruction = Instruction(opcode, operands); - debug(instruction.to_string()); - instructions.emplace_back(std::move(instruction)); + case OperandType::UINT64: { + uint64_t operand_u64 = 0; + uint8_t const* pos_ptr = &bytecode.at(pos); + serialize::read(pos_ptr, operand_u64); + operands.emplace_back(operand_u64); + break; + } + case OperandType::UINT128: { + uint128_t operand_u128 = 0; + uint8_t const* pos_ptr = &bytecode.at(pos); + serialize::read(pos_ptr, operand_u128); + operands.emplace_back(operand_u128); + break; + } + case OperandType::FF: { + FF operand_ff; + uint8_t const* pos_ptr = &bytecode.at(pos); + read(pos_ptr, operand_ff); + operands.emplace_back(operand_ff); + } + } + pos += operand_size; } - return instructions; + + auto instruction = Instruction(opcode, operands); + debug(instruction.to_string()); + return instruction; }; +/** + * @brief Parse supplied bytecode in a static manner, i.e., parsing instruction by instruction + * without any control flow resolution. In other words, pc is incremented by the size + * of the current parsed instruction. + * + * @param bytecode The bytecode to be parsed as a vector of bytes/uint8_t + * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range + * @return The list of instructions as a vector + */ +std::vector Deserialization::parse_bytecode_statically(std::vector const& bytecode) +{ + uint32_t pc = 0; + std::vector instructions; + while (pc < bytecode.size()) { + const auto instruction = parse(bytecode, pc); + instructions.emplace_back(instruction); + pc += get_pc_increment(instruction.op_code); + } + return instructions; +} + } // namespace bb::avm_trace diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp index 0876f7c7716..96ab9fad7a5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp @@ -1,6 +1,7 @@ #pragma once #include "barretenberg/vm/avm/trace/instructions.hpp" +#include "barretenberg/vm/avm/trace/opcode.hpp" #include #include @@ -17,7 +18,9 @@ class Deserialization { public: Deserialization() = default; - static std::vector parse(std::vector const& bytecode); + static Instruction parse(std::vector const& bytecode, size_t pos); + static std::vector parse_bytecode_statically(std::vector const& bytecode); + static size_t get_pc_increment(OpCode opcode); }; } // namespace bb::avm_trace diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index cc03e9375fb..c04378bbf2a 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -268,7 +268,6 @@ std::vector Execution::gen_trace(std::vector const& calldata, ExecutionHints const& execution_hints) { - vinfo("------- GENERATING TRACE -------"); // TODO(https://github.com/AztecProtocol/aztec-packages/issues/6718): construction of the public input columns // should be done in the kernel - this is stubbed and underconstrained @@ -277,20 +276,19 @@ std::vector Execution::gen_trace(std::vector const& calldata, !public_inputs_vec.empty() ? static_cast(public_inputs_vec[START_SIDE_EFFECT_COUNTER_PCPI_OFFSET]) : 0; - // We should use the public input address, but for now we just take the first element in the list - std::vector bytecode = execution_hints.all_contract_bytecode.at(0).bytecode; - std::vector instructions = Deserialization::parse(bytecode); - vinfo("Deserialized " + std::to_string(instructions.size()) + " instructions"); AvmTraceBuilder trace_builder = Execution::trace_builder_constructor(public_inputs, execution_hints, start_side_effect_counter, calldata); + // We should use the public input address, but for now we just take the first element in the list + std::vector bytecode = execution_hints.all_contract_bytecode.at(0).bytecode; + // Copied version of pc maintained in trace builder. The value of pc is evolving based // on opcode logic and therefore is not maintained here. However, the next opcode in the execution // is determined by this value which require read access to the code below. uint32_t pc = 0; uint32_t counter = 0; - while ((pc = trace_builder.get_pc()) < instructions.size()) { - auto inst = instructions.at(pc); + while ((pc = trace_builder.get_pc()) < bytecode.size()) { + auto inst = Deserialization::parse(bytecode, pc); debug("[PC:" + std::to_string(pc) + "] [IC:" + std::to_string(counter++) + "] " + inst.to_string() + " (gasLeft l2=" + std::to_string(trace_builder.get_l2_gas_left()) + ")"); @@ -302,167 +300,195 @@ std::vector Execution::gen_trace(std::vector const& calldata, trace_builder.op_add(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::ADD_8); break; case OpCode::ADD_16: trace_builder.op_add(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::ADD_16); break; case OpCode::SUB_8: trace_builder.op_sub(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SUB_8); break; case OpCode::SUB_16: trace_builder.op_sub(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SUB_16); break; case OpCode::MUL_8: trace_builder.op_mul(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::MUL_8); break; case OpCode::MUL_16: trace_builder.op_mul(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::MUL_16); break; case OpCode::DIV_8: trace_builder.op_div(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::DIV_8); break; case OpCode::DIV_16: trace_builder.op_div(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::DIV_16); break; case OpCode::FDIV_8: trace_builder.op_fdiv(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::FDIV_8); break; case OpCode::FDIV_16: trace_builder.op_fdiv(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::FDIV_16); break; case OpCode::EQ_8: trace_builder.op_eq(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::EQ_8); break; case OpCode::EQ_16: trace_builder.op_eq(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::EQ_16); break; case OpCode::LT_8: trace_builder.op_lt(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::LT_8); break; case OpCode::LT_16: trace_builder.op_lt(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::LT_16); break; case OpCode::LTE_8: trace_builder.op_lte(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::LTE_8); break; case OpCode::LTE_16: trace_builder.op_lte(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::LTE_16); break; case OpCode::AND_8: trace_builder.op_and(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::AND_8); break; case OpCode::AND_16: trace_builder.op_and(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::AND_16); break; case OpCode::OR_8: trace_builder.op_or(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::OR_8); break; case OpCode::OR_16: trace_builder.op_or(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::OR_16); break; case OpCode::XOR_8: trace_builder.op_xor(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::XOR_8); break; case OpCode::XOR_16: trace_builder.op_xor(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::XOR_16); break; case OpCode::NOT_8: trace_builder.op_not(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2))); + std::get(inst.operands.at(2)), + OpCode::NOT_8); break; case OpCode::NOT_16: trace_builder.op_not(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2))); + std::get(inst.operands.at(2)), + OpCode::NOT_16); break; case OpCode::SHL_8: trace_builder.op_shl(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SHL_8); break; case OpCode::SHL_16: trace_builder.op_shl(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SHL_16); break; case OpCode::SHR_8: trace_builder.op_shr(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SHR_8); break; case OpCode::SHR_16: trace_builder.op_shr(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), std::get(inst.operands.at(2)), - std::get(inst.operands.at(3))); + std::get(inst.operands.at(3)), + OpCode::SHR_16); break; // Compute - Type Conversions @@ -470,13 +496,15 @@ std::vector Execution::gen_trace(std::vector const& calldata, trace_builder.op_cast(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::CAST_8); break; case OpCode::CAST_16: trace_builder.op_cast(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::CAST_16); break; // Execution Environment @@ -528,53 +556,61 @@ std::vector Execution::gen_trace(std::vector const& calldata, trace_builder.op_set(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_8); break; } case OpCode::SET_16: { trace_builder.op_set(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_16); break; } case OpCode::SET_32: { trace_builder.op_set(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_32); break; } case OpCode::SET_64: { trace_builder.op_set(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_64); break; } case OpCode::SET_128: { trace_builder.op_set(std::get(inst.operands.at(0)), uint256_t::from_uint128(std::get(inst.operands.at(2))), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_128); break; } case OpCode::SET_FF: { trace_builder.op_set(std::get(inst.operands.at(0)), std::get(inst.operands.at(2)), std::get(inst.operands.at(3)), - std::get(inst.operands.at(1))); + std::get(inst.operands.at(1)), + OpCode::SET_FF); break; } case OpCode::MOV_8: trace_builder.op_mov(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2))); + std::get(inst.operands.at(2)), + OpCode::MOV_8); break; case OpCode::MOV_16: trace_builder.op_mov(std::get(inst.operands.at(0)), std::get(inst.operands.at(1)), - std::get(inst.operands.at(2))); + std::get(inst.operands.at(2)), + OpCode::MOV_16); break; // World State diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 6eea5900967..2bb6f4498c4 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -23,6 +23,7 @@ #include "barretenberg/vm/avm/trace/addressing_mode.hpp" #include "barretenberg/vm/avm/trace/bytecode_trace.hpp" #include "barretenberg/vm/avm/trace/common.hpp" +#include "barretenberg/vm/avm/trace/deserialization.hpp" #include "barretenberg/vm/avm/trace/fixed_bytes.hpp" #include "barretenberg/vm/avm/trace/fixed_gas.hpp" #include "barretenberg/vm/avm/trace/fixed_powers.hpp" @@ -212,7 +213,7 @@ void AvmTraceBuilder::write_to_memory(AddressWithMode addr, FF val, AvmMemoryTag // op_set increments the pc, so we need to store the current pc and then jump back to it // to legaly reset the pc. auto current_pc = pc; - op_set(static_cast(addr.mode), val, addr.offset, w_tag, true); + op_set(static_cast(addr.mode), val, addr.offset, w_tag, OpCode::SET_FF, true); op_jump(current_pc, true); } @@ -288,7 +289,8 @@ AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs, * @param dst_offset An index in memory pointing to the output of the addition. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_add(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_add( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -333,7 +335,7 @@ void AvmTraceBuilder::op_add(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -346,6 +348,9 @@ void AvmTraceBuilder::op_add(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::ADD_8 || op_code == OpCode::ADD_16); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -357,7 +362,8 @@ void AvmTraceBuilder::op_add(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param dst_offset An index in memory pointing to the output of the subtraction. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_sub(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_sub( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -402,7 +408,7 @@ void AvmTraceBuilder::op_sub(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -415,6 +421,9 @@ void AvmTraceBuilder::op_sub(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::SUB_8 || op_code == OpCode::SUB_16); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -426,7 +435,8 @@ void AvmTraceBuilder::op_sub(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param dst_offset An index in memory pointing to the output of the multiplication. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_mul(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_mul( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -471,7 +481,7 @@ void AvmTraceBuilder::op_mul(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -484,6 +494,9 @@ void AvmTraceBuilder::op_mul(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::MUL_8 || op_code == OpCode::MUL_16); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -495,7 +508,8 @@ void AvmTraceBuilder::op_mul(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param dst_offset An index in memory pointing to the output of the division. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_div(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_div( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -553,7 +567,7 @@ void AvmTraceBuilder::op_div(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_dst.direct_address), .main_op_err = tag_match ? error : FF(1), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -566,6 +580,9 @@ void AvmTraceBuilder::op_div(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::DIV_8 || op_code == OpCode::DIV_16); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -577,7 +594,8 @@ void AvmTraceBuilder::op_div(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param dst_offset An index in memory pointing to the output of the division. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_fdiv(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_fdiv( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -632,7 +650,7 @@ void AvmTraceBuilder::op_fdiv(uint8_t indirect, uint32_t a_offset, uint32_t b_of .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), .main_op_err = tag_match ? error : FF(1), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -645,6 +663,9 @@ void AvmTraceBuilder::op_fdiv(uint8_t indirect, uint32_t a_offset, uint32_t b_of .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(AvmMemoryTag::FF)), }); + + ASSERT(op_code == OpCode::FDIV_8 || op_code == OpCode::FDIV_16); + pc += Deserialization::get_pc_increment(op_code); } /************************************************************************************************** @@ -660,7 +681,7 @@ void AvmTraceBuilder::op_fdiv(uint8_t indirect, uint32_t a_offset, uint32_t b_of * @param dst_offset An index in memory pointing to the output of the equality. * @param in_tag The instruction memory tag of the operands. */ -void AvmTraceBuilder::op_eq(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_eq(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -703,7 +724,7 @@ void AvmTraceBuilder::op_eq(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -716,9 +737,12 @@ void AvmTraceBuilder::op_eq(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(AvmMemoryTag::U1)), }); + + ASSERT(op_code == OpCode::EQ_8 || op_code == OpCode::EQ_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_lt(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_lt(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -757,7 +781,7 @@ void AvmTraceBuilder::op_lt(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -770,9 +794,13 @@ void AvmTraceBuilder::op_lt(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(AvmMemoryTag::U1)), }); + + ASSERT(op_code == OpCode::LT_8 || op_code == OpCode::LT_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_lte(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_lte( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -812,7 +840,7 @@ void AvmTraceBuilder::op_lte(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -825,13 +853,17 @@ void AvmTraceBuilder::op_lte(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(AvmMemoryTag::U1)), }); + + ASSERT(op_code == OpCode::LTE_8 || op_code == OpCode::LTE_16); + pc += Deserialization::get_pc_increment(op_code); } /************************************************************************************************** * COMPUTE - BITWISE **************************************************************************************************/ -void AvmTraceBuilder::op_and(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_and( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -870,7 +902,7 @@ void AvmTraceBuilder::op_and(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_bin = FF(1), @@ -884,9 +916,12 @@ void AvmTraceBuilder::op_and(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::AND_8 || op_code == OpCode::AND_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_or(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_or(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; auto [resolved_a, resolved_b, resolved_c] = @@ -924,7 +959,7 @@ void AvmTraceBuilder::op_or(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_bin = FF(1), @@ -938,9 +973,13 @@ void AvmTraceBuilder::op_or(uint8_t indirect, uint32_t a_offset, uint32_t b_offs .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::OR_8 || op_code == OpCode::OR_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_xor( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -979,7 +1018,7 @@ void AvmTraceBuilder::op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_bin = FF(1), @@ -993,6 +1032,9 @@ void AvmTraceBuilder::op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::XOR_8 || op_code == OpCode::XOR_16); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -1002,7 +1044,7 @@ void AvmTraceBuilder::op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param a_offset An index in memory pointing to the only operand of Not. * @param dst_offset An index in memory pointing to the output of Not. */ -void AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -1041,7 +1083,7 @@ void AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_o .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -1052,9 +1094,13 @@ void AvmTraceBuilder::op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_o .main_tag_err = FF(static_cast(!read_a.tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::NOT_8 || op_code == OpCode::NOT_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_shl( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -1096,7 +1142,7 @@ void AvmTraceBuilder::op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_mem_addr_a = FF(read_a.direct_address), //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -1109,9 +1155,13 @@ void AvmTraceBuilder::op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::SHL_8 || op_code == OpCode::SHL_16); + pc += Deserialization::get_pc_increment(op_code); } -void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_shr( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code) { auto clk = static_cast(main_trace.size()) + 1; @@ -1155,7 +1205,7 @@ void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_off // TODO(8603): uncomment //.main_mem_addr_b = FF(read_b.direct_address), .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(in_tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -1170,6 +1220,9 @@ void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_off .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(in_tag)), }); + + ASSERT(op_code == OpCode::SHR_8 || op_code == OpCode::SHR_16); + pc += Deserialization::get_pc_increment(op_code); } /************************************************************************************************** @@ -1185,7 +1238,8 @@ void AvmTraceBuilder::op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_off * @param dst_offset Offset of destination memory cell. * @param dst_tag Destination tag specifying the type the source value must be casted to. */ -void AvmTraceBuilder::op_cast(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, AvmMemoryTag dst_tag) +void AvmTraceBuilder::op_cast( + uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, AvmMemoryTag dst_tag, OpCode op_code) { auto const clk = static_cast(main_trace.size()) + 1; bool tag_match = true; @@ -1217,7 +1271,7 @@ void AvmTraceBuilder::op_cast(uint8_t indirect, uint32_t a_offset, uint32_t dst_ .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(resolved_a), .main_mem_addr_c = FF(resolved_c), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(memEntry.tag)), .main_rwc = FF(1), .main_sel_mem_op_a = FF(1), @@ -1226,6 +1280,9 @@ void AvmTraceBuilder::op_cast(uint8_t indirect, uint32_t a_offset, uint32_t dst_ .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(dst_tag)), }); + + ASSERT(op_code == OpCode::CAST_8 || op_code == OpCode::CAST_16); + pc += Deserialization::get_pc_increment(op_code); } /************************************************************************************************** @@ -1262,7 +1319,7 @@ Row AvmTraceBuilder::create_kernel_lookup_opcode(uint8_t indirect, uint32_t dst_ .main_ind_addr_a = FF(write_dst.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(write_dst.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_rwa = 1, .main_sel_mem_op_a = 1, .main_sel_resolve_ind_addr_a = FF(static_cast(write_dst.is_indirect)), @@ -1338,6 +1395,7 @@ void AvmTraceBuilder::op_get_env_var(uint8_t indirect, uint8_t env_var, uint32_t break; } } + pc += Deserialization::get_pc_increment(OpCode::GETENVVAR_16); } void AvmTraceBuilder::op_address(uint8_t indirect, uint32_t dst_offset) @@ -1547,13 +1605,15 @@ void AvmTraceBuilder::op_calldata_copy(uint8_t indirect, .main_ib = copy_size, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_c = dst_offset_resolved, - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_op_calldata_copy = 1, .main_sel_slice_gadget = static_cast(tag_match), .main_tag_err = static_cast(!tag_match), .main_w_in_tag = static_cast(AvmMemoryTag::FF), }); + + pc += Deserialization::get_pc_increment(OpCode::CALLDATACOPY); } void AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offset) @@ -1575,11 +1635,13 @@ void AvmTraceBuilder::op_returndata_size(uint8_t indirect, uint32_t dst_offset) .main_clk = clk, .main_call_ptr = call_ptr, .main_internal_return_ptr = FF(internal_return_ptr), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_sel_op_returndata_size = FF(1), .main_tag_err = FF(static_cast(!tag_match)), .main_w_in_tag = FF(static_cast(AvmMemoryTag::U32)), }); + + pc += Deserialization::get_pc_increment(OpCode::RETURNDATASIZE); } void AvmTraceBuilder::op_returndata_copy(uint8_t indirect, @@ -1607,7 +1669,7 @@ void AvmTraceBuilder::op_returndata_copy(uint8_t indirect, main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_sel_op_returndata_copy = FF(1), .main_tag_err = FF(static_cast(!tag_match)), }); @@ -1617,6 +1679,7 @@ void AvmTraceBuilder::op_returndata_copy(uint8_t indirect, auto returndata_slice = std::vector(nested_returndata.begin() + rd_offset, nested_returndata.begin() + rd_offset + copy_size); write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); + pc += Deserialization::get_pc_increment(OpCode::RETURNDATACOPY); } /************************************************************************************************** @@ -1655,7 +1718,7 @@ void AvmTraceBuilder::execute_gasleft(EnvironmentVariable var, uint8_t indirect, .main_ind_addr_a = FF(write_dst.indirect_address), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(write_dst.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_rwa = FF(1), .main_sel_mem_op_a = FF(1), .main_sel_op_dagasleft = (var == EnvironmentVariable::DAGASLEFT) ? FF(1) : FF(0), @@ -1869,7 +1932,8 @@ void AvmTraceBuilder::op_internal_return() * @param dst_offset Memory destination offset where val is written to * @param in_tag The instruction memory tag */ -void AvmTraceBuilder::op_set(uint8_t indirect, FF val_ff, uint32_t dst_offset, AvmMemoryTag in_tag, bool skip_gas) +void AvmTraceBuilder::op_set( + uint8_t indirect, FF val_ff, uint32_t dst_offset, AvmMemoryTag in_tag, OpCode op_code, bool skip_gas) { auto const clk = static_cast(main_trace.size()) + 1; auto [resolved_dst_offset] = Addressing<1>::fromWire(indirect, call_ptr).resolve({ dst_offset }, mem_trace_builder); @@ -1890,7 +1954,7 @@ void AvmTraceBuilder::op_set(uint8_t indirect, FF val_ff, uint32_t dst_offset, A .main_ind_addr_c = FF(write_c.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_c = FF(write_c.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_rwc = 1, .main_sel_mem_op_c = 1, .main_sel_op_set = 1, @@ -1898,6 +1962,11 @@ void AvmTraceBuilder::op_set(uint8_t indirect, FF val_ff, uint32_t dst_offset, A .main_tag_err = static_cast(!write_c.tag_match), .main_w_in_tag = static_cast(in_tag), }); + + const std::set set_family{ OpCode::SET_8, OpCode::SET_16, OpCode::SET_32, + OpCode::SET_64, OpCode::SET_128, OpCode::SET_FF }; + ASSERT(set_family.contains(op_code)); + pc += Deserialization::get_pc_increment(op_code); } /** @@ -1908,7 +1977,7 @@ void AvmTraceBuilder::op_set(uint8_t indirect, FF val_ff, uint32_t dst_offset, A * @param src_offset Offset of source memory cell * @param dst_offset Offset of destination memory cell */ -void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset) +void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset, OpCode op_code) { auto const clk = static_cast(main_trace.size()) + 1; @@ -1936,7 +2005,7 @@ void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = resolved_src_offset, .main_mem_addr_c = resolved_dst_offset, - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(tag), .main_rwc = 1, .main_sel_mem_op_a = 1, @@ -1946,6 +2015,9 @@ void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst .main_tag_err = static_cast(!tag_match), .main_w_in_tag = static_cast(tag), }); + + ASSERT(op_code == OpCode::MOV_8 || op_code == OpCode::MOV_16); + pc += Deserialization::get_pc_increment(op_code); } /************************************************************************************************** @@ -1976,7 +2048,7 @@ Row AvmTraceBuilder::create_kernel_output_opcode(uint8_t indirect, uint32_t clk, .main_ind_addr_a = FF(read_a.indirect_address), .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_rwa = 0, .main_sel_mem_op_a = 1, @@ -2023,7 +2095,7 @@ Row AvmTraceBuilder::create_kernel_output_opcode_with_metadata(uint8_t indirect, .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(data_r_tag), .main_rwa = 0, .main_rwb = 0, @@ -2073,7 +2145,7 @@ Row AvmTraceBuilder::create_kernel_output_opcode_with_set_metadata_output_from_h .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(write_b.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_rwa = 0, .main_rwb = 1, @@ -2112,7 +2184,7 @@ Row AvmTraceBuilder::create_kernel_output_opcode_for_leaf_index(uint32_t clk, .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(write_b.direct_address), - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_rwa = 0, .main_rwb = 1, @@ -2214,7 +2286,6 @@ void AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t // clk++; AddressWithMode write_dst = resolved_dest; - auto old_pc = pc; // Loop over the size and write the hints to memory for (uint32_t i = 0; i < size; i++) { FF value = execution_hints.get_side_effect_hints().at(side_effect_counter); @@ -2222,6 +2293,7 @@ void AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t auto write_a = constrained_write_to_memory( call_ptr, clk, write_dst, value, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); + // TODO(8945): remove fake rows auto row = Row{ .main_clk = clk, .main_ia = value, @@ -2229,9 +2301,7 @@ void AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t .main_ind_addr_a = write_a.indirect_address, .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = write_a.direct_address, // direct address incremented at end of the loop - // FIXME: We are forced to increment the pc since this is in the main trace, - // but this will have to be fixed before bytecode decomposition. - .main_pc = pc++, + .main_pc = pc, .main_rwa = 1, .main_sel_mem_op_a = 1, .main_sel_op_sload = FF(1), @@ -2258,8 +2328,7 @@ void AvmTraceBuilder::op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t // After the first loop, all future write destinations are direct, increment the direct address write_dst = AddressWithMode{ AddressingMode::DIRECT, write_a.direct_address + 1 }; } - // FIXME: Since we changed the PC, we need to reset it - op_jump(old_pc + 1, true); // TODO(8945) + pc += Deserialization::get_pc_increment(OpCode::SLOAD); } void AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t size, uint32_t slot_offset) @@ -2297,11 +2366,11 @@ void AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t // This loop reads a _size_ number of elements from memory and places them into a tuple of (ele, slot) // in the kernel lookup. - auto old_pc = pc; for (uint32_t i = 0; i < size; i++) { auto read_a = constrained_read_from_memory( call_ptr, clk, read_src, AvmMemoryTag::FF, AvmMemoryTag::FF, IntermRegister::IA); + // TODO(8945): remove fake rows Row row = Row{ .main_clk = clk, .main_ia = read_a.val, @@ -2309,9 +2378,7 @@ void AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t .main_ind_addr_a = read_a.indirect_address, .main_internal_return_ptr = internal_return_ptr, .main_mem_addr_a = read_a.direct_address, // direct address incremented at end of the loop - // FIXME: We are forced to increment the pc since this is in the main trace, - // but this will have to be fixed before bytecode decomposition. - .main_pc = pc++, + .main_pc = pc, .main_r_in_tag = static_cast(AvmMemoryTag::FF), .main_sel_mem_op_a = 1, .main_sel_q_kernel_output_lookup = 1, @@ -2334,8 +2401,8 @@ void AvmTraceBuilder::op_sstore(uint8_t indirect, uint32_t src_offset, uint32_t // All future reads are direct, increment the direct address read_src = AddressWithMode{ AddressingMode::DIRECT, read_a.direct_address + 1 }; } - // FIXME: Since we changed the PC, we need to reset it - op_jump(old_pc + 1, true); // TODO(8945) + + pc += Deserialization::get_pc_increment(OpCode::SSTORE); } void AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, @@ -2383,6 +2450,8 @@ void AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash_off debug("emit_note_hash side-effect cnt: ", side_effect_counter); side_effect_counter++; + + pc += Deserialization::get_pc_increment(OpCode::NOTEHASHEXISTS); } void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, @@ -2405,6 +2474,8 @@ void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, debug("nullifier_exists side-effect cnt: ", side_effect_counter); side_effect_counter++; + + pc += Deserialization::get_pc_increment(OpCode::NULLIFIEREXISTS); } void AvmTraceBuilder::op_emit_nullifier(uint8_t indirect, uint32_t nullifier_offset) @@ -2422,6 +2493,8 @@ void AvmTraceBuilder::op_emit_nullifier(uint8_t indirect, uint32_t nullifier_off debug("emit_nullifier side-effect cnt: ", side_effect_counter); side_effect_counter++; + + pc += Deserialization::get_pc_increment(OpCode::EMITNULLIFIER); } void AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, @@ -2451,6 +2524,8 @@ void AvmTraceBuilder::op_l1_to_l2_msg_exists(uint8_t indirect, main_trace.push_back(row); debug("l1_to_l2_msg_exists side-effect cnt: ", side_effect_counter); + + pc += Deserialization::get_pc_increment(OpCode::L1TOL2MSGEXISTS); } void AvmTraceBuilder::op_get_contract_instance( @@ -2468,7 +2543,7 @@ void AvmTraceBuilder::op_get_contract_instance( .main_call_ptr = call_ptr, .main_internal_return_ptr = internal_return_ptr, .main_op_err = FF(1), - .main_pc = pc++, + .main_pc = pc, .main_sel_op_get_contract_instance = FF(1), }; main_trace.push_back(row); @@ -2525,7 +2600,7 @@ void AvmTraceBuilder::op_get_contract_instance( .main_mem_addr_a = FF(read_address.direct_address), //.main_mem_addr_c = FF(write_dst.direct_address), //.main_mem_addr_d = FF(write_exists.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), //.main_sel_mem_op_c = FF(1), @@ -2546,6 +2621,8 @@ void AvmTraceBuilder::op_get_contract_instance( debug("contract_instance cnt: ", side_effect_counter); side_effect_counter++; + + pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); } } @@ -2611,7 +2688,7 @@ void AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, .main_ia = trunc_hash, .main_ib = metadata_log_length, .main_internal_return_ptr = internal_return_ptr, - .main_pc = pc++, + .main_pc = pc, }; kernel_trace_builder.op_emit_unencrypted_log(clk, side_effect_counter, trunc_hash, metadata_log_length); row.main_sel_op_emit_unencrypted_log = FF(1); @@ -2623,6 +2700,7 @@ void AvmTraceBuilder::op_emit_unencrypted_log(uint8_t indirect, debug("emit_unencrypted_log side-effect cnt: ", side_effect_counter); side_effect_counter++; + pc += Deserialization::get_pc_increment(OpCode::EMITUNENCRYPTEDLOG); } void AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipient_offset, uint32_t content_offset) @@ -2642,6 +2720,8 @@ void AvmTraceBuilder::op_emit_l2_to_l1_msg(uint8_t indirect, uint32_t recipient_ debug("emit_l2_to_l1_msg side-effect cnt: ", side_effect_counter); side_effect_counter++; + + pc += Deserialization::get_pc_increment(OpCode::SENDL2TOL1MSG); } /************************************************************************************************** @@ -2703,7 +2783,7 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, .main_mem_addr_b = FF(read_gas_l2.direct_address + 1), .main_mem_addr_c = FF(read_addr.direct_address), .main_mem_addr_d = FF(read_args.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), @@ -2728,6 +2808,8 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, if (opcode == OpCode::CALL) { side_effect_counter = static_cast(hint.end_side_effect_counter); } + + pc += Deserialization::get_pc_increment(opcode); } /** @@ -2933,7 +3015,7 @@ void AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t input_ .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = resolved_input_offset, .main_mem_addr_b = resolved_output_offset, - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_sel_op_poseidon2 = FF(1), }); @@ -3013,6 +3095,8 @@ void AvmTraceBuilder::op_poseidon2_permutation(uint8_t indirect, uint32_t input_ AvmMemoryTag::FF, IntermRegister::ID, AvmMemTraceBuilder::POSEIDON2); + + pc += Deserialization::get_pc_increment(OpCode::POSEIDON2PERM); } /** @@ -3066,7 +3150,7 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect, .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(read_a.direct_address), .main_mem_addr_b = FF(read_b.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U32)), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_b = FF(1), @@ -3104,6 +3188,8 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect, // Write the result to memory after write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); + + pc += Deserialization::get_pc_increment(OpCode::SHA256COMPRESSION); } /** @@ -3137,7 +3223,7 @@ void AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, u .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_a = FF(input_read.direct_address), .main_mem_addr_c = FF(output_read.direct_address), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::U64)), .main_sel_mem_op_a = FF(1), .main_sel_mem_op_c = FF(1), @@ -3159,6 +3245,8 @@ void AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, u std::array result = keccak_trace_builder.keccakf1600(clk, input); // Write the result to memory after write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); + + pc += Deserialization::get_pc_increment(OpCode::KECCAKF1600); } void AvmTraceBuilder::op_ec_add(uint16_t indirect, @@ -3208,7 +3296,7 @@ void AvmTraceBuilder::op_ec_add(uint16_t indirect, main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_sel_op_ecadd = 1, .main_tag_err = FF(0), }); @@ -3219,6 +3307,8 @@ void AvmTraceBuilder::op_ec_add(uint16_t indirect, write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 2, result.is_point_at_infinity(), AvmMemoryTag::U8); + + pc += Deserialization::get_pc_increment(OpCode::ECADD); } void AvmTraceBuilder::op_variable_msm(uint8_t indirect, @@ -3289,7 +3379,7 @@ void AvmTraceBuilder::op_variable_msm(uint8_t indirect, main_trace.push_back(Row{ .main_clk = clk, .main_internal_return_ptr = FF(internal_return_ptr), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_sel_op_msm = 1, .main_tag_err = FF(0), }); @@ -3302,6 +3392,8 @@ void AvmTraceBuilder::op_variable_msm(uint8_t indirect, write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 2, result.is_point_at_infinity(), AvmMemoryTag::U8); + + pc += Deserialization::get_pc_increment(OpCode::MSM); } /************************************************************************************************** @@ -3378,7 +3470,7 @@ void AvmTraceBuilder::op_to_radix_be(uint8_t indirect, // TODO(8603): uncomment //.main_mem_addr_b = read_radix.direct_address, .main_op_err = error ? FF(1) : FF(0), - .main_pc = FF(pc++), + .main_pc = FF(pc), .main_r_in_tag = FF(static_cast(AvmMemoryTag::FF)), .main_sel_mem_op_a = FF(1), // TODO(8603): uncomment @@ -3391,6 +3483,8 @@ void AvmTraceBuilder::op_to_radix_be(uint8_t indirect, }); write_slice_to_memory(resolved_dst_offset, w_in_tag, res); + + pc += Deserialization::get_pc_increment(OpCode::TORADIXBE); } /************************************************************************************************** diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp index f1d0b227abd..76a7162ecb0 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp @@ -40,27 +40,44 @@ class AvmTraceBuilder { uint32_t get_da_gas_left() const { return gas_trace_builder.get_da_gas_left(); } // Compute - Arithmetic - void op_add(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_sub(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_mul(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_div(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_fdiv(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); + void op_add( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::ADD_16); + void op_sub( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::SUB_16); + void op_mul( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::MUL_16); + void op_div( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::DIV_16); + void op_fdiv( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::FDIV_16); // Compute - Comparators - void op_eq(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_lt(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_lte(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); + void op_eq( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::EQ_16); + void op_lt( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::LT_16); + void op_lte( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::LTE_16); // Compute - Bitwise - void op_and(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_or(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset); - void op_shl(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); - void op_shr(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset); + void op_and( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::AND_16); + void op_or( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::OR_16); + void op_xor( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::XOR_16); + void op_not(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, OpCode op_code = OpCode::NOT_16); + void op_shl( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::SHL_16); + void op_shr( + uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, OpCode op_code = OpCode::SHR_16); // Compute - Type Conversions - void op_cast(uint8_t indirect, uint32_t a_offset, uint32_t dst_offset, AvmMemoryTag dst_tag); + void op_cast(uint8_t indirect, + uint32_t a_offset, + uint32_t dst_offset, + AvmMemoryTag dst_tag, + OpCode op_code = OpCode::CAST_16); // Execution Environment void op_get_env_var(uint8_t indirect, uint8_t env_var, uint32_t dst_offset); @@ -100,8 +117,13 @@ class AvmTraceBuilder { // Machine State - Memory // TODO(8945): skip_gas boolean is temporary and should be removed once all fake rows are removed - void op_set(uint8_t indirect, FF val, uint32_t dst_offset, AvmMemoryTag in_tag, bool skip_gas = false); - void op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset); + void op_set(uint8_t indirect, + FF val, + uint32_t dst_offset, + AvmMemoryTag in_tag, + OpCode op_code = OpCode::SET_FF, + bool skip_gas = false); + void op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset, OpCode op_code = OpCode::MOV_16); // World State void op_sload(uint8_t indirect, uint32_t slot_offset, uint32_t size, uint32_t dest_offset); diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 357b2bf4ab7..0805b51d831 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -1081,7 +1081,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { // infinitely loop back to the tested instruction // infinite loop should break on side effect overrun error, // but otherwise will run out of gas - // Note: 15 is the byte index, calucalted as 3*size(Set.wireFormat8) + // Note: 15 is the byte index, calculated as 3*size(Set.wireFormat8) new Jump(/*jumpOffset*/ 15), ]); const context = initContext({ persistableState }); From a241eb2f85ba5f26f9d6555b646834a54a153653 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 1 Nov 2024 16:02:13 +0000 Subject: [PATCH 09/13] Fixing some unit tests --- barretenberg/cpp/pil/avm/main.pil | 12 +- .../vm/avm/generated/relations/main.hpp | 91 ++++----- .../vm/avm/tests/control_flow.test.cpp | 61 +++--- .../vm/avm/tests/execution.test.cpp | 175 ++++++++++-------- .../src/barretenberg/vm/avm/trace/trace.cpp | 53 +++--- 5 files changed, 209 insertions(+), 183 deletions(-) diff --git a/barretenberg/cpp/pil/avm/main.pil b/barretenberg/cpp/pil/avm/main.pil index dc2d5f88507..5242f3db0da 100644 --- a/barretenberg/cpp/pil/avm/main.pil +++ b/barretenberg/cpp/pil/avm/main.pil @@ -379,19 +379,20 @@ namespace main(256); sel_op_jump * (pc' - ia) = 0; #[PC_JUMPI] - sel_op_jumpi * ((1 - id_zero) * (pc' - ia) + id_zero * (pc' - pc - 1)) = 0; + sel_op_jumpi * ((1 - id_zero) * (pc' - ia) + id_zero * (pc' - pc - 8)) = 0; // 8 = size of JUMPI_32 instruction // TODO: Consolidation with #[PC_JUMP] and sel_op_internal_call * (pc' - ia) = 0; sel_op_internal_return * (pc' - ia) = 0; //===== INTERNAL_CALL ====================================================== // - The program counter in the next row should be equal to the value loaded from the ia register - // - We then write the return location (pc + 1) into the call stack (in memory) + // - We then write the return location (pc + 5) into the call stack (in memory), whereby the constant 5 + // corresponds to the size of the internal_call instruction in bytes. #[RETURN_POINTER_INCREMENT] sel_op_internal_call * (internal_return_ptr' - (internal_return_ptr + 1)) = 0; sel_op_internal_call * (internal_return_ptr - mem_addr_b) = 0; sel_op_internal_call * (pc' - ia) = 0; - sel_op_internal_call * ((pc + 1) - ib) = 0; + sel_op_internal_call * ((pc + 5) - ib) = 0; // 5 = size in bytes of internal call instruction // TODO(md): Below relations may be removed through sub-op table lookup sel_op_internal_call * (rwb - 1) = 0; @@ -434,8 +435,9 @@ namespace main(256); // When considering two adjacent main trace rows, // the program counter must increment if not jumping or returning. - #[PC_INCREMENT] - CUR_AND_NEXT_ARE_MAIN * (1 - SEL_ALL_CTRL_FLOW) * (pc' - (pc + 1)) = 0; + // TODO: Adapt PC increment to byte-based PC indexing + // #[PC_INCREMENT] + // CUR_AND_NEXT_ARE_MAIN * (1 - SEL_ALL_CTRL_FLOW) * (pc' - (pc + 1)) = 0; // When considering two adjacent main trace rows, // the internal return ptr must stay the same if not jumping or returning. diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/main.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/main.hpp index 95eb0206a11..4278b2cbb29 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/main.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/generated/relations/main.hpp @@ -10,11 +10,11 @@ template class mainImpl { public: using FF = FF_; - static constexpr std::array SUBRELATION_PARTIAL_LENGTHS = { - 2, 3, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, - 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, - 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 5, 4, 4, 3, 3, 3, 3, 4, 3, 3, 3, - 3, 3, 3, 3, 3, 3, 3, 3, 5, 5, 3, 3, 4, 4, 3, 3, 3, 3, 4, 3, 3, 3, 3, 4, 2, 2 + static constexpr std::array SUBRELATION_PARTIAL_LENGTHS = { + 2, 3, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 5, 4, 4, 3, 3, 3, 3, 4, + 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 5, 3, 3, 4, 4, 3, 3, 3, 3, 4, 3, 3, 3, 3, 4, 2, 2 }; template @@ -599,7 +599,7 @@ template class mainImpl { using Accumulator = typename std::tuple_element_t<83, ContainerOverSubrelations>; auto tmp = (new_term.main_sel_op_jumpi * (((FF(1) - new_term.main_id_zero) * (new_term.main_pc_shift - new_term.main_ia)) + - (new_term.main_id_zero * ((new_term.main_pc_shift - new_term.main_pc) - FF(1))))); + (new_term.main_id_zero * ((new_term.main_pc_shift - new_term.main_pc) - FF(8))))); tmp *= scaling_factor; std::get<83>(evals) += typename Accumulator::View(tmp); } @@ -625,7 +625,7 @@ template class mainImpl { } { using Accumulator = typename std::tuple_element_t<87, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_op_internal_call * ((new_term.main_pc + FF(1)) - new_term.main_ib)); + auto tmp = (new_term.main_sel_op_internal_call * ((new_term.main_pc + FF(5)) - new_term.main_ib)); tmp *= scaling_factor; std::get<87>(evals) += typename Accumulator::View(tmp); } @@ -676,119 +676,112 @@ template class mainImpl { { using Accumulator = typename std::tuple_element_t<95, ContainerOverSubrelations>; auto tmp = ((main_CUR_AND_NEXT_ARE_MAIN * (FF(1) - main_SEL_ALL_CTRL_FLOW)) * - (new_term.main_pc_shift - (new_term.main_pc + FF(1)))); + (new_term.main_internal_return_ptr_shift - new_term.main_internal_return_ptr)); tmp *= scaling_factor; std::get<95>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<96, ContainerOverSubrelations>; - auto tmp = ((main_CUR_AND_NEXT_ARE_MAIN * (FF(1) - main_SEL_ALL_CTRL_FLOW)) * - (new_term.main_internal_return_ptr_shift - new_term.main_internal_return_ptr)); + auto tmp = ((new_term.main_sel_op_internal_call + new_term.main_sel_op_internal_return) * + (new_term.main_space_id - constants_misc_INTERNAL_CALL_SPACE_ID)); tmp *= scaling_factor; std::get<96>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<97, ContainerOverSubrelations>; - auto tmp = ((new_term.main_sel_op_internal_call + new_term.main_sel_op_internal_return) * - (new_term.main_space_id - constants_misc_INTERNAL_CALL_SPACE_ID)); + auto tmp = (((FF(1) - new_term.main_sel_op_internal_call) - new_term.main_sel_op_internal_return) * + (new_term.main_call_ptr - new_term.main_space_id)); tmp *= scaling_factor; std::get<97>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<98, ContainerOverSubrelations>; - auto tmp = (((FF(1) - new_term.main_sel_op_internal_call) - new_term.main_sel_op_internal_return) * - (new_term.main_call_ptr - new_term.main_space_id)); + auto tmp = (new_term.main_sel_op_jumpi * + (((new_term.main_id * new_term.main_inv) - FF(1)) + new_term.main_id_zero)); tmp *= scaling_factor; std::get<98>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<99, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_op_jumpi * - (((new_term.main_id * new_term.main_inv) - FF(1)) + new_term.main_id_zero)); + auto tmp = ((new_term.main_sel_op_jumpi * new_term.main_id_zero) * (FF(1) - new_term.main_inv)); tmp *= scaling_factor; std::get<99>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<100, ContainerOverSubrelations>; - auto tmp = ((new_term.main_sel_op_jumpi * new_term.main_id_zero) * (FF(1) - new_term.main_inv)); + auto tmp = (new_term.main_sel_mov_ia_to_ic - (new_term.main_sel_op_mov * (FF(1) - new_term.main_id_zero))); tmp *= scaling_factor; std::get<100>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<101, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_mov_ia_to_ic - (new_term.main_sel_op_mov * (FF(1) - new_term.main_id_zero))); + auto tmp = (new_term.main_sel_mov_ia_to_ic * (new_term.main_ia - new_term.main_ic)); tmp *= scaling_factor; std::get<101>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<102, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_mov_ia_to_ic * (new_term.main_ia - new_term.main_ic)); + auto tmp = (new_term.main_sel_mov_ib_to_ic * (new_term.main_ib - new_term.main_ic)); tmp *= scaling_factor; std::get<102>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<103, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_mov_ib_to_ic * (new_term.main_ib - new_term.main_ic)); + auto tmp = (new_term.main_sel_op_mov * (new_term.main_r_in_tag - new_term.main_w_in_tag)); tmp *= scaling_factor; std::get<103>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<104, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_op_mov * (new_term.main_r_in_tag - new_term.main_w_in_tag)); + auto tmp = (new_term.main_sel_alu - + ((main_SEL_ALL_ALU * (FF(1) - new_term.main_tag_err)) * (FF(1) - new_term.main_op_err))); tmp *= scaling_factor; std::get<104>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<105, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_alu - - ((main_SEL_ALL_ALU * (FF(1) - new_term.main_tag_err)) * (FF(1) - new_term.main_op_err))); + auto tmp = (main_SEL_ALU_R_TAG * (new_term.main_alu_in_tag - new_term.main_r_in_tag)); tmp *= scaling_factor; std::get<105>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<106, ContainerOverSubrelations>; - auto tmp = (main_SEL_ALU_R_TAG * (new_term.main_alu_in_tag - new_term.main_r_in_tag)); + auto tmp = (main_SEL_ALU_W_TAG * (new_term.main_alu_in_tag - new_term.main_w_in_tag)); tmp *= scaling_factor; std::get<106>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<107, ContainerOverSubrelations>; - auto tmp = (main_SEL_ALU_W_TAG * (new_term.main_alu_in_tag - new_term.main_w_in_tag)); + auto tmp = (new_term.main_sel_op_l2gasleft * (new_term.main_ia - new_term.main_l2_gas_remaining_shift)); tmp *= scaling_factor; std::get<107>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<108, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_op_l2gasleft * (new_term.main_ia - new_term.main_l2_gas_remaining_shift)); + auto tmp = (new_term.main_sel_op_dagasleft * (new_term.main_ia - new_term.main_da_gas_remaining_shift)); tmp *= scaling_factor; std::get<108>(evals) += typename Accumulator::View(tmp); } { using Accumulator = typename std::tuple_element_t<109, ContainerOverSubrelations>; - auto tmp = (new_term.main_sel_op_dagasleft * (new_term.main_ia - new_term.main_da_gas_remaining_shift)); - tmp *= scaling_factor; - std::get<109>(evals) += typename Accumulator::View(tmp); - } - { - using Accumulator = typename std::tuple_element_t<110, ContainerOverSubrelations>; auto tmp = ((new_term.main_ib * (FF(1) - new_term.main_tag_err)) * ((new_term.main_sel_op_calldata_copy + new_term.main_sel_op_external_return) - new_term.main_sel_slice_gadget)); tmp *= scaling_factor; - std::get<110>(evals) += typename Accumulator::View(tmp); + std::get<109>(evals) += typename Accumulator::View(tmp); } { - using Accumulator = typename std::tuple_element_t<111, ContainerOverSubrelations>; + using Accumulator = typename std::tuple_element_t<110, ContainerOverSubrelations>; auto tmp = (new_term.main_bin_op_id - (new_term.main_sel_op_or + (FF(2) * new_term.main_sel_op_xor))); tmp *= scaling_factor; - std::get<111>(evals) += typename Accumulator::View(tmp); + std::get<110>(evals) += typename Accumulator::View(tmp); } { - using Accumulator = typename std::tuple_element_t<112, ContainerOverSubrelations>; + using Accumulator = typename std::tuple_element_t<111, ContainerOverSubrelations>; auto tmp = (new_term.main_sel_bin - ((new_term.main_sel_op_and + new_term.main_sel_op_or) + new_term.main_sel_op_xor)); tmp *= scaling_factor; - std::get<112>(evals) += typename Accumulator::View(tmp); + std::get<111>(evals) += typename Accumulator::View(tmp); } } }; @@ -825,30 +818,28 @@ template class main : public Relation> { case 90: return "RETURN_POINTER_DECREMENT"; case 95: - return "PC_INCREMENT"; - case 96: return "INTERNAL_RETURN_POINTER_CONSISTENCY"; - case 97: + case 96: return "SPACE_ID_INTERNAL"; - case 98: + case 97: return "SPACE_ID_STANDARD_OPCODES"; - case 99: + case 98: return "JMP_CONDITION_RES_1"; - case 100: + case 99: return "JMP_CONDITION_RES_2"; - case 102: + case 101: return "MOV_SAME_VALUE_A"; - case 103: + case 102: return "MOV_SAME_VALUE_B"; - case 104: + case 103: return "MOV_MAIN_SAME_TAG"; - case 108: + case 107: return "L2GASLEFT"; - case 109: + case 108: return "DAGASLEFT"; - case 111: + case 110: return "BIN_SEL_1"; - case 112: + case 111: return "BIN_SEL_2"; } return std::to_string(index); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp index 160bccf7800..312fc74519f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp @@ -1,4 +1,7 @@ +#include "barretenberg/vm/avm/trace/deserialization.hpp" +#include "barretenberg/vm/avm/trace/opcode.hpp" #include "common.test.hpp" +#include namespace tests_avm { @@ -15,7 +18,7 @@ void validate_internal_call(Row const& row, uint32_t current_pc, uint32_t target EXPECT_EQ(row.main_internal_return_ptr, FF(stack_ptr)); EXPECT_EQ(row.main_sel_mem_op_b, FF(1)); EXPECT_EQ(row.main_rwb, FF(1)); - EXPECT_EQ(row.main_ib, FF(current_pc + 1)); + EXPECT_EQ(row.main_ib, FF(current_pc + Deserialization::get_pc_increment(OpCode::INTERNALCALL))); EXPECT_EQ(row.main_mem_addr_b, FF(stack_ptr)); EXPECT_EQ(row.main_w_in_tag, FF(static_cast(AvmMemoryTag::U32))); EXPECT_EQ(row.main_space_id, FF(INTERNAL_CALL_SPACE_ID)); @@ -127,12 +130,12 @@ TEST_F(AvmControlFlowTests, simpleJump) TEST_F(AvmControlFlowTests, simpleCallAndReturn) { uint32_t const CALL_PC = 20; - uint32_t const RETURN_PC = 1; + uint32_t const RETURN_PC = static_cast(Deserialization::get_pc_increment(OpCode::INTERNALCALL)); // trace_builder for the following operation // pc opcode // 0 INTERNAL_CALL(pc=20) // 20 INTERNAL_RETURN - // 1 RETURN + // 5 RETURN trace_builder.op_internal_call(CALL_PC); trace_builder.op_internal_return(); trace_builder.op_return(0, 0, 0); @@ -173,12 +176,18 @@ TEST_F(AvmControlFlowTests, simpleCallAndReturn) TEST_F(AvmControlFlowTests, multipleCallsAndReturns) { - uint32_t const CALL_PC_1 = 420; - uint32_t const CALL_PC_2 = 69; - uint32_t const CALL_PC_3 = 1337; - uint32_t const CALL_PC_4 = 4; + const uint32_t CALL_PC_1 = 420; + const uint32_t CALL_PC_2 = 69; + const uint32_t CALL_PC_3 = 1337; + const uint32_t CALL_PC_4 = 10; - uint32_t const JUMP_PC_1 = 22; + const uint32_t JUMP_PC_1 = 22; + + const uint32_t INTERNALCALL_SIZE = static_cast(Deserialization::get_pc_increment(OpCode::INTERNALCALL)); + const uint32_t NEXT_PC_1 = INTERNALCALL_SIZE; + const uint32_t NEXT_PC_2 = CALL_PC_1 + INTERNALCALL_SIZE; + const uint32_t NEXT_PC_3 = CALL_PC_2 + INTERNALCALL_SIZE; + const uint32_t NEXT_PC_4 = CALL_PC_2 + 2 * INTERNALCALL_SIZE; // trace_builder for the following operation // pc opcode @@ -186,11 +195,11 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) // 420 INTERNAL_CALL(pc=69) // 69 INTERNAL_CALL(pc=1337) // 1337 INTERNAL_RETURN - // 70 INTERNAL_CALL(pc=4) - // 4 INTERNAL_RETURN - // 71 JUMP(pc=22) + // 74 INTERNAL_CALL(pc=10) + // 10 INTERNAL_RETURN + // 79 JUMP(pc=22) // 22 INTERNAL_RETURN - // 421 INTERNAL_RETURN + // 425 INTERNAL_RETURN // 1 RETURN trace_builder.op_internal_call(CALL_PC_1); trace_builder.op_internal_call(CALL_PC_2); @@ -207,8 +216,8 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) // Check call 1 { - auto call_1 = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { - return r.main_sel_op_internal_call == FF(1) && r.main_ib == FF(1); + auto call_1 = std::ranges::find_if(trace.begin(), trace.end(), [NEXT_PC_1](Row r) { + return r.main_sel_op_internal_call == FF(1) && r.main_ib == FF(NEXT_PC_1); }); EXPECT_TRUE(call_1 != trace.end()); auto& call_1_row = trace.at(static_cast(call_1 - trace.begin())); @@ -241,17 +250,17 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_internal_return == FF(1); }); EXPECT_TRUE(return_1 != trace.end()); auto& return_1_row = trace.at(static_cast(return_1 - trace.begin())); - validate_internal_return(return_1_row, CALL_PC_3, CALL_PC_2 + 1, 3); + validate_internal_return(return_1_row, CALL_PC_3, NEXT_PC_3, 3); } // Call 4 { - auto call_4 = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { - return r.main_sel_op_internal_call == FF(1) && r.main_pc == FF(CALL_PC_2 + 1); + auto call_4 = std::ranges::find_if(trace.begin(), trace.end(), [NEXT_PC_3](Row r) { + return r.main_sel_op_internal_call == FF(1) && r.main_pc == FF(NEXT_PC_3); }); EXPECT_TRUE(call_4 != trace.end()); auto& call_4_row = trace.at(static_cast(call_4 - trace.begin())); - validate_internal_call(call_4_row, CALL_PC_2 + 1, CALL_PC_4, 2); + validate_internal_call(call_4_row, NEXT_PC_3, CALL_PC_4, 2); } // Return 2 @@ -261,13 +270,13 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) }); EXPECT_TRUE(return_2 != trace.end()); auto& return_2_row = trace.at(static_cast(return_2 - trace.begin())); - validate_internal_return(return_2_row, CALL_PC_4, CALL_PC_2 + 2, 3); + validate_internal_return(return_2_row, CALL_PC_4, NEXT_PC_4, 3); } // Jump 1 { - auto jump_1 = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { - return r.main_sel_op_jump == FF(1) && r.main_pc == FF(CALL_PC_2 + 2); + auto jump_1 = std::ranges::find_if(trace.begin(), trace.end(), [NEXT_PC_4](Row r) { + return r.main_sel_op_jump == FF(1) && r.main_pc == FF(NEXT_PC_4); }); EXPECT_TRUE(jump_1 != trace.end()); EXPECT_EQ(jump_1->main_ia, FF(JUMP_PC_1)); @@ -281,17 +290,17 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) }); EXPECT_TRUE(return_3 != trace.end()); auto& return_3_row = trace.at(static_cast(return_3 - trace.begin())); - validate_internal_return(return_3_row, JUMP_PC_1, CALL_PC_1 + 1, 2); + validate_internal_return(return_3_row, JUMP_PC_1, NEXT_PC_2, 2); } // Return 4 { - auto return_4 = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { - return r.main_sel_op_internal_return == FF(1) && r.main_pc == FF(CALL_PC_1 + 1); + auto return_4 = std::ranges::find_if(trace.begin(), trace.end(), [NEXT_PC_2](Row r) { + return r.main_sel_op_internal_return == FF(1) && r.main_pc == FF(NEXT_PC_2); }); EXPECT_TRUE(return_4 != trace.end()); auto& return_4_row = trace.at(static_cast(return_4 - trace.begin())); - validate_internal_return(return_4_row, CALL_PC_1 + 1, 1, 1); + validate_internal_return(return_4_row, NEXT_PC_2, NEXT_PC_1, 1); } // Halt row @@ -299,7 +308,7 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_external_return == FF(1); }); EXPECT_TRUE(halt_row != trace.end()); - EXPECT_EQ(halt_row->main_pc, FF(1)); + EXPECT_EQ(halt_row->main_pc, FF(NEXT_PC_1)); validate_trace(std::move(trace), public_inputs); } diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index 9e1bce08918..ba245cbcfc0 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -317,13 +317,14 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) // CALL internal routine // ADD M[4] with M[7] and output in M[9] // Internal routine bytecode is at the end. -// Bytecode layout: SET INTERNAL_CALL ADD RETURN SET INTERNAL_RETURN -// 0 1 2 3 4 5 +// Bytecode layout: SET_32 INTERNAL_CALL ADD_16 RETURN SET_32 INTERNAL_RETURN +// Instr. Index 0 1 2 3 4 5 +// PC Index 0 9 14 22 28 37 TEST_F(AvmExecutionTests, simpleInternalCall) { - std::string bytecode_hex = to_hex(OpCode::SET_32) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + + std::string bytecode_hex = to_hex(OpCode::SET_32) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // "0D3D2518" // val 222111000 = 0xD3D2518 "0004" // dst_offset 4 + to_hex(OpCode::INTERNALCALL) + // opcode INTERNALCALL @@ -363,7 +364,7 @@ TEST_F(AvmExecutionTests, simpleInternalCall) auto trace = gen_trace_from_bytecode(bytecode); // Expected sequence of PCs during execution - std::vector pc_sequence{ 0, 1, 4, 5, 2, 3 }; + std::vector pc_sequence{ 0, 9, 28, 37, 14, 22 }; for (size_t i = 0; i < 6; i++) { EXPECT_EQ(trace.at(i + 1).main_pc, pc_sequence.at(i)); @@ -384,10 +385,11 @@ TEST_F(AvmExecutionTests, simpleInternalCall) // MAIN: SET(4,2) SET(7,3) G // Whole execution should compute: (4 + 7) * 17 = 187 // Bytecode layout: SET(4,2) SET(7,3) INTERNAL_CALL_G RETURN BYTECODE(F2) BYTECODE(F1) BYTECODE(G) -// 0 1 2 3 4 6 8 +// Instr Index: 0 1 2 3 4 6 8 +// PC Index: 0 9 18 23 29 35 41 // BYTECODE(F1): ADD(2,3,2) INTERNAL_RETURN // BYTECODE(F2): MUL(2,3,2) INTERNAL_RETURN -// BYTECODE(G): INTERNAL_CALL(6) SET(17,3) INTERNAL_CALL(4) INTERNAL_RETURN +// BYTECODE(G): INTERNAL_CALL(35) SET(17,3) INTERNAL_CALL(29) INTERNAL_RETURN TEST_F(AvmExecutionTests, nestedInternalCalls) { auto internalCallInstructionHex = [](std::string const& dst_offset) { @@ -414,10 +416,10 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) const std::string bytecode_f1 = to_hex(OpCode::ADD_8) + tag_address_arguments + to_hex(OpCode::INTERNALRETURN); const std::string bytecode_f2 = to_hex(OpCode::MUL_8) + tag_address_arguments + to_hex(OpCode::INTERNALRETURN); - const std::string bytecode_g = internalCallInstructionHex("06") + setInstructionHex("11", "03") + - internalCallInstructionHex("04") + to_hex(OpCode::INTERNALRETURN); + const std::string bytecode_g = internalCallInstructionHex("23") + setInstructionHex("11", "03") + + internalCallInstructionHex("1D") + to_hex(OpCode::INTERNALRETURN); std::string bytecode_hex = setInstructionHex("04", "02") + setInstructionHex("07", "03") + - internalCallInstructionHex("08") + return_instruction_hex + bytecode_f2 + bytecode_f1 + + internalCallInstructionHex("29") + return_instruction_hex + bytecode_f2 + bytecode_f1 + bytecode_g; auto bytecode = hex_to_bytes(bytecode_hex); @@ -438,45 +440,46 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) auto trace = gen_trace_from_bytecode(bytecode); // Expected sequence of PCs during execution - std::vector pc_sequence{ 0, 1, 2, 8, 6, 7, 9, 10, 4, 5, 11, 3 }; + std::vector pc_sequence{ 0, 9, 18, 41, 35, 40, 46, 55, 29, 34, 60, 23 }; - for (size_t i = 0; i < 6; i++) { + for (size_t i = 0; i < 12; i++) { EXPECT_EQ(trace.at(i + 1).main_pc, pc_sequence.at(i)); } // Find the first row enabling the multiplication selector. auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_mul == 1; }); EXPECT_EQ(row->main_ic, 187); - EXPECT_EQ(row->main_pc, 4); + EXPECT_EQ(row->main_pc, 29); validate_trace(std::move(trace), public_inputs); } // Positive test with JUMP and CALLDATACOPY -// We test bytecode which first invoke CALLDATACOPY on a FF array of two values. +// We test bytecode which first invokes CALLDATACOPY on a FF array of two values. // Then, a JUMP call skips a SUB opcode to land to a FDIV operation and RETURN. // Calldata: [13, 156] -// Bytecode layout: CALLDATACOPY JUMP SUB FDIV RETURN -// 0 1 2 3 4 +// Bytecode layout: SET_8 SET_8 CALLDATACOPY JUMP SUB FDIV RETURN +// Instr. Index: 0 1 2 3 4 5 6 +// PC index: 0 5 10 18 23 28 33 TEST_F(AvmExecutionTests, jumpAndCalldatacopy) { - std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + - "00" // val - "00" // dst_offset 101 - + to_hex(OpCode::SET_8) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + + std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // + "00" // val + "00" // dst_offset + + to_hex(OpCode::SET_8) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // "02" // val - "01" // dst_offset 101 + "01" // dst_offset + to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY (no in tag) "00" // Indirect flag "0000" // cd_offset - "0001" // copy_size + "0001" // copy_size offset 2 and copysize 2 "000A" // dst_offset // M[10] = 13, M[11] = 156 + to_hex(OpCode::JUMP_32) + // opcode JUMP - "00000005" // jmp_dest (FDIV located at 3) + "0000001C" // jmp_dest (FDIV located at 28) + to_hex(OpCode::SUB_8) + // opcode SUB "00" // Indirect flag "0B" // addr 11 @@ -512,23 +515,22 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) // JUMP EXPECT_THAT(instructions.at(3), AllOf(Field(&Instruction::op_code, OpCode::JUMP_32), - Field(&Instruction::operands, ElementsAre(VariantWith(5))))); + Field(&Instruction::operands, ElementsAre(VariantWith(28))))); std::vector returndata; ExecutionHints execution_hints; auto trace = gen_trace(bytecode, std::vector{ 13, 156 }, public_inputs_vec, returndata, execution_hints); // Expected sequence of PCs during execution - std::vector pc_sequence{ - 0, 1, 2, 3, 4, 6, - }; + std::vector pc_sequence{ 0, 5, 10, 18, 28, 33 }; - for (size_t i = 0; i < 4; i++) { + for (size_t i = 0; i < 6; i++) { EXPECT_EQ(trace.at(i + 1).main_pc, pc_sequence.at(i)); } // Find the first row enabling the fdiv selector. auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_fdiv == 1; }); + ASSERT_TRUE(row != trace.end()); EXPECT_EQ(row->main_ic, 12); // Find the first row enabling the subtraction selector. @@ -545,19 +547,20 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) // Then, we set value 20 (UINT16) at memory offset 101. // Then, a JUMPI call is performed. Depending of the conditional value, the next opcode (ADD) is // omitted or not, i.e., we jump to the subsequent opcode MUL. -// Bytecode layout: CALLDATACOPY SET JUMPI ADD MUL RETURN -// 0 1 2 3 4 5 -// We test this bytecode with two calldatacopy values: 9873123 and 0. +// Bytecode layout: SET SET CALLDATACOPY SET JUMPI ADD MUL RETURN +// Instr. Index: 0 1 2 3 4 5 6 7 +// PC Index: 0 5 10 18 23 31 39 44 +// We test this bytecode with two calldatacopy inputs: {9873123} and {0}. TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) { - std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + - "00" // val - "00" // dst_offset - + to_hex(OpCode::SET_8) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + + std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // + "00" // val + "00" // dst_offset + + to_hex(OpCode::SET_8) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // "01" // val "01" // dst_offset + to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY (no in tag) @@ -567,27 +570,27 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) "000A" // dst_offset 10 + to_hex(OpCode::SET_8) + // opcode SET "00" // Indirect flag - + to_hex(AvmMemoryTag::U16) + - "14" // val 20 - "65" // dst_offset 101 - + to_hex(OpCode::JUMPI_32) + // opcode JUMPI - "00" // Indirect flag - "00000006" // jmp_dest (MUL located at 6) - "000A" // cond_offset 10 - + to_hex(OpCode::ADD_16) + // opcode ADD - "00" // Indirect flag - "0065" // addr 101 - "0065" // addr 101 - "0065" // output addr 101 - + to_hex(OpCode::MUL_8) + // opcode MUL - "00" // Indirect flag - "65" // addr 101 - "65" // addr 101 - "66" // output of MUL addr 102 - + to_hex(OpCode::RETURN) + // opcode RETURN - "00" // Indirect flag - "0000" // ret offset 0 - "0000" // ret size 0 + + to_hex(AvmMemoryTag::U16) + // + "14" // val 20 + "65" // dst_offset 101 + + to_hex(OpCode::JUMPI_32) + // opcode JUMPI + "00" // Indirect flag + "00000027" // jmp_dest (MUL located at 39) + "000A" // cond_offset 10 + + to_hex(OpCode::ADD_16) + // opcode ADD + "00" // Indirect flag + "0065" // addr 101 + "0065" // addr 101 + "0065" // output addr 101 + + to_hex(OpCode::MUL_8) + // opcode MUL + "00" // Indirect flag + "65" // addr 101 + "65" // addr 101 + "66" // output of MUL addr 102 + + to_hex(OpCode::RETURN) + // opcode RETURN + "00" // Indirect flag + "0000" // ret offset 0 + "0000" // ret size 0 ; auto bytecode = hex_to_bytes(bytecode_hex); @@ -602,7 +605,7 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) instructions.at(4), AllOf(Field(&Instruction::op_code, OpCode::JUMPI_32), Field(&Instruction::operands, - ElementsAre(VariantWith(0), VariantWith(6), VariantWith(10))))); + ElementsAre(VariantWith(0), VariantWith(39), VariantWith(10))))); std::vector returndata; ExecutionHints execution_hints; @@ -610,15 +613,15 @@ TEST_F(AvmExecutionTests, jumpiAndCalldatacopy) auto trace_no_jump = gen_trace(bytecode, std::vector{ 0 }, public_inputs_vec, returndata, execution_hints); // Expected sequence of PCs during execution with jump - std::vector pc_sequence_jump{ 0, 1, 2, 3, 4, 6, 7 }; + std::vector pc_sequence_jump{ 0, 5, 10, 18, 23, 39, 44 }; // Expected sequence of PCs during execution without jump - std::vector pc_sequence_no_jump{ 0, 1, 2, 3, 4, 5, 6, 7 }; + std::vector pc_sequence_no_jump{ 0, 5, 10, 18, 23, 31, 39, 44 }; - for (size_t i = 0; i < 5; i++) { + for (size_t i = 0; i < 7; i++) { EXPECT_EQ(trace_jump.at(i + 1).main_pc, pc_sequence_jump.at(i)); } - for (size_t i = 0; i < 6; i++) { + for (size_t i = 0; i < 8; i++) { EXPECT_EQ(trace_no_jump.at(i + 1).main_pc, pc_sequence_no_jump.at(i)); } @@ -1723,12 +1726,14 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) // CHECK EMIT NULLIFIER auto emit_nullifier_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_emit_nullifier == 1; }); + ASSERT_TRUE(emit_nullifier_row != trace.end()); EXPECT_EQ(emit_nullifier_row->main_ia, 1); EXPECT_EQ(emit_nullifier_row->main_side_effect_counter, 1); uint32_t emit_nullifier_out_offset = START_EMIT_NULLIFIER_WRITE_OFFSET; auto emit_nullifier_kernel_out_row = std::ranges::find_if( trace.begin(), trace.end(), [&](Row r) { return r.main_clk == emit_nullifier_out_offset; }); + ASSERT_TRUE(emit_nullifier_kernel_out_row != trace.end()); EXPECT_EQ(emit_nullifier_kernel_out_row->main_kernel_value_out, 1); EXPECT_EQ(emit_nullifier_kernel_out_row->main_kernel_side_effect_out, 1); feed_output(emit_nullifier_out_offset, 1, 1, 0); @@ -1736,6 +1741,8 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) // CHECK EMIT UNENCRYPTED LOG auto emit_log_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_emit_unencrypted_log == 1; }); + ASSERT_TRUE(emit_log_row != trace.end()); + // Trust me bro for now, this is the truncated sha output FF expected_hash = FF(std::string("0x00b5c135991581f3049df936e35ef23af34bb04a4775426481d944d35a618e9d")); EXPECT_EQ(emit_log_row->main_ia, expected_hash); @@ -1746,6 +1753,7 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) uint32_t emit_log_out_offset = START_EMIT_UNENCRYPTED_LOG_WRITE_OFFSET; auto emit_log_kernel_out_row = std::ranges::find_if(trace.begin(), trace.end(), [&](Row r) { return r.main_clk == emit_log_out_offset; }); + ASSERT_TRUE(emit_log_kernel_out_row != trace.end()); EXPECT_EQ(emit_log_kernel_out_row->main_kernel_value_out, expected_hash); EXPECT_EQ(emit_log_kernel_out_row->main_kernel_side_effect_out, 2); EXPECT_EQ(emit_log_kernel_out_row->main_kernel_metadata_out, 40); @@ -1754,12 +1762,14 @@ TEST_F(AvmExecutionTests, kernelOutputEmitOpcodes) // CHECK SEND L2 TO L1 MSG auto send_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_emit_l2_to_l1_msg == 1; }); + ASSERT_TRUE(send_row != trace.end()); EXPECT_EQ(send_row->main_ia, 1); EXPECT_EQ(send_row->main_ib, 1); EXPECT_EQ(send_row->main_side_effect_counter, 3); auto msg_out_row = std::ranges::find_if( trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_EMIT_L2_TO_L1_MSG_WRITE_OFFSET; }); + ASSERT_TRUE(msg_out_row != trace.end()); EXPECT_EQ(msg_out_row->main_kernel_value_out, 1); EXPECT_EQ(msg_out_row->main_kernel_side_effect_out, 3); EXPECT_EQ(msg_out_row->main_kernel_metadata_out, 1); @@ -1960,15 +1970,14 @@ TEST_F(AvmExecutionTests, kernelOutputStorageOpcodes) TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) { // hash exists from a value that has not previously been written to will require a hint to process - std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET - "00" // Indirect flag - + to_hex(AvmMemoryTag::U32) + - "01" // value 1 - "01" // dst_offset 1 - // Cast set to field - + to_hex(OpCode::CAST_8) + // opcode CAST - "00" // Indirect flag - + to_hex(AvmMemoryTag::FF) + + std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET + "00" // Indirect flag + + to_hex(AvmMemoryTag::U32) + // + "01" // value 1 + "01" // dst_offset 1 + + to_hex(OpCode::CAST_8) + // opcode CAST to field + "00" // Indirect flag + + to_hex(AvmMemoryTag::FF) + // "01" // dst 1 "01" // dst 1 + to_hex(OpCode::NOTEHASHEXISTS) + // opcode NOTEHASHEXISTS @@ -2009,12 +2018,14 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) // CHECK NOTEHASHEXISTS auto note_hash_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_note_hash_exists == 1; }); + ASSERT_TRUE(note_hash_row != trace.end()); EXPECT_EQ(note_hash_row->main_ia, 1); // Read value EXPECT_EQ(note_hash_row->main_ib, 1); // Storage slot EXPECT_EQ(note_hash_row->main_side_effect_counter, 0); auto note_hash_out_row = std::ranges::find_if( trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_NOTE_HASH_EXISTS_WRITE_OFFSET; }); + ASSERT_TRUE(note_hash_out_row != trace.end()); EXPECT_EQ(note_hash_out_row->main_kernel_value_out, 1); // value EXPECT_EQ(note_hash_out_row->main_kernel_side_effect_out, 0); EXPECT_EQ(note_hash_out_row->main_kernel_metadata_out, 1); // exists @@ -2023,12 +2034,14 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) // CHECK NULLIFIEREXISTS auto nullifier_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_nullifier_exists == 1; }); + ASSERT_TRUE(nullifier_row != trace.end()); EXPECT_EQ(nullifier_row->main_ia, 1); // Read value EXPECT_EQ(nullifier_row->main_ib, 1); // Storage slot EXPECT_EQ(nullifier_row->main_side_effect_counter, 1); auto nullifier_out_row = std::ranges::find_if( trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_NULLIFIER_EXISTS_OFFSET; }); + ASSERT_TRUE(nullifier_out_row != trace.end()); EXPECT_EQ(nullifier_out_row->main_kernel_value_out, 1); // value // TODO(#8287) EXPECT_EQ(nullifier_out_row->main_kernel_side_effect_out, 0); @@ -2038,12 +2051,14 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes) // CHECK L1TOL2MSGEXISTS auto l1_to_l2_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_l1_to_l2_msg_exists == 1; }); + ASSERT_TRUE(l1_to_l2_row != trace.end()); EXPECT_EQ(l1_to_l2_row->main_ia, 1); // Read value EXPECT_EQ(l1_to_l2_row->main_ib, 1); // Storage slot EXPECT_EQ(l1_to_l2_row->main_side_effect_counter, 2); auto msg_out_row = std::ranges::find_if( trace.begin(), trace.end(), [&](Row r) { return r.main_clk == START_L1_TO_L2_MSG_EXISTS_WRITE_OFFSET; }); + ASSERT_TRUE(msg_out_row != trace.end()); EXPECT_EQ(msg_out_row->main_kernel_value_out, 1); // value // TODO(#8287) EXPECT_EQ(msg_out_row->main_kernel_side_effect_out, 0); @@ -2239,6 +2254,7 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcodeBadEnum) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse_bytecode_statically(bytecode); + ASSERT_THAT(instructions, SizeIs(2)); std::vector calldata; std::vector returndata; @@ -2248,6 +2264,7 @@ TEST_F(AvmExecutionTests, opGetContractInstanceOpcodeBadEnum) // Bad enum should raise error flag auto address_row = std::ranges::find_if( trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_get_contract_instance == 1; }); + ASSERT_TRUE(address_row != trace.end()); EXPECT_EQ(address_row->main_op_err, FF(1)); validate_trace(std::move(trace), public_inputs, calldata, returndata); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 2bb6f4498c4..05f3be299c9 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -211,7 +211,7 @@ FF AvmTraceBuilder::unconstrained_read_from_memory(AddressWithMode addr) void AvmTraceBuilder::write_to_memory(AddressWithMode addr, FF val, AvmMemoryTag w_tag) { // op_set increments the pc, so we need to store the current pc and then jump back to it - // to legaly reset the pc. + // to legally reset the pc. auto current_pc = pc; op_set(static_cast(addr.mode), val, addr.offset, w_tag, OpCode::SET_FF, true); op_jump(current_pc, true); @@ -1678,8 +1678,11 @@ void AvmTraceBuilder::op_returndata_copy(uint8_t indirect, // TODO: validate bounds auto returndata_slice = std::vector(nested_returndata.begin() + rd_offset, nested_returndata.begin() + rd_offset + copy_size); - write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); + pc += Deserialization::get_pc_increment(OpCode::RETURNDATACOPY); + + // Crucial to perform this operation after having incremented pc + write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); } /************************************************************************************************** @@ -1799,7 +1802,8 @@ void AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t con const bool id_zero = read_d.val == 0; FF const inv = !id_zero ? read_d.val.invert() : 1; - uint32_t next_pc = !id_zero ? jmp_dest : pc + 1; + uint32_t next_pc = + !id_zero ? jmp_dest : pc + static_cast(Deserialization::get_pc_increment(OpCode::JUMPI_32)); // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::JUMPI_32); @@ -1841,13 +1845,13 @@ void AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t con void AvmTraceBuilder::op_internal_call(uint32_t jmp_dest) { auto clk = static_cast(main_trace.size()) + 1; - + const auto next_pc = pc + Deserialization::get_pc_increment(OpCode::INTERNALCALL); // We store the next instruction as the return location mem_trace_builder.write_into_memory(INTERNAL_CALL_SPACE_ID, clk, IntermRegister::IB, internal_return_ptr, - FF(pc + 1), + FF(next_pc), AvmMemoryTag::FF, AvmMemoryTag::U32); @@ -1858,7 +1862,7 @@ void AvmTraceBuilder::op_internal_call(uint32_t jmp_dest) .main_clk = clk, .main_call_ptr = call_ptr, .main_ia = FF(jmp_dest), - .main_ib = FF(pc + 1), + .main_ib = FF(next_pc), .main_internal_return_ptr = FF(internal_return_ptr), .main_mem_addr_b = FF(internal_return_ptr), .main_pc = FF(pc), @@ -2433,6 +2437,7 @@ void AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, main_trace.push_back(row); debug("note_hash_exists side-effect cnt: ", side_effect_counter); + pc += Deserialization::get_pc_increment(OpCode::NOTEHASHEXISTS); } void AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash_offset) @@ -2451,7 +2456,7 @@ void AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash_off debug("emit_note_hash side-effect cnt: ", side_effect_counter); side_effect_counter++; - pc += Deserialization::get_pc_increment(OpCode::NOTEHASHEXISTS); + pc += Deserialization::get_pc_increment(OpCode::EMITNOTEHASH); } void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, @@ -2547,7 +2552,7 @@ void AvmTraceBuilder::op_get_contract_instance( .main_sel_op_get_contract_instance = FF(1), }; main_trace.push_back(row); - + pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); } else { ContractInstanceMember chosen_member = static_cast(member_enum); @@ -2612,6 +2617,8 @@ void AvmTraceBuilder::op_get_contract_instance( .main_tag_err = FF(static_cast(!tag_match)), }); + pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); + // TODO(8603): once instructions can have multiple different tags for writes, remove this and do a constrained // writes write_to_memory(resolved_dst_offset, member_value, AvmMemoryTag::FF); @@ -2621,8 +2628,6 @@ void AvmTraceBuilder::op_get_contract_instance( debug("contract_instance cnt: ", side_effect_counter); side_effect_counter++; - - pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); } } @@ -2797,6 +2802,8 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, .main_tag_err = FF(static_cast(!tag_match)), }); + pc += Deserialization::get_pc_increment(opcode); + // Write the success flag to memory write_to_memory(resolved_success_offset, hint.success, AvmMemoryTag::U1); external_call_counter++; @@ -2808,8 +2815,6 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, if (opcode == OpCode::CALL) { side_effect_counter = static_cast(hint.end_side_effect_counter); } - - pc += Deserialization::get_pc_increment(opcode); } /** @@ -3186,10 +3191,10 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect, ff_result.emplace_back(result[i]); } - // Write the result to memory after - write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); - pc += Deserialization::get_pc_increment(OpCode::SHA256COMPRESSION); + + // Crucial to perform this operation after having incremented pc + write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); } /** @@ -3243,10 +3248,11 @@ void AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, u // Note: We use the keccak_op_clk to ensure that the keccakf1600 operation is performed at the same clock cycle // as the main trace that has the selector std::array result = keccak_trace_builder.keccakf1600(clk, input); - // Write the result to memory after - write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); pc += Deserialization::get_pc_increment(OpCode::KECCAKF1600); + + // Crucial to perform this operation after having incremented pc + write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); } void AvmTraceBuilder::op_ec_add(uint16_t indirect, @@ -3303,12 +3309,12 @@ void AvmTraceBuilder::op_ec_add(uint16_t indirect, gas_trace_builder.constrain_gas(clk, OpCode::ECADD); + pc += Deserialization::get_pc_increment(OpCode::ECADD); + // Write point coordinates write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 2, result.is_point_at_infinity(), AvmMemoryTag::U8); - - pc += Deserialization::get_pc_increment(OpCode::ECADD); } void AvmTraceBuilder::op_variable_msm(uint8_t indirect, @@ -3388,12 +3394,12 @@ void AvmTraceBuilder::op_variable_msm(uint8_t indirect, // run out of gas. Casting/truncating here is not secure. gas_trace_builder.constrain_gas(clk, OpCode::MSM, static_cast(points_length)); + pc += Deserialization::get_pc_increment(OpCode::MSM); + // Write the result back to memory [x, y, inf] with tags [FF, FF, U8] write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 2, result.is_point_at_infinity(), AvmMemoryTag::U8); - - pc += Deserialization::get_pc_increment(OpCode::MSM); } /************************************************************************************************** @@ -3482,9 +3488,10 @@ void AvmTraceBuilder::op_to_radix_be(uint8_t indirect, .main_w_in_tag = FF(static_cast(w_in_tag)), }); - write_slice_to_memory(resolved_dst_offset, w_in_tag, res); - pc += Deserialization::get_pc_increment(OpCode::TORADIXBE); + + // Crucial to perform this operation after having incremented pc + write_slice_to_memory(resolved_dst_offset, w_in_tag, res); } /************************************************************************************************** From ea230436b412ba252131e723ad8ae4a90d80587c Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Mon, 4 Nov 2024 11:02:41 +0000 Subject: [PATCH 10/13] fix call stack struct --- avm-transpiler/src/transpile.rs | 12 ------------ yarn-project/simulator/src/avm/avm_machine_state.ts | 11 ++++++++--- yarn-project/simulator/src/avm/errors.ts | 3 ++- .../simulator/src/avm/opcodes/control_flow.ts | 11 +++++++---- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 4a8934601b8..b02a917d874 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -1528,18 +1528,6 @@ pub fn patch_debug_info_pcs( debug_infos } -/// Patch the assert messages with updated PCs since transpilation injects extra -/// opcodes into the bytecode. -pub fn patch_assert_message_pcs( - assert_messages: HashMap, - brillig_pcs_to_avm_pcs: &[usize], -) -> HashMap { - assert_messages - .into_iter() - .map(|(brillig_pc, message)| (brillig_pcs_to_avm_pcs[brillig_pc], message)) - .collect() -} - fn tag_from_bit_size(bit_size: BitSize) -> AvmTypeTag { match bit_size { BitSize::Integer(IntegerBitSize::U1) => AvmTypeTag::UINT1, diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index 202e4ff23c3..3c1aac84528 100644 --- a/yarn-project/simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/simulator/src/avm/avm_machine_state.ts @@ -12,6 +12,11 @@ export type InitialAvmMachineState = { daGasLeft: number; }; +type CallStackEntry = { + callPc: number; + returnPc: number; +}; + /** * Avm state modified on an instruction-per-instruction basis. */ @@ -27,10 +32,10 @@ export class AvmMachineState { public nestedReturndata: Fr[] = []; /** - * On INTERNALCALL, internal call stack is pushed to with the current pc + 1 - * On INTERNALRETURN, value is popped from the internal call stack and assigned to the pc. + * On INTERNALCALL, internal call stack is pushed to with the current pc and the return pc. + * On INTERNALRETURN, value is popped from the internal call stack and assigned to the return pc. */ - public internalCallStack: number[] = []; + public internalCallStack: CallStackEntry[] = []; /** Memory accessible to user code */ public readonly memory: TaggedMemory = new TaggedMemory(); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 9a311440f80..444d95ce28b 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -113,7 +113,8 @@ function createRevertReason(message: string, context: AvmContext, nestedError?: // 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; - const internalCallStack = context.machineState.internalCallStack; + // 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]); } diff --git a/yarn-project/simulator/src/avm/opcodes/control_flow.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.ts index 9bb3320f982..f4dd23246c1 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.ts @@ -80,7 +80,10 @@ export class InternalCall extends Instruction { public async execute(context: AvmContext): Promise { context.machineState.consumeGas(this.gasCost()); - context.machineState.internalCallStack.push(context.machineState.pc); + context.machineState.internalCallStack.push({ + callPc: context.machineState.pc, + returnPc: context.machineState.nextPc, + }); context.machineState.pc = this.loc; context.machineState.memory.assert({}); @@ -104,11 +107,11 @@ export class InternalReturn extends Instruction { public async execute(context: AvmContext): Promise { context.machineState.consumeGas(this.gasCost()); - const jumpOffset = context.machineState.internalCallStack.pop(); - if (jumpOffset === undefined) { + const stackEntry = context.machineState.internalCallStack.pop(); + if (stackEntry === undefined) { throw new InstructionExecutionError('Internal call stack empty!'); } - context.machineState.pc = jumpOffset + 1; + context.machineState.pc = stackEntry.returnPc; context.machineState.memory.assert({}); } From 741c48882411ee23a448f7cf10e31111ef503728 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Mon, 4 Nov 2024 12:02:57 +0000 Subject: [PATCH 11/13] Bugfix for bulk test --- .../cpp/src/barretenberg/vm/avm/trace/deserialization.cpp | 4 ---- .../cpp/src/barretenberg/vm/avm/trace/execution.cpp | 2 +- .../protocol-contracts/src/protocol_contract_data.ts | 8 ++++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index 55b47920127..575ea0ee193 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -229,9 +229,6 @@ Instruction Deserialization::parse(std::vector const& bytecode, size_t { const auto length = bytecode.size(); - debug("------- PARSING BYTECODE -------"); - debug("Parse bytecode of length: " + std::to_string(length), " at position: ", pos); - if (pos >= length) { throw_or_abort("Position is out of range. Position: " + std::to_string(pos) + " Bytecode length: " + std::to_string(length)); @@ -314,7 +311,6 @@ Instruction Deserialization::parse(std::vector const& bytecode, size_t } auto instruction = Instruction(opcode, operands); - debug(instruction.to_string()); return instruction; }; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index c04378bbf2a..7d9a026d1a2 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -721,7 +721,7 @@ std::vector Execution::gen_trace(std::vector const& calldata, case OpCode::DEBUGLOG: // We want a noop, but we need to execute something that both advances the PC, // and adds a valid row to the trace. - trace_builder.op_jump(pc + 1); + trace_builder.op_jump(pc + static_cast(Deserialization::get_pc_increment(OpCode::DEBUGLOG))); break; // Gadgets diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index 1978867c801..db770ce0da4 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x1b6d9c59d70879bf40b7162dc09636b98b79c71de6da4f6220c3ce68299b32ff'), + AuthRegistry: Fr.fromString('0x04d70cb3d8222ae04cfa59e8bfed4f804832aaaef4f485d1debb004d1b9d6362'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x24f4432454069413e1fd66afb8948401568606bf58aeb177faeb6fa1986d594e'), - Router: Fr.fromString('0x04a71f51bcba229640326146613b63e0f18d5bc95825a6ccea1273825c49b973'), + FeeJuice: Fr.fromString('0x1067e9dc15d3046b6d21aaa8eafcfec88216217242cee3f9d722165ffc03c767'), + Router: Fr.fromString('0x16ab75e4efc0964c0ee3d715ac645d7972b722bfe60eea730a60b527c0681973'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x25472984a7cd9688418bae074f9443884f00e467ea7ac5d458f3aca5e73f6d22', + '0x2673f1d0618d2c98ccb3a11282073002f73335c4791eac16f67bf522e24151d1', ); From d23ac42d7796db8a1d859badd3b5684e71a25424 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Mon, 4 Nov 2024 12:46:11 +0000 Subject: [PATCH 12/13] Little improvements on style and comments --- .../vm/avm/trace/deserialization.cpp | 14 ++++++------- .../vm/avm/trace/deserialization.hpp | 4 ++-- .../barretenberg/vm/avm/trace/execution.cpp | 2 +- .../src/barretenberg/vm/avm/trace/trace.cpp | 20 +++++++++++++++---- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index 575ea0ee193..c9be4a6fa5c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -198,7 +198,7 @@ const std::unordered_map OPERAND_TYPE_SIZE = { // TODO: once opcodes are frozen, this function can be replaced by a table/map of constants size_t Deserialization::get_pc_increment(OpCode opcode) { - auto const iter = OPCODE_WIRE_FORMAT.find(opcode); + const auto iter = OPCODE_WIRE_FORMAT.find(opcode); if (iter == OPCODE_WIRE_FORMAT.end()) { return 0; @@ -207,7 +207,7 @@ size_t Deserialization::get_pc_increment(OpCode opcode) // OPCODE_WIRE_FORMAT does not contain the opcode itself which accounts for 1 byte size_t increment = 1; - std::vector inst_format = iter->second; + const std::vector& inst_format = iter->second; for (const auto& op_type : inst_format) { increment += OPERAND_TYPE_SIZE.at(op_type); } @@ -225,7 +225,7 @@ size_t Deserialization::get_pc_increment(OpCode opcode) * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range * @return The instruction */ -Instruction Deserialization::parse(std::vector const& bytecode, size_t pos) +Instruction Deserialization::parse(const std::vector& bytecode, size_t pos) { const auto length = bytecode.size(); @@ -241,12 +241,12 @@ Instruction Deserialization::parse(std::vector const& bytecode, size_t } pos++; - auto const opcode = static_cast(opcode_byte); - auto const iter = OPCODE_WIRE_FORMAT.find(opcode); + const auto opcode = static_cast(opcode_byte); + const auto iter = OPCODE_WIRE_FORMAT.find(opcode); if (iter == OPCODE_WIRE_FORMAT.end()) { throw_or_abort("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); } - std::vector inst_format = iter->second; + const std::vector& inst_format = iter->second; std::vector operands; for (OperandType const& op_type : inst_format) { @@ -323,7 +323,7 @@ Instruction Deserialization::parse(std::vector const& bytecode, size_t * @throws runtime_error exception when the bytecode is invalid or pos is out-of-range * @return The list of instructions as a vector */ -std::vector Deserialization::parse_bytecode_statically(std::vector const& bytecode) +std::vector Deserialization::parse_bytecode_statically(const std::vector& bytecode) { uint32_t pc = 0; std::vector instructions; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp index 96ab9fad7a5..bfdf7446193 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp @@ -18,8 +18,8 @@ class Deserialization { public: Deserialization() = default; - static Instruction parse(std::vector const& bytecode, size_t pos); - static std::vector parse_bytecode_statically(std::vector const& bytecode); + static Instruction parse(const std::vector& bytecode, size_t pos); + static std::vector parse_bytecode_statically(const std::vector& bytecode); static size_t get_pc_increment(OpCode opcode); }; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 7d9a026d1a2..0e9aeca964c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -280,7 +280,7 @@ std::vector Execution::gen_trace(std::vector const& calldata, Execution::trace_builder_constructor(public_inputs, execution_hints, start_side_effect_counter, calldata); // We should use the public input address, but for now we just take the first element in the list - std::vector bytecode = execution_hints.all_contract_bytecode.at(0).bytecode; + const std::vector& bytecode = execution_hints.all_contract_bytecode.at(0).bytecode; // Copied version of pc maintained in trace builder. The value of pc is evolving based // on opcode logic and therefore is not maintained here. However, the next opcode in the execution diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index 05f3be299c9..ac046d4aa46 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -1681,7 +1681,8 @@ void AvmTraceBuilder::op_returndata_copy(uint8_t indirect, pc += Deserialization::get_pc_increment(OpCode::RETURNDATACOPY); - // Crucial to perform this operation after having incremented pc + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). write_slice_to_memory(dst_offset_resolved, AvmMemoryTag::FF, returndata_slice); } @@ -2619,6 +2620,8 @@ void AvmTraceBuilder::op_get_contract_instance( pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). // TODO(8603): once instructions can have multiple different tags for writes, remove this and do a constrained // writes write_to_memory(resolved_dst_offset, member_value, AvmMemoryTag::FF); @@ -2804,6 +2807,8 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, pc += Deserialization::get_pc_increment(opcode); + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). // Write the success flag to memory write_to_memory(resolved_success_offset, hint.success, AvmMemoryTag::U1); external_call_counter++; @@ -3193,7 +3198,8 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect, pc += Deserialization::get_pc_increment(OpCode::SHA256COMPRESSION); - // Crucial to perform this operation after having incremented pc + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U32, ff_result); } @@ -3251,7 +3257,8 @@ void AvmTraceBuilder::op_keccakf1600(uint8_t indirect, uint32_t output_offset, u pc += Deserialization::get_pc_increment(OpCode::KECCAKF1600); - // Crucial to perform this operation after having incremented pc + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). write_slice_to_memory(resolved_output_offset, AvmMemoryTag::U64, result); } @@ -3311,6 +3318,8 @@ void AvmTraceBuilder::op_ec_add(uint16_t indirect, pc += Deserialization::get_pc_increment(OpCode::ECADD); + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). // Write point coordinates write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); @@ -3396,6 +3405,8 @@ void AvmTraceBuilder::op_variable_msm(uint8_t indirect, pc += Deserialization::get_pc_increment(OpCode::MSM); + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). // Write the result back to memory [x, y, inf] with tags [FF, FF, U8] write_to_memory(resolved_output_offset, result.x, AvmMemoryTag::FF); write_to_memory(resolved_output_offset + 1, result.y, AvmMemoryTag::FF); @@ -3490,7 +3501,8 @@ void AvmTraceBuilder::op_to_radix_be(uint8_t indirect, pc += Deserialization::get_pc_increment(OpCode::TORADIXBE); - // Crucial to perform this operation after having incremented pc + // Crucial to perform this operation after having incremented pc because write_slice_to_memory + // is implemented with opcodes (SET and JUMP). write_slice_to_memory(resolved_dst_offset, w_in_tag, res); } From 08732e38a6b243634af9ac96e90526ef72b09cd2 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Mon, 4 Nov 2024 13:07:13 +0000 Subject: [PATCH 13/13] Fixing gcc compilation failure --- .../cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp | 4 ++-- .../cpp/src/barretenberg/vm/avm/trace/deserialization.cpp | 6 +++--- .../cpp/src/barretenberg/vm/avm/trace/deserialization.hpp | 2 +- .../cpp/src/barretenberg/vm/avm/trace/execution.cpp | 2 +- barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp | 3 +-- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp index 312fc74519f..7238a75f52e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp @@ -130,7 +130,7 @@ TEST_F(AvmControlFlowTests, simpleJump) TEST_F(AvmControlFlowTests, simpleCallAndReturn) { uint32_t const CALL_PC = 20; - uint32_t const RETURN_PC = static_cast(Deserialization::get_pc_increment(OpCode::INTERNALCALL)); + uint32_t const RETURN_PC = Deserialization::get_pc_increment(OpCode::INTERNALCALL); // trace_builder for the following operation // pc opcode // 0 INTERNAL_CALL(pc=20) @@ -183,7 +183,7 @@ TEST_F(AvmControlFlowTests, multipleCallsAndReturns) const uint32_t JUMP_PC_1 = 22; - const uint32_t INTERNALCALL_SIZE = static_cast(Deserialization::get_pc_increment(OpCode::INTERNALCALL)); + const uint32_t INTERNALCALL_SIZE = Deserialization::get_pc_increment(OpCode::INTERNALCALL); const uint32_t NEXT_PC_1 = INTERNALCALL_SIZE; const uint32_t NEXT_PC_2 = CALL_PC_1 + INTERNALCALL_SIZE; const uint32_t NEXT_PC_3 = CALL_PC_2 + INTERNALCALL_SIZE; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp index c9be4a6fa5c..75c9d023f0d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp @@ -187,7 +187,7 @@ const std::unordered_map> OPCODE_WIRE_FORMAT = OperandType::UINT8 } }, }; -const std::unordered_map OPERAND_TYPE_SIZE = { +const std::unordered_map OPERAND_TYPE_SIZE = { { OperandType::INDIRECT8, 1 }, { OperandType::INDIRECT16, 2 }, { OperandType::TAG, 1 }, { OperandType::UINT8, 1 }, { OperandType::UINT16, 2 }, { OperandType::UINT32, 4 }, { OperandType::UINT64, 8 }, { OperandType::UINT128, 16 }, { OperandType::FF, 32 } @@ -196,7 +196,7 @@ const std::unordered_map OPERAND_TYPE_SIZE = { } // Anonymous namespace // TODO: once opcodes are frozen, this function can be replaced by a table/map of constants -size_t Deserialization::get_pc_increment(OpCode opcode) +uint32_t Deserialization::get_pc_increment(OpCode opcode) { const auto iter = OPCODE_WIRE_FORMAT.find(opcode); @@ -205,7 +205,7 @@ size_t Deserialization::get_pc_increment(OpCode opcode) } // OPCODE_WIRE_FORMAT does not contain the opcode itself which accounts for 1 byte - size_t increment = 1; + uint32_t increment = 1; const std::vector& inst_format = iter->second; for (const auto& op_type : inst_format) { diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp index bfdf7446193..eb58ddc803d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.hpp @@ -20,7 +20,7 @@ class Deserialization { static Instruction parse(const std::vector& bytecode, size_t pos); static std::vector parse_bytecode_statically(const std::vector& bytecode); - static size_t get_pc_increment(OpCode opcode); + static uint32_t get_pc_increment(OpCode opcode); }; } // namespace bb::avm_trace diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 0e9aeca964c..4d15050345c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -721,7 +721,7 @@ std::vector Execution::gen_trace(std::vector const& calldata, case OpCode::DEBUGLOG: // We want a noop, but we need to execute something that both advances the PC, // and adds a valid row to the trace. - trace_builder.op_jump(pc + static_cast(Deserialization::get_pc_increment(OpCode::DEBUGLOG))); + trace_builder.op_jump(pc + Deserialization::get_pc_increment(OpCode::DEBUGLOG)); break; // Gadgets diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index ac046d4aa46..650a7fe6f54 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -1803,8 +1803,7 @@ void AvmTraceBuilder::op_jumpi(uint8_t indirect, uint32_t jmp_dest, uint32_t con const bool id_zero = read_d.val == 0; FF const inv = !id_zero ? read_d.val.invert() : 1; - uint32_t next_pc = - !id_zero ? jmp_dest : pc + static_cast(Deserialization::get_pc_increment(OpCode::JUMPI_32)); + uint32_t next_pc = !id_zero ? jmp_dest : pc + Deserialization::get_pc_increment(OpCode::JUMPI_32); // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::JUMPI_32);