From cf44a491a324199b4cf457d28658da44b6dafc61 Mon Sep 17 00:00:00 2001 From: Danil Date: Tue, 3 Oct 2023 14:45:52 +0200 Subject: [PATCH] feat(vm): Introduce new way of returning from the tracer #2569 (#116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … private at a4d5ca6625210471d9de66d61e7c9a41a336afb8 # What ❔ ## 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 --- core/lib/vm/src/errors/halt.rs | 9 ++++ core/lib/vm/src/implementation/execution.rs | 10 +++-- core/lib/vm/src/tracers/default_tracers.rs | 42 +++++++++++++------ core/lib/vm/src/tracers/result_tracer.rs | 13 +++++- .../lib/vm/src/tracers/storage_invocations.rs | 15 +++++-- core/lib/vm/src/tracers/traits.rs | 30 +++++++++---- core/lib/vm/src/tracers/utils.rs | 5 ++- core/lib/vm/src/tracers/validation/mod.rs | 19 +++++++-- .../src/api_server/execution_sandbox/error.rs | 4 ++ .../api_server/execution_sandbox/validate.rs | 2 +- 10 files changed, 112 insertions(+), 37 deletions(-) diff --git a/core/lib/vm/src/errors/halt.rs b/core/lib/vm/src/errors/halt.rs index 10c8a8d702b9..0a5057a0616f 100644 --- a/core/lib/vm/src/errors/halt.rs +++ b/core/lib/vm/src/errors/halt.rs @@ -26,6 +26,8 @@ pub enum Halt { UnexpectedVMBehavior(String), // Bootloader is out of gas. BootloaderOutOfGas, + // Validation step is out of gas + ValidationOutOfGas, // Transaction has a too big gas limit and will not be executed by the server. TooBigGasLimit, // The bootloader did not have enough gas to start the transaction in the first place @@ -37,6 +39,7 @@ pub enum Halt { // Failed to publish information about the batch and the L2 block onto L1 FailedToAppendTransactionToL2Block(String), VMPanic, + TracerCustom(String), } impl Display for Halt { @@ -102,6 +105,12 @@ impl Display for Halt { reason ) } + Halt::TracerCustom(reason) => { + write!(f, "Tracer aborted execution: {}", reason) + } + Halt::ValidationOutOfGas => { + write!(f, "Validation run out of gas") + } } } } diff --git a/core/lib/vm/src/implementation/execution.rs b/core/lib/vm/src/implementation/execution.rs index 9944a37f7e83..52c4ff0cb0da 100644 --- a/core/lib/vm/src/implementation/execution.rs +++ b/core/lib/vm/src/implementation/execution.rs @@ -6,7 +6,9 @@ use crate::old_vm::{ utils::{vm_may_have_ended_inner, VmExecutionResult}, }; use crate::tracers::{ - traits::{BoxedTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}, + traits::{ + BoxedTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, VmTracer, + }, DefaultExecutionTracer, RefundsTracer, }; use crate::types::{inputs::VmExecutionMode, outputs::VmExecutionResultAndLogs}; @@ -104,11 +106,11 @@ impl Vm { break VmExecutionStopReason::VmFinished; } - if tracer.should_stop_execution() { - break VmExecutionStopReason::TracerRequestedStop; + if let TracerExecutionStatus::Stop(reason) = tracer.should_stop_execution() { + break VmExecutionStopReason::TracerRequestedStop(reason); } }; - tracer.after_vm_execution(&mut self.state, &self.bootloader_state, result); + tracer.after_vm_execution(&mut self.state, &self.bootloader_state, result.clone()); result } diff --git a/core/lib/vm/src/tracers/default_tracers.rs b/core/lib/vm/src/tracers/default_tracers.rs index 7cc1e19869cf..4df00193265d 100644 --- a/core/lib/vm/src/tracers/default_tracers.rs +++ b/core/lib/vm/src/tracers/default_tracers.rs @@ -16,14 +16,17 @@ use crate::bootloader_state::BootloaderState; 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, VmTracer}; +use crate::tracers::traits::{ + DynTracer, ExecutionEndTracer, ExecutionProcessing, 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::types::internals::ZkSyncVmState; -use crate::{VmExecutionMode, VmExecutionStopReason}; +use crate::{Halt, VmExecutionMode, VmExecutionStopReason}; /// Default tracer for the VM. It manages the other tracers execution and stop the vm when needed. pub(crate) struct DefaultExecutionTracer { @@ -141,17 +144,32 @@ impl Tracer for DefaultExecutionTracer { } impl ExecutionEndTracer for DefaultExecutionTracer { - fn should_stop_execution(&self) -> bool { - let mut should_stop = match self.execution_mode { - VmExecutionMode::OneTx => self.tx_has_been_processed(), - VmExecutionMode::Batch => false, - VmExecutionMode::Bootloader => self.ret_from_the_bootloader == Some(RetOpcode::Ok), + 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 => {} }; - should_stop = should_stop || self.validation_run_out_of_gas(); + if self.validation_run_out_of_gas() { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( + Halt::ValidationOutOfGas, + )); + } for tracer in self.custom_tracers.iter() { - should_stop = should_stop || tracer.should_stop_execution(); + let reason = tracer.should_stop_execution(); + if TracerExecutionStatus::Continue != reason { + return reason; + } } - should_stop + TracerExecutionStatus::Continue } } @@ -244,9 +262,9 @@ impl ExecutionProcessing for DefaultExecu stop_reason: VmExecutionStopReason, ) { self.result_tracer - .after_vm_execution(state, bootloader_state, stop_reason); + .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); + processor.after_vm_execution(state, bootloader_state, stop_reason.clone()); } } } diff --git a/core/lib/vm/src/tracers/result_tracer.rs b/core/lib/vm/src/tracers/result_tracer.rs index b8e089493565..dd61ea49cea6 100644 --- a/core/lib/vm/src/tracers/result_tracer.rs +++ b/core/lib/vm/src/tracers/result_tracer.rs @@ -24,6 +24,7 @@ use crate::types::{ }; use crate::constants::{BOOTLOADER_HEAP_PAGE, RESULT_SUCCESS_FIRST_SLOT}; +use crate::tracers::traits::TracerExecutionStopReason; use crate::{Halt, TxRevertReason}; use crate::{VmExecutionMode, VmExecutionStopReason}; @@ -120,9 +121,11 @@ impl ExecutionProcessing for ResultTracer // One of the tracers above has requested to stop the execution. // If it was the correct stop we already have the result, // otherwise it can be out of gas error - VmExecutionStopReason::TracerRequestedStop => { + VmExecutionStopReason::TracerRequestedStop(reason) => { match self.execution_mode { - VmExecutionMode::OneTx => self.vm_stopped_execution(state, bootloader_state), + VmExecutionMode::OneTx => { + self.vm_stopped_execution(state, bootloader_state, reason) + } VmExecutionMode::Batch => self.vm_finished_execution(state), VmExecutionMode::Bootloader => self.vm_finished_execution(state), }; @@ -188,7 +191,13 @@ impl ResultTracer { &mut self, state: &ZkSyncVmState, bootloader_state: &BootloaderState, + reason: TracerExecutionStopReason, ) { + if let TracerExecutionStopReason::Abort(halt) = reason { + self.result = Some(Result::Halt { reason: halt }); + return; + } + if self.bootloader_out_of_gas { self.result = Some(Result::Halt { reason: Halt::BootloaderOutOfGas, diff --git a/core/lib/vm/src/tracers/storage_invocations.rs b/core/lib/vm/src/tracers/storage_invocations.rs index ef4b59c60a88..bd6f419eddfb 100644 --- a/core/lib/vm/src/tracers/storage_invocations.rs +++ b/core/lib/vm/src/tracers/storage_invocations.rs @@ -1,7 +1,11 @@ use crate::bootloader_state::BootloaderState; use crate::old_vm::history_recorder::HistoryMode; -use crate::tracers::traits::{DynTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}; +use crate::tracers::traits::{ + DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, + TracerExecutionStopReason, VmTracer, +}; use crate::types::internals::ZkSyncVmState; +use crate::Halt; use zksync_state::WriteStorage; #[derive(Debug, Default, Clone)] @@ -21,8 +25,13 @@ impl StorageInvocations { impl DynTracer for StorageInvocations {} impl ExecutionEndTracer for StorageInvocations { - fn should_stop_execution(&self) -> bool { - self.current >= self.limit + 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 } } diff --git a/core/lib/vm/src/tracers/traits.rs b/core/lib/vm/src/tracers/traits.rs index 33e149066b16..4e76ed1fa15d 100644 --- a/core/lib/vm/src/tracers/traits.rs +++ b/core/lib/vm/src/tracers/traits.rs @@ -8,7 +8,7 @@ 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::VmExecutionStopReason; +use crate::{Halt, VmExecutionStopReason}; /// Run tracer for collecting data during the vm execution cycles pub trait ExecutionProcessing: @@ -31,14 +31,6 @@ pub trait ExecutionProcessing: } } -/// 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) -> bool { - false - } -} - /// Version of zk_evm::Tracer suitable for dynamic dispatch. pub trait DynTracer { fn before_decoding(&mut self, _state: VmLocalStateData<'_>, _memory: &SimpleMemory) {} @@ -83,3 +75,23 @@ impl + 'static> BoxedTracer { + // Returns whether the vm execution should stop. + fn should_stop_execution(&self) -> TracerExecutionStatus { + TracerExecutionStatus::Continue + } +} diff --git a/core/lib/vm/src/tracers/utils.rs b/core/lib/vm/src/tracers/utils.rs index f86b496b0787..5f9090d6180c 100644 --- a/core/lib/vm/src/tracers/utils.rs +++ b/core/lib/vm/src/tracers/utils.rs @@ -18,6 +18,7 @@ use crate::constants::{ use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; use crate::old_vm::utils::{aux_heap_page_from_base, heap_page_from_base}; +use crate::tracers::traits::TracerExecutionStopReason; #[derive(Clone, Debug, Copy)] pub(crate) enum VmHook { @@ -217,8 +218,8 @@ pub(crate) fn get_vm_hook_params(memory: &SimpleMemory) -> Ve ) } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub enum VmExecutionStopReason { VmFinished, - TracerRequestedStop, + TracerRequestedStop(TracerExecutionStopReason), } diff --git a/core/lib/vm/src/tracers/validation/mod.rs b/core/lib/vm/src/tracers/validation/mod.rs index 4b94e3f177b5..d85d031665ac 100644 --- a/core/lib/vm/src/tracers/validation/mod.rs +++ b/core/lib/vm/src/tracers/validation/mod.rs @@ -27,7 +27,10 @@ use zksync_utils::{ use crate::old_vm::history_recorder::HistoryMode; use crate::old_vm::memory::SimpleMemory; -use crate::tracers::traits::{DynTracer, ExecutionEndTracer, ExecutionProcessing, VmTracer}; +use crate::tracers::traits::{ + DynTracer, ExecutionEndTracer, ExecutionProcessing, TracerExecutionStatus, + TracerExecutionStopReason, VmTracer, +}; use crate::tracers::utils::{ computational_gas_price, get_calldata_page_via_abi, print_debug_if_needed, VmHook, }; @@ -38,7 +41,7 @@ pub use params::ValidationTracerParams; use types::NewTrustedValidationItems; use types::ValidationTracerMode; -use crate::VmExecutionResultAndLogs; +use crate::{Halt, VmExecutionResultAndLogs}; /// Tracer that is used to ensure that the validation adheres to all the rules /// to prevent DDoS attacks on the server. @@ -341,8 +344,16 @@ impl DynTracer for ValidationTracer { } impl ExecutionEndTracer for ValidationTracer { - fn should_stop_execution(&self) -> bool { - self.should_stop_execution || self.result.get().is_some() + fn should_stop_execution(&self) -> TracerExecutionStatus { + if self.should_stop_execution { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Finish); + } + if let Some(result) = self.result.get() { + return TracerExecutionStatus::Stop(TracerExecutionStopReason::Abort( + Halt::TracerCustom(format!("Validation error: {:#?}", result)), + )); + } + TracerExecutionStatus::Continue } } diff --git a/core/lib/zksync_core/src/api_server/execution_sandbox/error.rs b/core/lib/zksync_core/src/api_server/execution_sandbox/error.rs index abc50af37a5f..b4f04e2e5d60 100644 --- a/core/lib/zksync_core/src/api_server/execution_sandbox/error.rs +++ b/core/lib/zksync_core/src/api_server/execution_sandbox/error.rs @@ -61,6 +61,10 @@ impl From for SandboxExecutionError { Halt::FailedToAppendTransactionToL2Block(reason) => { SandboxExecutionError::Revert(reason, vec![]) } + Halt::TracerCustom(reason) => SandboxExecutionError::Revert(reason, vec![]), + Halt::ValidationOutOfGas => Self::AccountValidationFailed( + "The validation of the transaction ran out of gas".to_string(), + ), } } } diff --git a/core/lib/zksync_core/src/api_server/execution_sandbox/validate.rs b/core/lib/zksync_core/src/api_server/execution_sandbox/validate.rs index 05eb7f3ce2d9..2dd5ae7b9c25 100644 --- a/core/lib/zksync_core/src/api_server/execution_sandbox/validate.rs +++ b/core/lib/zksync_core/src/api_server/execution_sandbox/validate.rs @@ -86,8 +86,8 @@ impl TxSharedArgs { ]); let result = match (result.result, validation_result.get()) { - (ExecutionResult::Halt { reason }, _) => Err(ValidationError::FailedTx(reason)), (_, Some(err)) => Err(ValidationError::ViolatedRule(err.clone())), + (ExecutionResult::Halt { reason }, _) => Err(ValidationError::FailedTx(reason)), (_, None) => Ok(()), };