From ff601386686cdae0ab4f227203a006816e7a49a5 Mon Sep 17 00:00:00 2001 From: Danil Date: Thu, 12 Oct 2023 20:32:46 +0200 Subject: [PATCH] feat(vm): Improve tracer trait (#121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What ❔ Changing the way, how tracers are working in VM. Try to make less calls and add more static dispatching Tracer Api changes: 1. Method before decoding now always empty (zk_evm don't call it) 2. Method before cycle doesn't exist anymore 3. Method after_cycle renamed to finish_cycle and unified with should stop execution 4. Method save_results was removed 5. All methods from ExecutionProcessing trait moved to VmTracer Vm Changes: Refund tracer now dispatching statically and not dynamically ## Why ❔ ## Checklist - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted via `zk fmt` and `zk lint`. --------- Signed-off-by: Danil --- .../system-constants-generator/src/utils.rs | 12 +- core/lib/vm/src/implementation/execution.rs | 40 +++--- core/lib/vm/src/lib.rs | 5 +- core/lib/vm/src/tests/rollbacks.rs | 29 ++-- core/lib/vm/src/tracers/call.rs | 15 ++- core/lib/vm/src/tracers/default_tracers.rs | 124 ++++++++++-------- core/lib/vm/src/tracers/refunds.rs | 39 +++--- core/lib/vm/src/tracers/result_tracer.rs | 15 +-- .../lib/vm/src/tracers/storage_invocations.rs | 34 ++--- core/lib/vm/src/tracers/traits.rs | 43 +++--- core/lib/vm/src/tracers/validation/mod.rs | 19 ++- 11 files changed, 175 insertions(+), 200 deletions(-) diff --git a/core/bin/system-constants-generator/src/utils.rs b/core/bin/system-constants-generator/src/utils.rs index d55a73d4e8f7..2fe29be662c8 100644 --- a/core/bin/system-constants-generator/src/utils.rs +++ b/core/bin/system-constants-generator/src/utils.rs @@ -3,9 +3,9 @@ use std::cell::RefCell; use std::rc::Rc; use vm::constants::{BLOCK_GAS_LIMIT, BOOTLOADER_HEAP_PAGE}; use vm::{ - BootloaderState, BoxedTracer, DynTracer, ExecutionEndTracer, ExecutionProcessing, - HistoryEnabled, HistoryMode, L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode, Vm, - VmExecutionMode, VmExecutionStopReason, VmTracer, ZkSyncVmState, + BootloaderState, BoxedTracer, DynTracer, HistoryEnabled, HistoryMode, L1BatchEnv, L2BlockEnv, + SystemEnv, TxExecutionMode, Vm, VmExecutionMode, VmExecutionStopReason, VmTracer, + ZkSyncVmState, }; use zksync_contracts::{ load_sys_contract, read_bootloader_code, read_sys_contract_bytecode, read_zbin_bytecode, @@ -33,9 +33,7 @@ struct SpecialBootloaderTracer { impl DynTracer for SpecialBootloaderTracer {} -impl ExecutionEndTracer for SpecialBootloaderTracer {} - -impl ExecutionProcessing for SpecialBootloaderTracer { +impl VmTracer for SpecialBootloaderTracer { fn initialize_tracer(&mut self, state: &mut ZkSyncVmState) { state.memory.populate_page( BOOTLOADER_HEAP_PAGE as usize, @@ -55,8 +53,6 @@ impl ExecutionProcessing for SpecialBootl } } -impl VmTracer for SpecialBootloaderTracer {} - pub static GAS_TEST_SYSTEM_CONTRACTS: Lazy = Lazy::new(|| { let bytecode = read_bootloader_code("gas_test"); let hash = hash_bytecode(&bytecode); diff --git a/core/lib/vm/src/implementation/execution.rs b/core/lib/vm/src/implementation/execution.rs index 52c4ff0cb0da..aed11123c7fc 100644 --- a/core/lib/vm/src/implementation/execution.rs +++ b/core/lib/vm/src/implementation/execution.rs @@ -6,9 +6,7 @@ use crate::old_vm::{ utils::{vm_may_have_ended_inner, VmExecutionResult}, }; use crate::tracers::{ - traits::{ - BoxedTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, VmTracer, - }, + traits::{TracerExecutionStatus, VmTracer}, DefaultExecutionTracer, RefundsTracer, }; use crate::types::{inputs::VmExecutionMode, outputs::VmExecutionResultAndLogs}; @@ -18,16 +16,17 @@ use crate::VmExecutionStopReason; impl Vm { pub(crate) fn inspect_inner( &mut self, - mut tracers: Vec>>, + tracers: Vec>>, execution_mode: VmExecutionMode, ) -> VmExecutionResultAndLogs { + let mut enable_refund_tracer = false; if let VmExecutionMode::OneTx = execution_mode { - // For correct results we have to include refunds tracer to the desired tracers - tracers.push(RefundsTracer::new(self.batch_env.clone()).into_boxed()); // Move the pointer to the next transaction self.bootloader_state.move_tx_to_execute_pointer(); + enable_refund_tracer = true; } - let (_, result) = self.inspect_and_collect_results(tracers, execution_mode); + let (_, result) = + self.inspect_and_collect_results(tracers, execution_mode, enable_refund_tracer); result } @@ -37,12 +36,16 @@ impl Vm { &mut self, tracers: Vec>>, execution_mode: VmExecutionMode, + with_refund_tracer: bool, ) -> (VmExecutionStopReason, VmExecutionResultAndLogs) { + let refund_tracers = + with_refund_tracer.then_some(RefundsTracer::new(self.batch_env.clone())); let mut tx_tracer: DefaultExecutionTracer = DefaultExecutionTracer::new( self.system_env.default_validation_computational_gas_limit, execution_mode, tracers, self.storage.clone(), + refund_tracers, ); let timestamp_initial = Timestamp(self.state.local_state.timestamp); @@ -68,16 +71,18 @@ impl Vm { let result = tx_tracer.result_tracer.into_result(); - let mut result = VmExecutionResultAndLogs { + let refunds = tx_tracer + .refund_tracer + .map(|x| x.get_refunds()) + .unwrap_or_default(); + + let result = VmExecutionResultAndLogs { result, logs, statistics, - refunds: Default::default(), + refunds, }; - for tracer in tx_tracer.custom_tracers.iter_mut() { - tracer.save_results(&mut result); - } (stop_reason, result) } @@ -96,19 +101,18 @@ impl Vm { self.state ); - tracer.before_cycle(&mut self.state); self.state .cycle(tracer) .expect("Failed execution VM cycle."); - tracer.after_cycle(&mut self.state, &mut self.bootloader_state); + if let TracerExecutionStatus::Stop(reason) = + tracer.finish_cycle(&mut self.state, &mut self.bootloader_state) + { + break VmExecutionStopReason::TracerRequestedStop(reason); + } if self.has_ended() { break VmExecutionStopReason::VmFinished; } - - if let TracerExecutionStatus::Stop(reason) = tracer.should_stop_execution() { - break VmExecutionStopReason::TracerRequestedStop(reason); - } }; tracer.after_vm_execution(&mut self.state, &self.bootloader_state, result.clone()); result diff --git a/core/lib/vm/src/lib.rs b/core/lib/vm/src/lib.rs index f0a9d7c03301..d77fbab2014e 100644 --- a/core/lib/vm/src/lib.rs +++ b/core/lib/vm/src/lib.rs @@ -16,10 +16,7 @@ pub use errors::{ pub use tracers::{ call::CallTracer, - traits::{ - BoxedTracer, DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, - TracerExecutionStopReason, VmTracer, - }, + traits::{BoxedTracer, DynTracer, TracerExecutionStatus, TracerExecutionStopReason, VmTracer}, utils::VmExecutionStopReason, StorageInvocations, ValidationError, ValidationTracer, ValidationTracerParams, }; diff --git a/core/lib/vm/src/tests/rollbacks.rs b/core/lib/vm/src/tests/rollbacks.rs index 9d6c48b86908..387e304e2856 100644 --- a/core/lib/vm/src/tests/rollbacks.rs +++ b/core/lib/vm/src/tests/rollbacks.rs @@ -12,9 +12,8 @@ use crate::tests::tester::{ use crate::tests::utils::read_test_contract; use crate::types::inputs::system_env::TxExecutionMode; use crate::{ - BootloaderState, DynTracer, ExecutionEndTracer, ExecutionProcessing, HistoryEnabled, - HistoryMode, TracerExecutionStatus, TracerExecutionStopReason, VmExecutionMode, VmTracer, - ZkSyncVmState, + BootloaderState, DynTracer, HistoryEnabled, HistoryMode, TracerExecutionStatus, + TracerExecutionStopReason, VmExecutionMode, VmTracer, ZkSyncVmState, }; #[test] @@ -153,39 +152,28 @@ fn test_vm_loadnext_rollbacks() { // Testing tracer that does not allow the recursion to go deeper than a certain limit struct MaxRecursionTracer { max_recursion_depth: usize, - should_stop_execution: bool, } /// Tracer responsible for calculating the number of storage invocations and /// stopping the VM execution if the limit is reached. impl DynTracer for MaxRecursionTracer {} -impl ExecutionEndTracer for MaxRecursionTracer { - fn should_stop_execution(&self) -> TracerExecutionStatus { - if self.should_stop_execution { - TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish) - } else { - TracerExecutionStatus::Continue - } - } -} - -impl ExecutionProcessing for MaxRecursionTracer { - fn after_cycle( +impl VmTracer for MaxRecursionTracer { + fn finish_cycle( &mut self, state: &mut ZkSyncVmState, _bootloader_state: &mut BootloaderState, - ) { + ) -> TracerExecutionStatus { let current_depth = state.local_state.callstack.depth(); if current_depth > self.max_recursion_depth { - self.should_stop_execution = true; + TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish) + } else { + TracerExecutionStatus::Continue } } } -impl VmTracer for MaxRecursionTracer {} - #[test] fn test_layered_rollback() { // This test checks that the layered rollbacks work correctly, i.e. @@ -236,7 +224,6 @@ fn test_layered_rollback() { vm.vm.inspect( vec![Box::new(MaxRecursionTracer { max_recursion_depth: 15, - should_stop_execution: false, })], VmExecutionMode::OneTx, ); diff --git a/core/lib/vm/src/tracers/call.rs b/core/lib/vm/src/tracers/call.rs index aa2041f0bea9..bd008b7187d0 100644 --- a/core/lib/vm/src/tracers/call.rs +++ b/core/lib/vm/src/tracers/call.rs @@ -16,8 +16,8 @@ use zksync_types::U256; use crate::errors::VmRevertReason; use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; -use crate::tracers::traits::{DynTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}; -use crate::types::outputs::VmExecutionResultAndLogs; +use crate::tracers::traits::{DynTracer, VmTracer}; +use crate::{BootloaderState, VmExecutionStopReason, ZkSyncVmState}; #[derive(Debug, Clone)] pub struct CallTracer { @@ -88,12 +88,13 @@ impl DynTracer for CallTracer { } } -impl ExecutionEndTracer for CallTracer {} - -impl ExecutionProcessing for CallTracer {} - impl VmTracer for CallTracer { - fn save_results(&mut self, _result: &mut VmExecutionResultAndLogs) { + fn after_vm_execution( + &mut self, + _state: &mut ZkSyncVmState, + _bootloader_state: &BootloaderState, + _stop_reason: VmExecutionStopReason, + ) { self.result .set( std::mem::take(&mut self.stack) diff --git a/core/lib/vm/src/tracers/default_tracers.rs b/core/lib/vm/src/tracers/default_tracers.rs index 4df00193265d..1278990964a5 100644 --- a/core/lib/vm/src/tracers/default_tracers.rs +++ b/core/lib/vm/src/tracers/default_tracers.rs @@ -1,12 +1,12 @@ use std::fmt::{Debug, Formatter}; -use zk_evm::witness_trace::DummyTracer; -use zk_evm::zkevm_opcode_defs::{Opcode, RetOpcode}; use zk_evm::{ tracing::{ AfterDecodingData, AfterExecutionData, BeforeExecutionData, Tracer, VmLocalStateData, }, vm_state::VmLocalState, + witness_trace::DummyTracer, + zkevm_opcode_defs::{decoding::EncodingModeProduction, Opcode, RetOpcode}, }; use zksync_state::{StoragePtr, WriteStorage}; use zksync_types::Timestamp; @@ -17,14 +17,13 @@ use crate::constants::BOOTLOADER_HEAP_PAGE; use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; use crate::tracers::traits::{ - DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, - TracerExecutionStopReason, VmTracer, + DynTracer, TracerExecutionStatus, TracerExecutionStopReason, VmTracer, }; use crate::tracers::utils::{ computational_gas_price, gas_spent_on_bytecodes_and_long_messages_this_opcode, print_debug_if_needed, VmHook, }; -use crate::tracers::ResultTracer; +use crate::tracers::{RefundsTracer, ResultTracer}; use crate::types::internals::ZkSyncVmState; use crate::{Halt, VmExecutionMode, VmExecutionStopReason}; @@ -41,6 +40,10 @@ pub(crate) struct DefaultExecutionTracer { in_account_validation: bool, final_batch_info_requested: bool, pub(crate) result_tracer: ResultTracer, + // This tracer is designed specifically for calculating refunds. Its separation from the custom tracer + // ensures static dispatch, enhancing performance by avoiding dynamic dispatch overhead. + // Additionally, being an internal tracer, it saves the results directly to VmResultAndLogs. + pub(crate) refund_tracer: Option, pub(crate) custom_tracers: Vec>>, ret_from_the_bootloader: Option, storage: StoragePtr, @@ -53,17 +56,17 @@ impl Debug for DefaultExecutionTracer { } impl Tracer for DefaultExecutionTracer { - const CALL_BEFORE_DECODING: bool = true; + const CALL_BEFORE_DECODING: bool = false; const CALL_AFTER_DECODING: bool = true; const CALL_BEFORE_EXECUTION: bool = true; const CALL_AFTER_EXECUTION: bool = true; type SupportedMemory = SimpleMemory; - fn before_decoding(&mut self, state: VmLocalStateData<'_>, memory: &Self::SupportedMemory) { - >::before_decoding(&mut self.result_tracer, state, memory); - for tracer in self.custom_tracers.iter_mut() { - tracer.before_decoding(state, memory) - } + fn before_decoding( + &mut self, + _state: VmLocalStateData<'_, 8, EncodingModeProduction>, + _memory: &Self::SupportedMemory, + ) { } fn after_decoding( @@ -78,6 +81,11 @@ impl Tracer for DefaultExecutionTracer { data, memory, ); + + if let Some(refund_tracer) = &mut self.refund_tracer { + >::after_decoding(refund_tracer, state, data, memory); + } + for tracer in self.custom_tracers.iter_mut() { tracer.after_decoding(state, data, memory) } @@ -110,6 +118,10 @@ impl Tracer for DefaultExecutionTracer { gas_spent_on_bytecodes_and_long_messages_this_opcode(&state, &data); self.result_tracer .before_execution(state, data, memory, self.storage.clone()); + + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.before_execution(state, data, memory, self.storage.clone()); + } for tracer in self.custom_tracers.iter_mut() { tracer.before_execution(state, data, memory, self.storage.clone()); } @@ -137,48 +149,22 @@ impl Tracer for DefaultExecutionTracer { self.result_tracer .after_execution(state, data, memory, self.storage.clone()); + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.after_execution(state, data, memory, self.storage.clone()) + } for tracer in self.custom_tracers.iter_mut() { tracer.after_execution(state, data, memory, self.storage.clone()); } } } -impl ExecutionEndTracer for DefaultExecutionTracer { - fn should_stop_execution(&self) -> TracerExecutionStatus { - match self.execution_mode { - VmExecutionMode::OneTx => { - if self.tx_has_been_processed() { - return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); - } - } - VmExecutionMode::Bootloader => { - if self.ret_from_the_bootloader == Some(RetOpcode::Ok) { - return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); - } - } - VmExecutionMode::Batch => {} - }; - if self.validation_run_out_of_gas() { - return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( - Halt::ValidationOutOfGas, - )); - } - for tracer in self.custom_tracers.iter() { - let reason = tracer.should_stop_execution(); - if TracerExecutionStatus::Continue != reason { - return reason; - } - } - TracerExecutionStatus::Continue - } -} - impl DefaultExecutionTracer { pub(crate) fn new( computational_gas_limit: u32, execution_mode: VmExecutionMode, custom_tracers: Vec>>, storage: StoragePtr, + refund_tracer: Option, ) -> Self { Self { tx_has_been_processed: false, @@ -189,6 +175,7 @@ impl DefaultExecutionTracer { in_account_validation: false, final_batch_info_requested: false, result_tracer: ResultTracer::new(execution_mode), + refund_tracer, custom_tracers, ret_from_the_bootloader: None, storage, @@ -222,37 +209,60 @@ impl DefaultExecutionTracer { .populate_page(BOOTLOADER_HEAP_PAGE as usize, memory, current_timestamp); self.final_batch_info_requested = false; } + + fn should_stop_execution(&self) -> TracerExecutionStatus { + match self.execution_mode { + VmExecutionMode::OneTx if self.tx_has_been_processed() => { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); + } + VmExecutionMode::Bootloader if self.ret_from_the_bootloader == Some(RetOpcode::Ok) => { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); + } + _ => {} + }; + if self.validation_run_out_of_gas() { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( + Halt::ValidationOutOfGas, + )); + } + TracerExecutionStatus::Continue + } } impl DynTracer for DefaultExecutionTracer {} -impl ExecutionProcessing for DefaultExecutionTracer { +impl VmTracer for DefaultExecutionTracer { fn initialize_tracer(&mut self, state: &mut ZkSyncVmState) { self.result_tracer.initialize_tracer(state); - for processor in self.custom_tracers.iter_mut() { - processor.initialize_tracer(state); + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.initialize_tracer(state); } - } - - fn before_cycle(&mut self, state: &mut ZkSyncVmState) { - self.result_tracer.before_cycle(state); for processor in self.custom_tracers.iter_mut() { - processor.before_cycle(state); + processor.initialize_tracer(state); } } - fn after_cycle( + fn finish_cycle( &mut self, state: &mut ZkSyncVmState, bootloader_state: &mut BootloaderState, - ) { - self.result_tracer.after_cycle(state, bootloader_state); - for processor in self.custom_tracers.iter_mut() { - processor.after_cycle(state, bootloader_state); - } + ) -> TracerExecutionStatus { if self.final_batch_info_requested { self.set_fictive_l2_block(state, bootloader_state) } + + let mut result = self.result_tracer.finish_cycle(state, bootloader_state); + if let Some(refund_tracer) = &mut self.refund_tracer { + result = refund_tracer + .finish_cycle(state, bootloader_state) + .stricter(&result); + } + for processor in self.custom_tracers.iter_mut() { + result = processor + .finish_cycle(state, bootloader_state) + .stricter(&result); + } + result.stricter(&self.should_stop_execution()) } fn after_vm_execution( @@ -263,6 +273,10 @@ impl ExecutionProcessing for DefaultExecu ) { self.result_tracer .after_vm_execution(state, bootloader_state, stop_reason.clone()); + + if let Some(refund_tracer) = &mut self.refund_tracer { + refund_tracer.after_vm_execution(state, bootloader_state, stop_reason.clone()); + } for processor in self.custom_tracers.iter_mut() { processor.after_vm_execution(state, bootloader_state, stop_reason.clone()); } diff --git a/core/lib/vm/src/tracers/refunds.rs b/core/lib/vm/src/tracers/refunds.rs index d7f15d7c9b94..16e03f06916b 100644 --- a/core/lib/vm/src/tracers/refunds.rs +++ b/core/lib/vm/src/tracers/refunds.rs @@ -24,14 +24,11 @@ use crate::old_vm::{ use crate::bootloader_state::BootloaderState; use crate::tracers::utils::gas_spent_on_bytecodes_and_long_messages_this_opcode; use crate::tracers::{ - traits::{DynTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}, + traits::{DynTracer, VmTracer}, utils::{get_vm_hook_params, VmHook}, }; -use crate::types::{ - inputs::L1BatchEnv, - internals::ZkSyncVmState, - outputs::{Refunds, VmExecutionResultAndLogs}, -}; +use crate::types::{inputs::L1BatchEnv, internals::ZkSyncVmState, outputs::Refunds}; +use crate::TracerExecutionStatus; /// Tracer responsible for collecting information about refunds. #[derive(Debug, Clone)] @@ -79,6 +76,13 @@ impl RefundsTracer { 0 } + pub(crate) fn get_refunds(&self) -> Refunds { + Refunds { + gas_refunded: self.refund_gas, + operator_suggested_refund: self.operator_refund.unwrap_or_default(), + } + } + pub(crate) fn tx_body_refund( &self, bootloader_refund: u32, @@ -145,6 +149,7 @@ impl DynTracer for RefundsTracer { memory: &SimpleMemory, _storage: StoragePtr, ) { + self.timestamp_before_cycle = Timestamp(state.vm_local_state.timestamp); let hook = VmHook::from_opcode_memory(&state, &data); match hook { VmHook::NotifyAboutRefund => self.refund_gas = get_vm_hook_params(memory)[0].as_u32(), @@ -159,24 +164,18 @@ impl DynTracer for RefundsTracer { } } -impl ExecutionEndTracer for RefundsTracer {} - -impl ExecutionProcessing for RefundsTracer { +impl VmTracer for RefundsTracer { fn initialize_tracer(&mut self, state: &mut ZkSyncVmState) { self.timestamp_initial = Timestamp(state.local_state.timestamp); self.gas_remaining_before = state.local_state.callstack.current.ergs_remaining; self.spent_pubdata_counter_before = state.local_state.spent_pubdata_counter; } - fn before_cycle(&mut self, state: &mut ZkSyncVmState) { - self.timestamp_before_cycle = Timestamp(state.local_state.timestamp); - } - - fn after_cycle( + fn finish_cycle( &mut self, state: &mut ZkSyncVmState, bootloader_state: &mut BootloaderState, - ) { + ) -> TracerExecutionStatus { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] #[metrics(label = "type", rename_all = "snake_case")] enum RefundType { @@ -285,6 +284,7 @@ impl ExecutionProcessing for RefundsTrace (refund_to_propose as f64 - bootloader_refund as f64) / tx_gas_limit as f64 * 100.0; METRICS.refund_diff.observe(refund_diff); } + TracerExecutionStatus::Continue } } @@ -332,12 +332,3 @@ pub(crate) fn pubdata_published( + l2_l1_long_messages_bytes + published_bytecode_bytes } - -impl VmTracer for RefundsTracer { - fn save_results(&mut self, result: &mut VmExecutionResultAndLogs) { - result.refunds = Refunds { - gas_refunded: self.refund_gas, - operator_suggested_refund: self.operator_refund.unwrap_or_default(), - } - } -} diff --git a/core/lib/vm/src/tracers/result_tracer.rs b/core/lib/vm/src/tracers/result_tracer.rs index dd61ea49cea6..51c0ec4e9c5b 100644 --- a/core/lib/vm/src/tracers/result_tracer.rs +++ b/core/lib/vm/src/tracers/result_tracer.rs @@ -15,13 +15,10 @@ use crate::old_vm::{ utils::{vm_may_have_ended_inner, VmExecutionResult}, }; use crate::tracers::{ - traits::{DynTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}, + traits::{DynTracer, VmTracer}, utils::{get_vm_hook_params, read_pointer, VmHook}, }; -use crate::types::{ - internals::ZkSyncVmState, - outputs::{ExecutionResult, VmExecutionResultAndLogs}, -}; +use crate::types::{internals::ZkSyncVmState, outputs::ExecutionResult}; use crate::constants::{BOOTLOADER_HEAP_PAGE, RESULT_SUCCESS_FIRST_SLOT}; use crate::tracers::traits::TracerExecutionStopReason; @@ -104,9 +101,7 @@ impl DynTracer for ResultTracer { } } -impl ExecutionEndTracer for ResultTracer {} - -impl ExecutionProcessing for ResultTracer { +impl VmTracer for ResultTracer { fn after_vm_execution( &mut self, state: &mut ZkSyncVmState, @@ -237,10 +232,6 @@ impl ResultTracer { } } -impl VmTracer for ResultTracer { - fn save_results(&mut self, _result: &mut VmExecutionResultAndLogs) {} -} - pub(crate) fn tx_has_failed( state: &ZkSyncVmState, tx_id: u32, diff --git a/core/lib/vm/src/tracers/storage_invocations.rs b/core/lib/vm/src/tracers/storage_invocations.rs index bd7bbeb25c45..48195e19d13c 100644 --- a/core/lib/vm/src/tracers/storage_invocations.rs +++ b/core/lib/vm/src/tracers/storage_invocations.rs @@ -1,8 +1,7 @@ use crate::bootloader_state::BootloaderState; use crate::old_vm::history_recorder::HistoryMode; use crate::tracers::traits::{ - DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, - TracerExecutionStopReason, VmTracer, + DynTracer, TracerExecutionStatus, TracerExecutionStopReason, VmTracer, }; use crate::types::internals::ZkSyncVmState; use crate::Halt; @@ -11,12 +10,11 @@ use zksync_state::WriteStorage; #[derive(Debug, Default, Clone)] pub struct StorageInvocations { pub limit: usize, - current: usize, } impl StorageInvocations { pub fn new(limit: usize) -> Self { - Self { limit, current: 0 } + Self { limit } } } @@ -24,30 +22,24 @@ impl StorageInvocations { /// stopping the VM execution if the limit is reached. impl DynTracer for StorageInvocations {} -impl ExecutionEndTracer for StorageInvocations { - fn should_stop_execution(&self) -> TracerExecutionStatus { - if self.current >= self.limit { - return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( - Halt::TracerCustom("Storage invocations limit reached".to_string()), - )); - } - TracerExecutionStatus::Continue - } -} - -impl ExecutionProcessing for StorageInvocations { - fn after_cycle( +impl VmTracer for StorageInvocations { + fn finish_cycle( &mut self, state: &mut ZkSyncVmState, _bootloader_state: &mut BootloaderState, - ) { - self.current = state + ) -> TracerExecutionStatus { + let current = state .storage .storage .get_ptr() .borrow() .missed_storage_invocations(); + + if current >= self.limit { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( + Halt::TracerCustom("Storage invocations limit reached".to_string()), + )); + } + TracerExecutionStatus::Continue } } - -impl VmTracer for StorageInvocations {} diff --git a/core/lib/vm/src/tracers/traits.rs b/core/lib/vm/src/tracers/traits.rs index 4e76ed1fa15d..b9aee9e64fa3 100644 --- a/core/lib/vm/src/tracers/traits.rs +++ b/core/lib/vm/src/tracers/traits.rs @@ -7,21 +7,21 @@ use crate::bootloader_state::BootloaderState; use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; use crate::types::internals::ZkSyncVmState; -use crate::types::outputs::VmExecutionResultAndLogs; use crate::{Halt, VmExecutionStopReason}; /// Run tracer for collecting data during the vm execution cycles -pub trait ExecutionProcessing: - DynTracer + ExecutionEndTracer -{ +pub trait VmTracer: DynTracer { + /// Initialize the tracer before the vm execution fn initialize_tracer(&mut self, _state: &mut ZkSyncVmState) {} - fn before_cycle(&mut self, _state: &mut ZkSyncVmState) {} - fn after_cycle( + /// Run after each vm execution cycle + fn finish_cycle( &mut self, _state: &mut ZkSyncVmState, _bootloader_state: &mut BootloaderState, - ) { + ) -> TracerExecutionStatus { + TracerExecutionStatus::Continue } + /// Run after the vm execution fn after_vm_execution( &mut self, _state: &mut ZkSyncVmState, @@ -59,13 +59,6 @@ pub trait DynTracer { } } -/// Save the results of the vm execution. -pub trait VmTracer: - DynTracer + ExecutionEndTracer + ExecutionProcessing -{ - fn save_results(&mut self, _result: &mut VmExecutionResultAndLogs) {} -} - pub trait BoxedTracer { fn into_boxed(self) -> Box>; } @@ -88,10 +81,22 @@ pub enum TracerExecutionStatus { Stop(TracerExecutionStopReason), } -/// Stop the vm execution if the tracer conditions are met -pub trait ExecutionEndTracer { - // Returns whether the vm execution should stop. - fn should_stop_execution(&self) -> TracerExecutionStatus { - TracerExecutionStatus::Continue +impl TracerExecutionStatus { + /// Chose the stricter ExecutionStatus + /// If both statuses are Continue, then the result is Continue + /// If one of the statuses is Abort, then the result is Abort + /// If one of the statuses is Finish, then the result is Finish + pub fn stricter(&self, other: &Self) -> Self { + match (self, other) { + (Self::Continue, Self::Continue) => Self::Continue, + (Self::Stop(TracerExecutionStopReason::Abort(reason)), _) + | (_, Self::Stop(TracerExecutionStopReason::Abort(reason))) => { + Self::Stop(TracerExecutionStopReason::Abort(reason.clone())) + } + (Self::Stop(TracerExecutionStopReason::Finish), _) + | (_, Self::Stop(TracerExecutionStopReason::Finish)) => { + Self::Stop(TracerExecutionStopReason::Finish) + } + } } } diff --git a/core/lib/vm/src/tracers/validation/mod.rs b/core/lib/vm/src/tracers/validation/mod.rs index dd06fe8e5dbd..77baa420dd29 100644 --- a/core/lib/vm/src/tracers/validation/mod.rs +++ b/core/lib/vm/src/tracers/validation/mod.rs @@ -29,8 +29,7 @@ use zksync_utils::{ use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; use crate::tracers::traits::{ - DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, - TracerExecutionStopReason, VmTracer, + DynTracer, TracerExecutionStatus, TracerExecutionStopReason, VmTracer, }; use crate::tracers::utils::{ computational_gas_price, get_calldata_page_via_abi, print_debug_if_needed, VmHook, @@ -42,7 +41,7 @@ pub use params::ValidationTracerParams; use types::NewTrustedValidationItems; use types::ValidationTracerMode; -use crate::{Halt, VmExecutionResultAndLogs}; +use crate::{BootloaderState, Halt, ZkSyncVmState}; /// Tracer that is used to ensure that the validation adheres to all the rules /// to prevent DDoS attacks on the server. @@ -355,8 +354,12 @@ impl DynTracer for ValidationTracer { } } -impl ExecutionEndTracer for ValidationTracer { - fn should_stop_execution(&self) -> TracerExecutionStatus { +impl VmTracer for ValidationTracer { + fn finish_cycle( + &mut self, + _state: &mut ZkSyncVmState, + _bootloader_state: &mut BootloaderState, + ) -> TracerExecutionStatus { if self.should_stop_execution { return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); } @@ -369,12 +372,6 @@ impl ExecutionEndTracer for ValidationTracer { } } -impl ExecutionProcessing for ValidationTracer {} - -impl VmTracer for ValidationTracer { - fn save_results(&mut self, _result: &mut VmExecutionResultAndLogs) {} -} - fn touches_allowed_context(address: Address, key: U256) -> bool { // Context is not touched at all if address != SYSTEM_CONTEXT_ADDRESS {