From 5abbb4ca2a541667c28910d624c7211a602b243c Mon Sep 17 00:00:00 2001 From: mm-zk Date: Fri, 24 May 2024 10:22:24 +0200 Subject: [PATCH 1/2] mirror 4659a6e2dfe72b6d37e895e9cad7e2564d4ea4dc --- src/vm_state/cycle.rs | 125 +++++++++++++++++++++++++--------------- src/vm_state/helpers.rs | 1 + 2 files changed, 81 insertions(+), 45 deletions(-) diff --git a/src/vm_state/cycle.rs b/src/vm_state/cycle.rs index db35cc2..e7cee2d 100644 --- a/src/vm_state/cycle.rs +++ b/src/vm_state/cycle.rs @@ -17,6 +17,49 @@ pub struct PreState = EncodingModeProdu pub const OPCODES_PER_WORD_LOG_2: usize = 2; pub const OPCODES_PER_WORD: usize = 1 << OPCODES_PER_WORD_LOG_2; +fn read_and_cache_opcode< + const N: usize, + E: VmEncodingMode, + M: zk_evm_abstractions::vm::Memory, + WT: crate::witness_trace::VmWitnessTracer, +>( + local_state: &VmLocalState, + memory: &M, + witness_tracer: &mut WT, + super_pc: >::PcOrImm, + delayed_changes: &mut DelayedLocalStateChanges, +) -> U256 { + // we need to read the code word and select a proper subword + use zk_evm_abstractions::auxiliary::*; + use zk_evm_abstractions::vm::*; + + let code_page = local_state.callstack.get_current_stack().code_page; + let location = MemoryLocation { + memory_type: MemoryType::Code, + page: code_page, + index: MemoryIndex(super_pc.as_u64() as u32), + }; + let key = MemoryKey { + timestamp: local_state.timestamp_for_code_or_src_read(), + location, + }; + + assert!(delayed_changes.new_previous_code_memory_page.is_some()); + + // code read is never pending + let code_query = read_code( + memory, + witness_tracer, + local_state.monotonic_cycle_counter, + key, + ); + let u256_word = code_query.value; + delayed_changes.new_previous_code_word = Some(u256_word); + delayed_changes.new_previous_super_pc = Some(super_pc); + + u256_word +} + pub fn read_and_decode< const N: usize, E: VmEncodingMode, @@ -55,50 +98,35 @@ pub fn read_and_decode< != local_state.previous_code_memory_page; let (super_pc, sub_pc) = E::split_pc(pc); + let refresh_opcode_cache = match (code_pages_are_different, previous_super_pc == super_pc) { + (true, _) | (false, false) => true, + _ => false, + }; + // if we do not skip cycle then we read memory for a new opcode let opcode_encoding = if execution_has_ended == false && pending_exception == false { - let raw_opcode_u64 = match (code_pages_are_different, previous_super_pc == super_pc) { - (true, _) | (false, false) => { - // we need to read the code word and select a proper subword - let code_page = local_state.callstack.get_current_stack().code_page; - let location = MemoryLocation { - memory_type: MemoryType::Code, - page: code_page, - index: MemoryIndex(super_pc.as_u64() as u32), - }; - let key = MemoryKey { - timestamp: local_state.timestamp_for_code_or_src_read(), - location, - }; - - delayed_changes.new_previous_code_memory_page = Some(code_page); - - // code read is never pending - let code_query = read_code( - memory, - witness_tracer, - local_state.monotonic_cycle_counter, - key, - ); - let u256_word = code_query.value; - delayed_changes.new_previous_code_word = Some(u256_word); - delayed_changes.new_previous_super_pc = Some(super_pc); - - // our memory is a set of words in storage, and those are only re-interpreted - // as bytes in UMA. But natural storage for code is still bytes. - - // to ensure consistency with the future if we allow deployment of raw bytecode - // then for our BE machine we should consider that "first" bytes, that will be - // our integer's "highest" bytes, so to follow bytearray-like enumeration we - // have to use inverse order here - let u256_word = u256_word; - E::integer_representaiton_from_u256(u256_word, sub_pc) - } - (false, true) => { - // use a saved one - let u256_word = local_state.previous_code_word; - E::integer_representaiton_from_u256(u256_word, sub_pc) - } + let raw_opcode_u64 = if refresh_opcode_cache { + // we need to read the code word and select a proper subword + let u256_word = read_and_cache_opcode( + local_state, + memory, + witness_tracer, + super_pc, + &mut delayed_changes, + ); + // our memory is a set of words in storage, and those are only re-interpreted + // as bytes in UMA. But natural storage for code is still bytes. + + // to ensure consistency with the future if we allow deployment of raw bytecode + // then for our BE machine we should consider that "first" bytes, that will be + // our integer's "highest" bytes, so to follow bytearray-like enumeration we + // have to use inverse order here + let u256_word = u256_word; + E::integer_representaiton_from_u256(u256_word, sub_pc) + } else { + // use a saved one + let u256_word = local_state.previous_code_word; + E::integer_representaiton_from_u256(u256_word, sub_pc) }; raw_opcode_u64 @@ -106,13 +134,20 @@ pub fn read_and_decode< // there are no cases that set pending exception and // simultaneously finish the execution assert!(execution_has_ended == false); + // if we have an exception then we should still read the word for cache, even if we will have PC later on updated + if refresh_opcode_cache { + let _ = read_and_cache_opcode( + local_state, + memory, + witness_tracer, + super_pc, + &mut delayed_changes, + ); + } // so we can just remove the marker as soon as we are no longer pending delayed_changes.new_pending_exception = Some(false); - // anyway update super PC - delayed_changes.new_previous_super_pc = Some(super_pc); - E::exception_revert_encoding() } else { // we are skipping cycle for some reason, so we do nothing, diff --git a/src/vm_state/helpers.rs b/src/vm_state/helpers.rs index 865fb00..4a16644 100644 --- a/src/vm_state/helpers.rs +++ b/src/vm_state/helpers.rs @@ -392,6 +392,7 @@ impl< (self.local_state.pubdata_revert_counter.0, false) }; assert!(of == false); + assert!(new_revert_counter >= 0); // global counter can not be < 0, only local one can self.local_state.pubdata_revert_counter = PubdataCost(new_revert_counter); old_frame From 92e500e5e3e0a39448906c5b32e0e9ba856fc60d Mon Sep 17 00:00:00 2001 From: mm-zk Date: Fri, 24 May 2024 10:25:53 +0200 Subject: [PATCH 2/2] warnings --- src/opcodes/execution/far_call.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/opcodes/execution/far_call.rs b/src/opcodes/execution/far_call.rs index ef10c45..a60a971 100644 --- a/src/opcodes/execution/far_call.rs +++ b/src/opcodes/execution/far_call.rs @@ -1,8 +1,6 @@ use super::*; use crate::zkevm_opcode_defs::definitions::far_call::*; -use crate::zkevm_opcode_defs::system_params::DEPLOYER_SYSTEM_CONTRACT_ADDRESS; -use crate::zkevm_opcode_defs::system_params::STORAGE_AUX_BYTE; use crate::zkevm_opcode_defs::INITIAL_SP_ON_FAR_CALL; use zk_evm_abstractions::aux::*;