From edb7c4538414c7e587fad3dd73835fb536af2e56 Mon Sep 17 00:00:00 2001 From: Joonatan Saarhelo Date: Thu, 31 Oct 2024 12:48:10 +0100 Subject: [PATCH] stopped by tracer is distinguishable from other ends --- crates/vm2-interface/src/lib.rs | 46 +++---------------- crates/vm2-interface/src/tracer_interface.rs | 28 +++++++++-- crates/vm2/src/decode.rs | 3 +- crates/vm2/src/instruction.rs | 43 ++++++++++++++++- crates/vm2/src/instruction_handlers/common.rs | 10 ++-- crates/vm2/src/instruction_handlers/ret.rs | 16 +++---- crates/vm2/src/lib.rs | 3 +- crates/vm2/src/vm.rs | 5 +- 8 files changed, 91 insertions(+), 63 deletions(-) diff --git a/crates/vm2-interface/src/lib.rs b/crates/vm2-interface/src/lib.rs index e47bdc3..dfea319 100644 --- a/crates/vm2-interface/src/lib.rs +++ b/crates/vm2-interface/src/lib.rs @@ -28,6 +28,7 @@ //! # use zksync_vm2_interface as zksync_vm2_interface_v1; //! use zksync_vm2_interface_v1::{ //! StateInterface as StateInterfaceV1, GlobalStateInterface as GlobalStateInterfaceV1, Tracer as TracerV1, opcodes::NearCall, +//! ShouldStop, //! }; //! //! trait StateInterface: StateInterfaceV1 { @@ -60,7 +61,9 @@ //! //! trait Tracer { //! fn before_instruction(&mut self, state: &mut S) {} -//! fn after_instruction(&mut self, state: &mut S) {} +//! fn after_instruction(&mut self, state: &mut S) -> ShouldStop { +//! ShouldStop::Continue +//! } //! } //! //! impl Tracer for T { @@ -73,7 +76,9 @@ //! } //! } //! } -//! fn after_instruction(&mut self, state: &mut S) {} +//! fn after_instruction(&mut self, state: &mut S) -> ShouldStop { +//! todo!() +//! } //! } //! //! // Now you can use the new features by implementing TracerV2 @@ -91,40 +96,3 @@ pub use self::{state_interface::*, tracer_interface::*}; mod state_interface; mod tracer_interface; - -/// Returned from [`Tracer::after_instruction`] to indicate whether the VM should continue execution. -#[derive(Debug)] -pub enum ExecutionStatus { - /// Continue execution. - Running, - /// Stop or suspend execution. - Stopped(ExecutionEnd), -} - -impl ExecutionStatus { - /// Combines [`ExecutionStatus`] values from two sources, choosing the most severe one. - /// So if one tracer wants to suspend but the other wants to panic, the VM will panic. - #[must_use] - #[inline(always)] - pub fn merge(self, other: Self) -> Self { - use ExecutionStatus::{Running, Stopped}; - match (self, other) { - (Running | Stopped(ExecutionEnd::SuspendedOnHook(_)), other @ Stopped(_)) => other, - (me @ Stopped(_), _) => me, - _ => Running, - } - } -} - -/// VM stop reason and return value. -#[derive(Debug, PartialEq)] -pub enum ExecutionEnd { - /// The executed program has finished and returned the specified data. - ProgramFinished(Vec), - /// The executed program has reverted returning the specified data. - Reverted(Vec), - /// The executed program has panicked. - Panicked, - /// Returned when the bootloader writes to the heap location specified by [`hook_address`](crate::Settings.hook_address). - SuspendedOnHook(u32), -} diff --git a/crates/vm2-interface/src/tracer_interface.rs b/crates/vm2-interface/src/tracer_interface.rs index 0cf6a26..29539b9 100644 --- a/crates/vm2-interface/src/tracer_interface.rs +++ b/crates/vm2-interface/src/tracer_interface.rs @@ -1,4 +1,4 @@ -use crate::{ExecutionStatus, GlobalStateInterface}; +use crate::GlobalStateInterface; macro_rules! forall_simple_opcodes { ($m:ident) => { @@ -271,9 +271,9 @@ pub trait Tracer { fn after_instruction( &mut self, state: &mut S, - ) -> ExecutionStatus { + ) -> ShouldStop { let _ = state; - ExecutionStatus::Running + ShouldStop::Continue } /// Provides cycle statistics for "complex" instructions from the prover perspective (mostly precompile calls). @@ -282,6 +282,26 @@ pub trait Tracer { fn on_extra_prover_cycles(&mut self, _stats: CycleStats) {} } +/// Returned from [`Tracer::after_instruction`] to indicate if the VM should stop. +#[derive(Debug)] +pub enum ShouldStop { + /// The VM should stop. + Stop, + /// The VM should continue. + Continue, +} + +impl ShouldStop { + #[must_use] + #[inline(always)] + fn merge(self, other: ShouldStop) -> ShouldStop { + match (self, other) { + (ShouldStop::Continue, ShouldStop::Continue) => ShouldStop::Continue, + _ => ShouldStop::Stop, + } + } +} + /// Cycle statistics emitted by the VM and supplied to [`Tracer::on_extra_prover_cycles()`]. #[derive(Debug, Clone, Copy)] pub enum CycleStats { @@ -314,7 +334,7 @@ impl Tracer for (A, B) { fn after_instruction( &mut self, state: &mut S, - ) -> ExecutionStatus { + ) -> ShouldStop { self.0 .after_instruction::(state) .merge(self.1.after_instruction::(state)) diff --git a/crates/vm2/src/decode.rs b/crates/vm2/src/decode.rs index 33b9a7a..329a570 100644 --- a/crates/vm2/src/decode.rs +++ b/crates/vm2/src/decode.rs @@ -11,7 +11,7 @@ use zksync_vm2_interface::{ self, Add, And, Div, Mul, Or, PointerAdd, PointerPack, PointerShrink, PointerSub, RotateLeft, RotateRight, ShiftLeft, ShiftRight, Sub, Xor, }, - ExecutionEnd, ExecutionStatus, Tracer, + Tracer, }; use crate::{ @@ -20,6 +20,7 @@ use crate::{ Immediate1, Immediate2, Register, Register1, Register2, RegisterAndImmediate, RelativeStack, SourceWriter, }, + instruction::{ExecutionEnd, ExecutionStatus}, mode_requirements::ModeRequirements, Instruction, Predicate, VirtualMachine, World, }; diff --git a/crates/vm2/src/instruction.rs b/crates/vm2/src/instruction.rs index 78ceb6b..230c7a1 100644 --- a/crates/vm2/src/instruction.rs +++ b/crates/vm2/src/instruction.rs @@ -1,6 +1,6 @@ use std::fmt; -pub(crate) use zksync_vm2_interface::ExecutionStatus; +use zksync_vm2_interface::ShouldStop; use crate::{addressing_modes::Arguments, vm::VirtualMachine}; @@ -23,3 +23,44 @@ impl fmt::Debug for Instruction { } pub(crate) type Handler = fn(&mut VirtualMachine, &mut W, &mut T) -> ExecutionStatus; + +#[derive(Debug)] +pub(crate) enum ExecutionStatus { + Running, + Stopped(ExecutionEnd), +} + +impl ExecutionStatus { + #[must_use] + #[inline(always)] + pub(crate) fn merge_tracer(self, should_stop: ShouldStop) -> Self { + match (&self, should_stop) { + (Self::Running, ShouldStop::Stop) => Self::Stopped(ExecutionEnd::StoppedByTracer), + _ => self, + } + } +} + +impl From for ExecutionStatus { + fn from(should_stop: ShouldStop) -> Self { + match should_stop { + ShouldStop::Stop => Self::Stopped(ExecutionEnd::StoppedByTracer), + ShouldStop::Continue => Self::Running, + } + } +} + +/// VM stop reason returned from [`VirtualMachine::run()`]. +#[derive(Debug, PartialEq)] +pub enum ExecutionEnd { + /// The executed program has finished and returned the specified data. + ProgramFinished(Vec), + /// The executed program has reverted returning the specified data. + Reverted(Vec), + /// The executed program has panicked. + Panicked, + /// Returned when the bootloader writes to the heap location specified by [`hook_address`](crate::Settings.hook_address). + SuspendedOnHook(u32), + /// One of the tracers decided it is time to stop the VM. + StoppedByTracer, +} diff --git a/crates/vm2/src/instruction_handlers/common.rs b/crates/vm2/src/instruction_handlers/common.rs index 895000d..2576e25 100644 --- a/crates/vm2/src/instruction_handlers/common.rs +++ b/crates/vm2/src/instruction_handlers/common.rs @@ -58,13 +58,13 @@ pub(crate) fn full_boilerplate>( if args.predicate().satisfied(&vm.state.flags) { tracer.before_instruction::(&mut VmAndWorld { vm, world }); vm.state.current_frame.pc = unsafe { vm.state.current_frame.pc.add(1) }; - let result = business_logic(vm, args, world, tracer); - tracer - .after_instruction::(&mut VmAndWorld { vm, world }) - .merge(result) + business_logic(vm, args, world, tracer) + .merge_tracer(tracer.after_instruction::(&mut VmAndWorld { vm, world })) } else { tracer.before_instruction::(&mut VmAndWorld { vm, world }); vm.state.current_frame.pc = unsafe { vm.state.current_frame.pc.add(1) }; - tracer.after_instruction::(&mut VmAndWorld { vm, world }) + tracer + .after_instruction::(&mut VmAndWorld { vm, world }) + .into() } } diff --git a/crates/vm2/src/instruction_handlers/ret.rs b/crates/vm2/src/instruction_handlers/ret.rs index ebbd7f5..17c3699 100644 --- a/crates/vm2/src/instruction_handlers/ret.rs +++ b/crates/vm2/src/instruction_handlers/ret.rs @@ -1,7 +1,7 @@ use primitive_types::U256; use zksync_vm2_interface::{ opcodes::{self, Normal, Panic, Revert, TypeLevelReturnType}, - ExecutionEnd, ReturnType, Tracer, + ReturnType, Tracer, }; use super::{ @@ -12,7 +12,7 @@ use super::{ use crate::{ addressing_modes::{Arguments, Immediate1, Register1, Source, INVALID_INSTRUCTION_COST}, callframe::FrameRemnant, - instruction::ExecutionStatus, + instruction::{ExecutionEnd, ExecutionStatus}, mode_requirements::ModeRequirements, predication::Flags, tracing::VmAndWorld, @@ -146,13 +146,11 @@ pub(crate) fn free_panic>( ) -> ExecutionStatus { tracer.before_instruction::, _>(&mut VmAndWorld { vm, world }); // args aren't used for panics unless TO_LABEL - let result = naked_ret::( + naked_ret::( vm, &Arguments::new(Predicate::Always, 0, ModeRequirements::none()), - ); - tracer - .after_instruction::, _>(&mut VmAndWorld { vm, world }) - .merge(result) + ) + .merge_tracer(tracer.after_instruction::, _>(&mut VmAndWorld { vm, world })) } /// Formally, a far call pushes a new frame and returns from it immediately if it panics. @@ -173,7 +171,9 @@ pub(crate) fn panic_from_failed_far_call>( vm.state.flags = Flags::new(true, false, false); vm.state.current_frame.set_pc_from_u16(exception_handler); - tracer.after_instruction::, _>(&mut VmAndWorld { vm, world }) + tracer + .after_instruction::, _>(&mut VmAndWorld { vm, world }) + .into() } fn invalid>( diff --git a/crates/vm2/src/lib.rs b/crates/vm2/src/lib.rs index daf52e1..c49514e 100644 --- a/crates/vm2/src/lib.rs +++ b/crates/vm2/src/lib.rs @@ -6,7 +6,6 @@ use std::hash::{DefaultHasher, Hash, Hasher}; use primitive_types::{H160, U256}; pub use zksync_vm2_interface as interface; -pub use zksync_vm2_interface::ExecutionEnd; use zksync_vm2_interface::Tracer; // Re-export missing modules if single instruction testing is enabled @@ -14,7 +13,7 @@ use zksync_vm2_interface::Tracer; pub(crate) use self::single_instruction_test::{heap, program, stack}; pub use self::{ fat_pointer::FatPointer, - instruction::Instruction, + instruction::{ExecutionEnd, Instruction}, mode_requirements::ModeRequirements, predication::Predicate, program::Program, diff --git a/crates/vm2/src/vm.rs b/crates/vm2/src/vm.rs index 32c396a..6c8d436 100644 --- a/crates/vm2/src/vm.rs +++ b/crates/vm2/src/vm.rs @@ -1,13 +1,12 @@ use std::fmt; use primitive_types::H160; -use zksync_vm2_interface::{ - opcodes::TypeLevelCallingMode, CallingMode, ExecutionStatus, HeapId, Tracer, -}; +use zksync_vm2_interface::{opcodes::TypeLevelCallingMode, CallingMode, HeapId, Tracer}; use crate::{ callframe::{Callframe, FrameRemnant}, decommit::u256_into_address, + instruction::ExecutionStatus, stack::StackPool, state::{State, StateSnapshot}, world_diff::{ExternalSnapshot, Snapshot, WorldDiff},