From 29724f3f2db5a3b6e399e54a9a84af8686807fb6 Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 4 Nov 2024 15:39:43 +0000 Subject: [PATCH] feat(avm)!: byte indexed PC (#9582) This PR moves the AVM to use byte-indexed PCs * Modifies the transpiler to remap brillig PCs * Modifies the simulator to use byte indexed PCs * Modifies witgen and circuit to use byte indexed PCs Why are we doing this? * Needed for bytecode decomposition in the circuit. * Allow storing other stuff besides code in a contract, and then be able to use it in memory with an opcode "CODECOPY" or similar. --- A note on how PCs are mapped in the transpiler: we do 2 passes. First we translate all instructions and leave brillig location operands as `BRILLIG_LOCATION`. On a second pass, since now we know the structure of the program and the brillig=>AVM pcs, we replace those. There are a few big caveats 1. ~Since the JUMP(I) and INTERNALCALL operands are U16, we cannot jump or call a location bigger than 2^16. This effectively constrains the contract size to 65kB.~ We use 32 bit jumps now. 2. We can do the transformation in (only) 2 passes because we only have 1 variant of JUMP etc. Suppose we had an 8 bit variant, or a 32 bit variant, then we wouldn't know which one to use until the original PC has been mapped, but that itself can change the size of the instructions and trigger a remapping! Solutions? * For (1) I might propose having relative jumps JUMP(I)R with 8 and 16 bit variants, and an absolute JUMP with 32 bits. * For (2) we might just need to remap until there is no change. Part of #9059. --- avm-transpiler/src/instructions.rs | 11 + avm-transpiler/src/transpile.rs | 98 +++--- avm-transpiler/src/transpile_contract.rs | 8 +- avm-transpiler/src/utils.rs | 4 +- 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 | 283 ++++++++++-------- .../vm/avm/trace/deserialization.cpp | 209 +++++++------ .../vm/avm/trace/deserialization.hpp | 5 +- .../barretenberg/vm/avm/trace/execution.cpp | 128 +++++--- .../src/barretenberg/vm/avm/trace/trace.cpp | 260 +++++++++++----- .../src/barretenberg/vm/avm/trace/trace.hpp | 56 ++-- .../src/protocol_contract_data.ts | 8 +- .../simulator/src/avm/avm_machine_state.ts | 22 +- .../simulator/src/avm/avm_simulator.test.ts | 8 +- .../simulator/src/avm/avm_simulator.ts | 23 +- yarn-project/simulator/src/avm/errors.ts | 3 +- .../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 - .../src/avm/opcodes/control_flow.test.ts | 43 +-- .../simulator/src/avm/opcodes/control_flow.ts | 29 +- .../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 | 11 +- .../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 - .../serialization/bytecode_serialization.ts | 53 ++-- 36 files changed, 838 insertions(+), 623 deletions(-) diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs index aaadc278ffa..8289f444ac3 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: u32 }, } 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 d1442196a80..b02a917d874 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,22 @@ pub fn brillig_to_avm( }); } BrilligOpcode::Jump { location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 32); 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 }], ..Default::default() }); } BrilligOpcode::JumpIf { condition, location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 32); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::JUMPI_32, indirect: Some( AddressingModeBuilder::default().direct_operand(condition).build(), ), operands: vec![ - make_operand(32, &avm_loc), + AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }, make_operand(16, &condition.to_usize()), ], ..Default::default() @@ -295,10 +297,10 @@ pub fn brillig_to_avm( )); } BrilligOpcode::Call { location } => { - let avm_loc = brillig_pcs_to_avm_pcs[*location]; + assert!(location.num_bits() <= 32); avm_instrs.push(AvmInstruction { opcode: AvmOpcode::INTERNALCALL, - operands: vec![AvmOperand::U32 { value: avm_loc as u32 }], + operands: vec![AvmOperand::BRILLIG_LOCATION { brillig_pc: *location as u32 }], ..Default::default() }); } @@ -342,8 +344,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_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() <= 32, "Oops! AVM PC is too large!"); + AvmOperand::U32 { value: avm_pc as u32 } + } + _ => 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 +393,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 @@ -1496,48 +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() -} - -/// 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 0c38dbe1e0b..6bb30a3adc2 100644 --- a/avm-transpiler/src/transpile_contract.rs +++ b/avm-transpiler/src/transpile_contract.rs @@ -6,7 +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_debug_info_pcs}; +use crate::transpile::{brillig_to_avm, patch_debug_info_pcs}; use crate::utils::extract_brillig_from_acir_program; /// Representation of a contract with some transpiled functions @@ -88,17 +88,15 @@ 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; let brillig_bytecode = extract_brillig_from_acir_program(&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); - // 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", diff --git a/avm-transpiler/src/utils.rs b/avm-transpiler/src/utils.rs index 018276e4b6d..ba78ff14808 100644 --- a/avm-transpiler/src/utils.rs +++ b/avm-transpiler/src/utils.rs @@ -50,9 +50,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/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..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 @@ -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 = 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 = 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 4640e94585d..ba245cbcfc0 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); @@ -313,17 +317,18 @@ 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 - "00000004" // jmp_dest + "0000001C" // jmp_dest 28 + to_hex(OpCode::ADD_16) + // opcode ADD "00" // Indirect flag "0004" // addr a 4 @@ -342,7 +347,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,15 +356,15 @@ 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 }; + 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)); @@ -380,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) { @@ -410,14 +416,14 @@ 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); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(12)); @@ -431,48 +437,49 @@ 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 }; + 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 @@ -490,7 +497,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)); @@ -508,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. @@ -537,23 +543,24 @@ 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. -// 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) @@ -563,31 +570,31 @@ 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); - auto instructions = Deserialization::parse(bytecode); + auto instructions = Deserialization::parse_bytecode_statically(bytecode); ASSERT_THAT(instructions, SizeIs(8)); @@ -598,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; @@ -606,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)); } @@ -641,7 +648,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 +668,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 +706,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 +716,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 +745,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 +758,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 +813,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 +882,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 +954,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 +1016,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 +1089,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 +1157,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 +1246,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 +1312,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 +1521,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 +1555,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 +1567,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 +1599,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 +1611,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 +1638,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 +1658,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 +1698,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)); @@ -1719,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); @@ -1732,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); @@ -1742,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); @@ -1750,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); @@ -1788,7 +1802,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 +1863,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 +1920,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)); @@ -1956,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 @@ -1988,7 +2001,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)); @@ -2005,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 @@ -2019,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); @@ -2034,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); @@ -2125,7 +2144,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 +2218,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 +2253,8 @@ 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); + ASSERT_THAT(instructions, SizeIs(2)); std::vector calldata; std::vector returndata; @@ -2244,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); @@ -2262,7 +2283,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 +2300,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..75c9d023f0d 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 @@ -186,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 } @@ -194,104 +195,144 @@ 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 +uint32_t Deserialization::get_pc_increment(OpCode opcode) +{ + const auto 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 + uint32_t increment = 1; + + const 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(const std::vector& 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); + 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++; + if (!Bytecode::is_valid(opcode_byte)) { + throw_or_abort("Invalid opcode byte: " + to_hex(opcode_byte) + " at position: " + std::to_string(pos)); + } + pos++; + + 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)); + } + const 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; - } - 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; + 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::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; + } + 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; } - auto instruction = Instruction(opcode, operands); - debug(instruction.to_string()); - instructions.emplace_back(std::move(instruction)); + 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); + 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(const std::vector& 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..eb58ddc803d 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(const std::vector& bytecode, size_t pos); + static std::vector parse_bytecode_statically(const std::vector& bytecode); + 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 cc03e9375fb..4d15050345c 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 + 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 // 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 @@ -685,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 + 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 6eea5900967..650a7fe6f54 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" @@ -210,9 +211,9 @@ 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, 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)), }); @@ -1616,6 +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); + + pc += Deserialization::get_pc_increment(OpCode::RETURNDATACOPY); + + // 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); } @@ -1655,7 +1722,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), @@ -1736,7 +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 + 1; + 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); @@ -1778,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); @@ -1795,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), @@ -1869,7 +1936,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 +1958,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 +1966,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 +1981,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 +2009,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 +2019,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 +2052,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 +2099,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 +2149,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 +2188,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 +2290,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 +2297,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 +2305,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 +2332,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 +2370,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 +2382,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 +2405,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, @@ -2366,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) @@ -2383,6 +2455,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::EMITNOTEHASH); } void AvmTraceBuilder::op_nullifier_exists(uint8_t indirect, @@ -2405,6 +2479,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 +2498,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 +2529,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,11 +2548,11 @@ 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); - + pc += Deserialization::get_pc_increment(OpCode::GETCONTRACTINSTANCE); } else { ContractInstanceMember chosen_member = static_cast(member_enum); @@ -2525,7 +2605,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), @@ -2537,6 +2617,10 @@ void AvmTraceBuilder::op_get_contract_instance( .main_tag_err = FF(static_cast(!tag_match)), }); + 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); @@ -2611,7 +2695,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 +2707,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 +2727,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 +2790,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), @@ -2717,6 +2804,10 @@ void AvmTraceBuilder::constrain_external_call(OpCode opcode, .main_tag_err = FF(static_cast(!tag_match)), }); + 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++; @@ -2933,7 +3024,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 +3104,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 +3159,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), @@ -3102,7 +3195,10 @@ void AvmTraceBuilder::op_sha256_compression(uint8_t indirect, ff_result.emplace_back(result[i]); } - // Write the result to memory after + pc += Deserialization::get_pc_increment(OpCode::SHA256COMPRESSION); + + // 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); } @@ -3137,7 +3233,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), @@ -3157,7 +3253,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 + + pc += Deserialization::get_pc_increment(OpCode::KECCAKF1600); + + // 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); } @@ -3208,13 +3308,17 @@ 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), }); gas_trace_builder.constrain_gas(clk, OpCode::ECADD); + 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); @@ -3289,7 +3393,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), }); @@ -3298,6 +3402,10 @@ 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); + + // 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); @@ -3378,7 +3486,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 @@ -3390,6 +3498,10 @@ void AvmTraceBuilder::op_to_radix_be(uint8_t indirect, .main_w_in_tag = FF(static_cast(w_in_tag)), }); + pc += Deserialization::get_pc_increment(OpCode::TORADIXBE); + + // 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); } 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/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index b3643734e90..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('0x28a7d6b69b161a277995403f70e5ed4d4bb27fccfe06277f0ce260f0c34665b9'), + AuthRegistry: Fr.fromString('0x04d70cb3d8222ae04cfa59e8bfed4f804832aaaef4f485d1debb004d1b9d6362'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x094caa4d1259aa99ea4a8a97a9fa8cbae29ecc8ed818b0f0516cf9fd753b97e6'), - Router: Fr.fromString('0x215265750d313695cc669740b19f926207cdcf2c8ee69a8504921b361b0dda08'), + FeeJuice: Fr.fromString('0x1067e9dc15d3046b6d21aaa8eafcfec88216217242cee3f9d722165ffc03c767'), + Router: Fr.fromString('0x16ab75e4efc0964c0ee3d715ac645d7972b722bfe60eea730a60b527c0681973'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x034740a11a8535c69a6e99b1ca126667c5eb1ea9c7f537d96bc9e51c67b18cfa', + '0x2673f1d0618d2c98ccb3a11282073002f73335c4791eac16f67bf522e24151d1', ); diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index a46f0028d63..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. */ @@ -19,16 +24,18 @@ 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; + /** program counter of the next instruction, byte based */ + public nextPc: number = 0; /** return/revertdata of the last nested call. */ 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(); @@ -91,13 +98,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.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 0bb1fabce16..70682ec440a 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -425,10 +425,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { 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(resultBuffer.equals(expectedResults)).toBe(true); + expect(results.output.map(f => f.toNumber().toString()).join('')).toEqual('0010101011'); }); describe('Side effects, world state, nested calls', () => { @@ -1084,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, calculated 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/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index de8478283ef..788a3908b2b 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -16,8 +16,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; @@ -71,24 +70,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,7 +97,12 @@ 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 + machineState.pc += bytesRead; + } // gas used by this instruction - used for profiling/tallying const gasUsed: Gas = { @@ -114,9 +111,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/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/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.test.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.test.ts index 9500c62ff37..d5cad41ed7f 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 () => { @@ -112,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/opcodes/control_flow.ts b/yarn-project/simulator/src/avm/opcodes/control_flow.ts index 815f5025612..f4dd23246c1 100644 --- a/yarn-project/simulator/src/avm/opcodes/control_flow.ts +++ b/yarn-project/simulator/src/avm/opcodes/control_flow.ts @@ -22,6 +22,10 @@ export class Jump extends Instruction { context.machineState.memory.assert({}); } + + public override handlesPC(): boolean { + return true; + } } export class JumpI extends Instruction { @@ -50,13 +54,17 @@ export class JumpI extends Instruction { const condition = memory.getAs(condOffset); if (condition.toBigInt() == 0n) { - context.machineState.incrementPc(); + context.machineState.pc = context.machineState.nextPc; } else { context.machineState.pc = this.loc; } memory.assert({ reads: 1, addressing }); } + + public override handlesPC(): boolean { + return true; + } } export class InternalCall extends Instruction { @@ -72,11 +80,18 @@ 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({}); } + + public override handlesPC(): boolean { + return true; + } } export class InternalReturn extends Instruction { @@ -92,12 +107,16 @@ 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({}); } + + 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 3a3aa66b6c8..d1200cdc491 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -111,7 +111,6 @@ abstract class ExternalCall extends Instruction { ); memory.assert({ reads: calldataSize + 4, writes: 1, addressing }); - context.machineState.incrementPc(); } public abstract override get type(): 'CALL' | 'STATICCALL'; @@ -163,6 +162,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 { @@ -201,6 +204,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. */ 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.ts b/yarn-project/simulator/src/avm/serialization/bytecode_serialization.ts index 88f9ef16de5..cebf4a73cfe 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,40 @@ 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 - */ +// For testing only export function decodeFromBytecode( bytecode: Buffer, 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`); - } - - const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; - const i: Instruction = instructionDeserializer(cursor); - instructions.push(i); + 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, + pc: number, + instructionSet: InstructionSet = INSTRUCTION_SET(), +): [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 instructionDeserializerOrUndef = instructionSet.get(opcode); + if (instructionDeserializerOrUndef === undefined) { + throw new Error(`Opcode ${Opcode[opcode]} (0x${opcode.toString(16)}) not implemented`); + } + + const instructionDeserializer: InstructionDeserializer = instructionDeserializerOrUndef; + const instruction = instructionDeserializer(cursor); + return [instruction, cursor.position() - startingPosition]; +}