From bf09d5b01545bd125dc99b50464ed2d93704048a Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 21 Aug 2024 13:41:47 +0300 Subject: [PATCH 01/24] Sketch customizable error reporting --- core/lib/multivm/src/versions/shadow.rs | 241 +++++++++++++++--------- 1 file changed, 156 insertions(+), 85 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index 6af546318af4..ce557b750fc3 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,9 +1,9 @@ use std::{ + cell::RefCell, collections::{BTreeMap, HashSet}, fmt, }; -use anyhow::Context as _; use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; use crate::{ @@ -17,10 +17,30 @@ use crate::{ vm_fast, }; +struct VmWithReporting { + vm: vm_fast::Vm>, + reporter: Box, +} + +impl fmt::Debug for VmWithReporting { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("VmWithReporting") + .field("vm", &self.vm) + .finish_non_exhaustive() + } +} + +impl VmWithReporting { + fn report(mut self, err: anyhow::Error) { + (self.reporter)(err); + } +} + #[derive(Debug)] pub struct ShadowVm { main: T, - shadow: vm_fast::Vm>, + shadow: RefCell>>, } impl VmFactory> for ShadowVm @@ -33,9 +53,14 @@ where system_env: SystemEnv, storage: StoragePtr>, ) -> Self { + let main = T::new(batch_env.clone(), system_env.clone(), storage.clone()); + let shadow = vm_fast::Vm::new(batch_env, system_env, ImmutableStorageView::new(storage)); Self { - main: T::new(batch_env.clone(), system_env.clone(), storage.clone()), - shadow: vm_fast::Vm::new(batch_env, system_env, ImmutableStorageView::new(storage)), + main, + shadow: RefCell::new(Some(VmWithReporting { + vm: shadow, + reporter: Box::new(|err| panic!("{err:?}")), + })), } } } @@ -48,19 +73,23 @@ where type TracerDispatcher = T::TracerDispatcher; fn push_transaction(&mut self, tx: Transaction) { - self.shadow.push_transaction(tx.clone()); + if let Some(shadow) = self.shadow.get_mut() { + shadow.vm.push_transaction(tx.clone()); + } self.main.push_transaction(tx); } fn execute(&mut self, execution_mode: VmExecutionMode) -> VmExecutionResultAndLogs { let main_result = self.main.execute(execution_mode); - let shadow_result = self.shadow.execute(execution_mode); - let mut errors = DivergenceErrors::default(); - errors.check_results_match(&main_result, &shadow_result); - errors - .into_result() - .with_context(|| format!("executing VM with mode {execution_mode:?}")) - .unwrap(); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_result = shadow.vm.execute(execution_mode); + let mut errors = DivergenceErrors::default(); + errors.check_results_match(&main_result, &shadow_result); + if let Err(err) = errors.into_result() { + let ctx = format!("executing VM with mode {execution_mode:?}"); + self.shadow.take().unwrap().report(err.context(ctx)); + } + } main_result } @@ -69,46 +98,68 @@ where dispatcher: Self::TracerDispatcher, execution_mode: VmExecutionMode, ) -> VmExecutionResultAndLogs { - let shadow_result = self.shadow.inspect((), execution_mode); let main_result = self.main.inspect(dispatcher, execution_mode); - let mut errors = DivergenceErrors::default(); - errors.check_results_match(&main_result, &shadow_result); - errors - .into_result() - .with_context(|| format!("executing VM with mode {execution_mode:?}")) - .unwrap(); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_result = shadow.vm.inspect((), execution_mode); + let mut errors = DivergenceErrors::default(); + errors.check_results_match(&main_result, &shadow_result); + + if let Err(err) = errors.into_result() { + self.shadow + .take() + .unwrap() + .report(err.context(format!("executing VM with mode {execution_mode:?}"))); + } + } main_result } fn get_bootloader_memory(&self) -> BootloaderMemory { let main_memory = self.main.get_bootloader_memory(); - let shadow_memory = self.shadow.get_bootloader_memory(); - DivergenceErrors::single("get_bootloader_memory", &main_memory, &shadow_memory).unwrap(); + if let Some(shadow) = &*self.shadow.borrow() { + let shadow_memory = shadow.vm.get_bootloader_memory(); + let result = + DivergenceErrors::single("get_bootloader_memory", &main_memory, &shadow_memory); + if let Err(err) = result { + self.shadow.take().unwrap().report(err); + } + } main_memory } fn get_last_tx_compressed_bytecodes(&self) -> Vec { let main_bytecodes = self.main.get_last_tx_compressed_bytecodes(); - let shadow_bytecodes = self.shadow.get_last_tx_compressed_bytecodes(); - DivergenceErrors::single( - "get_last_tx_compressed_bytecodes", - &main_bytecodes, - &shadow_bytecodes, - ) - .unwrap(); + if let Some(shadow) = &*self.shadow.borrow() { + let shadow_bytecodes = shadow.vm.get_last_tx_compressed_bytecodes(); + let result = DivergenceErrors::single( + "get_last_tx_compressed_bytecodes", + &main_bytecodes, + &shadow_bytecodes, + ); + if let Err(err) = result { + self.shadow.take().unwrap().report(err); + } + } main_bytecodes } fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { - self.shadow.start_new_l2_block(l2_block_env); self.main.start_new_l2_block(l2_block_env); + if let Some(shadow) = self.shadow.get_mut() { + shadow.vm.start_new_l2_block(l2_block_env); + } } fn get_current_execution_state(&self) -> CurrentExecutionState { let main_state = self.main.get_current_execution_state(); - let shadow_state = self.shadow.get_current_execution_state(); - DivergenceErrors::single("get_current_execution_state", &main_state, &shadow_state) - .unwrap(); + if let Some(shadow) = &*self.shadow.borrow() { + let shadow_state = shadow.vm.get_current_execution_state(); + let result = + DivergenceErrors::single("get_current_execution_state", &main_state, &shadow_state); + if let Err(err) = result { + self.shadow.take().unwrap().report(err); + } + } main_state } @@ -124,17 +175,19 @@ where let main_result = self .main .execute_transaction_with_bytecode_compression(tx.clone(), with_compression); - let shadow_result = self - .shadow - .execute_transaction_with_bytecode_compression(tx, with_compression); - let mut errors = DivergenceErrors::default(); - errors.check_results_match(&main_result.1, &shadow_result.1); - errors - .into_result() - .with_context(|| { - format!("executing transaction {tx_hash:?}, with_compression={with_compression:?}") - }) - .unwrap(); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_result = shadow + .vm + .execute_transaction_with_bytecode_compression(tx, with_compression); + let mut errors = DivergenceErrors::default(); + errors.check_results_match(&main_result.1, &shadow_result.1); + if let Err(err) = errors.into_result() { + let ctx = format!( + "executing transaction {tx_hash:?}, with_compression={with_compression:?}" + ); + self.shadow.take().unwrap().report(err.context(ctx)); + } + } main_result } @@ -153,17 +206,20 @@ where tx.clone(), with_compression, ); - let shadow_result = - self.shadow - .inspect_transaction_with_bytecode_compression((), tx, with_compression); - let mut errors = DivergenceErrors::default(); - errors.check_results_match(&main_result.1, &shadow_result.1); - errors - .into_result() - .with_context(|| { - format!("inspecting transaction {tx_hash:?}, with_compression={with_compression:?}") - }) - .unwrap(); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_result = + shadow + .vm + .inspect_transaction_with_bytecode_compression((), tx, with_compression); + let mut errors = DivergenceErrors::default(); + errors.check_results_match(&main_result.1, &shadow_result.1); + if let Err(err) = errors.into_result() { + let ctx = format!( + "inspecting transaction {tx_hash:?}, with_compression={with_compression:?}" + ); + self.shadow.take().unwrap().report(err.context(ctx)); + } + } main_result } @@ -173,40 +229,49 @@ where fn gas_remaining(&self) -> u32 { let main_gas = self.main.gas_remaining(); - let shadow_gas = self.shadow.gas_remaining(); - DivergenceErrors::single("gas_remaining", &main_gas, &shadow_gas).unwrap(); + if let Some(shadow) = &*self.shadow.borrow() { + let shadow_gas = shadow.vm.gas_remaining(); + let result = DivergenceErrors::single("gas_remaining", &main_gas, &shadow_gas); + if let Err(err) = result { + self.shadow.take().unwrap().report(err); + } + } main_gas } fn finish_batch(&mut self) -> FinishedL1Batch { let main_batch = self.main.finish_batch(); - let shadow_batch = self.shadow.finish_batch(); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_batch = shadow.vm.finish_batch(); + let mut errors = DivergenceErrors::default(); + errors.check_results_match( + &main_batch.block_tip_execution_result, + &shadow_batch.block_tip_execution_result, + ); + errors.check_final_states_match( + &main_batch.final_execution_state, + &shadow_batch.final_execution_state, + ); + errors.check_match( + "final_bootloader_memory", + &main_batch.final_bootloader_memory, + &shadow_batch.final_bootloader_memory, + ); + errors.check_match( + "pubdata_input", + &main_batch.pubdata_input, + &shadow_batch.pubdata_input, + ); + errors.check_match( + "state_diffs", + &main_batch.state_diffs, + &shadow_batch.state_diffs, + ); - let mut errors = DivergenceErrors::default(); - errors.check_results_match( - &main_batch.block_tip_execution_result, - &shadow_batch.block_tip_execution_result, - ); - errors.check_final_states_match( - &main_batch.final_execution_state, - &shadow_batch.final_execution_state, - ); - errors.check_match( - "final_bootloader_memory", - &main_batch.final_bootloader_memory, - &shadow_batch.final_bootloader_memory, - ); - errors.check_match( - "pubdata_input", - &main_batch.pubdata_input, - &shadow_batch.pubdata_input, - ); - errors.check_match( - "state_diffs", - &main_batch.state_diffs, - &shadow_batch.state_diffs, - ); - errors.into_result().unwrap(); + if let Err(err) = errors.into_result() { + self.shadow.take().unwrap().report(err); + } + } main_batch } } @@ -365,17 +430,23 @@ where T: VmInterfaceHistoryEnabled, { fn make_snapshot(&mut self) { - self.shadow.make_snapshot(); + if let Some(shadow) = self.shadow.get_mut() { + shadow.vm.make_snapshot(); + } self.main.make_snapshot(); } fn rollback_to_the_latest_snapshot(&mut self) { - self.shadow.rollback_to_the_latest_snapshot(); + if let Some(shadow) = self.shadow.get_mut() { + shadow.vm.rollback_to_the_latest_snapshot(); + } self.main.rollback_to_the_latest_snapshot(); } fn pop_snapshot_no_rollback(&mut self) { - self.shadow.pop_snapshot_no_rollback(); + if let Some(shadow) = self.shadow.get_mut() { + shadow.vm.pop_snapshot_no_rollback(); + } self.main.pop_snapshot_no_rollback(); } } From 1abd71f68bb208eecb304765de6a9ed6d6b7acc8 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 21 Aug 2024 15:02:41 +0300 Subject: [PATCH 02/24] Sketch VM state dumping --- Cargo.lock | 2 + core/lib/basic_types/src/vm.rs | 4 + core/lib/multivm/Cargo.toml | 2 + core/lib/multivm/src/versions/shadow.rs | 128 ++++++++++++++++-- core/lib/multivm/src/vm_instance.rs | 6 +- core/lib/protobuf_config/src/experimental.rs | 2 + .../src/proto/config/experimental.proto | 1 + .../src/batch_executor/main_executor.rs | 17 ++- prover/Cargo.lock | 2 + 9 files changed, 150 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 856e9f20a6ed..6f3c84e0153e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8937,6 +8937,8 @@ dependencies = [ "itertools 0.10.5", "once_cell", "pretty_assertions", + "serde", + "serde_json", "thiserror", "tokio", "tracing", diff --git a/core/lib/basic_types/src/vm.rs b/core/lib/basic_types/src/vm.rs index c178c853b2dc..a1782a3199b9 100644 --- a/core/lib/basic_types/src/vm.rs +++ b/core/lib/basic_types/src/vm.rs @@ -35,5 +35,9 @@ pub enum FastVmMode { /// Run only the new Vm. New, /// Run both the new and old VM and compare their outputs for each transaction execution. + /// The VM will panic on divergence. Shadow, + /// Run both the new and old VM and compare their outputs for each transaction execution. + /// The new VM will get dropped on divergence. + ShadowLenient, } diff --git a/core/lib/multivm/Cargo.toml b/core/lib/multivm/Cargo.toml index a245acdfacf6..f0b611c9a65c 100644 --- a/core/lib/multivm/Cargo.toml +++ b/core/lib/multivm/Cargo.toml @@ -38,6 +38,8 @@ pretty_assertions.workspace = true thiserror.workspace = true tracing.workspace = true vise.workspace = true +serde.workspace = true +serde_json.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["time"] } diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index ce557b750fc3..7f7aa2086630 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,10 +1,13 @@ use std::{ cell::RefCell, - collections::{BTreeMap, HashSet}, - fmt, + collections::{BTreeMap, HashMap, HashSet}, + fmt, fs, io, + path::PathBuf, + time::SystemTime, }; -use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; +use serde::Serialize; +use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, H256}; use crate::{ interface::{ @@ -17,9 +20,42 @@ use crate::{ vm_fast, }; +#[derive(Debug, Serialize)] +enum BlockOrTransaction { + Block(L2BlockEnv), + Transaction(Box), +} + +#[derive(Debug, Serialize)] +struct VmStateDump { + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + blocks_and_transactions: Vec, + read_storage_keys: HashMap, +} + +impl VmStateDump { + fn new(l1_batch_env: L1BatchEnv, system_env: SystemEnv) -> Self { + Self { + l1_batch_env, + system_env, + blocks_and_transactions: vec![], + read_storage_keys: HashMap::new(), + } + } + + fn push_transaction(&mut self, tx: Transaction) { + let tx = BlockOrTransaction::Transaction(Box::new(tx)); + self.blocks_and_transactions.push(tx); + } +} + struct VmWithReporting { vm: vm_fast::Vm>, - reporter: Box, + storage: StoragePtr>, + partial_dump: VmStateDump, + dumps_directory: Option, + panic_on_divergence: bool, } impl fmt::Debug for VmWithReporting { @@ -27,13 +63,50 @@ impl fmt::Debug for VmWithReporting { formatter .debug_struct("VmWithReporting") .field("vm", &self.vm) + .field("dumps_directory", &self.dumps_directory) + .field("panic_on_divergence", &self.panic_on_divergence) .finish_non_exhaustive() } } impl VmWithReporting { - fn report(mut self, err: anyhow::Error) { - (self.reporter)(err); + fn report(self, err: anyhow::Error) { + let mut dump = self.partial_dump; + let batch_number = dump.l1_batch_env.number.0; + tracing::error!("VM execution diverged on batch #{batch_number}!"); + + let read_keys = self.storage.borrow().cache().read_storage_keys(); + dump.read_storage_keys = read_keys + .into_iter() + .filter(|(_, value)| !value.is_zero()) + .map(|(key, value)| (key.hashed_key(), value)) + .collect(); + + if let Some(dumps_directory) = self.dumps_directory { + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("bogus clock"); + let timestamp = timestamp.as_millis(); + let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); + let dump_filename = dumps_directory.join(dump_filename); + tracing::info!("Dumping VM state to file `{}`", dump_filename.display()); + + let writer = fs::File::create(&dump_filename).expect("failed creating dump file"); + let writer = io::BufWriter::new(writer); + serde_json::to_writer(writer, &dump).expect("failed dumping VM state to file"); + } else { + let json = serde_json::to_string(&dump).expect("failed dumping VM state to string"); + tracing::error!("VM state: {json}"); + } + + if self.panic_on_divergence { + panic!("{err:?}"); + } else { + tracing::error!("{err:?}"); + tracing::warn!( + "New VM is dropped; following VM actions will be executed on the main VM only" + ); + } } } @@ -43,6 +116,24 @@ pub struct ShadowVm { shadow: RefCell>>, } +impl ShadowVm +where + S: ReadStorage, + T: VmInterface, +{ + pub fn set_dumps_directory(&mut self, dir: PathBuf) { + if let Some(shadow) = self.shadow.get_mut() { + shadow.dumps_directory = Some(dir); + } + } + + pub(crate) fn set_panic_on_divergence(&mut self, panic: bool) { + if let Some(shadow) = self.shadow.get_mut() { + shadow.panic_on_divergence = panic; + } + } +} + impl VmFactory> for ShadowVm where S: ReadStorage, @@ -54,13 +145,21 @@ where storage: StoragePtr>, ) -> Self { let main = T::new(batch_env.clone(), system_env.clone(), storage.clone()); - let shadow = vm_fast::Vm::new(batch_env, system_env, ImmutableStorageView::new(storage)); + let shadow = vm_fast::Vm::new( + batch_env.clone(), + system_env.clone(), + ImmutableStorageView::new(storage.clone()), + ); + let shadow = VmWithReporting { + vm: shadow, + storage, + partial_dump: VmStateDump::new(batch_env, system_env), + dumps_directory: None, + panic_on_divergence: true, + }; Self { main, - shadow: RefCell::new(Some(VmWithReporting { - vm: shadow, - reporter: Box::new(|err| panic!("{err:?}")), - })), + shadow: RefCell::new(Some(shadow)), } } } @@ -74,6 +173,7 @@ where fn push_transaction(&mut self, tx: Transaction) { if let Some(shadow) = self.shadow.get_mut() { + shadow.partial_dump.push_transaction(tx.clone()); shadow.vm.push_transaction(tx.clone()); } self.main.push_transaction(tx); @@ -146,6 +246,10 @@ where fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { self.main.start_new_l2_block(l2_block_env); if let Some(shadow) = self.shadow.get_mut() { + shadow + .partial_dump + .blocks_and_transactions + .push(BlockOrTransaction::Block(l2_block_env)); shadow.vm.start_new_l2_block(l2_block_env); } } @@ -176,6 +280,7 @@ where .main .execute_transaction_with_bytecode_compression(tx.clone(), with_compression); if let Some(shadow) = self.shadow.get_mut() { + shadow.partial_dump.push_transaction(tx.clone()); let shadow_result = shadow .vm .execute_transaction_with_bytecode_compression(tx, with_compression); @@ -207,6 +312,7 @@ where with_compression, ); if let Some(shadow) = self.shadow.get_mut() { + shadow.partial_dump.push_transaction(tx.clone()); let shadow_result = shadow .vm diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 0e4cefd3c808..e2cb6d9a1e2c 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -263,8 +263,10 @@ impl VmInstance { let storage = ImmutableStorageView::new(storage_view); Self::VmFast(crate::vm_fast::Vm::new(l1_batch_env, system_env, storage)) } - FastVmMode::Shadow => { - Self::ShadowedVmFast(ShadowVm::new(l1_batch_env, system_env, storage_view)) + FastVmMode::Shadow | FastVmMode::ShadowLenient => { + let mut vm = ShadowVm::new(l1_batch_env, system_env, storage_view); + vm.set_panic_on_divergence(matches!(mode, FastVmMode::Shadow)); + Self::ShadowedVmFast(vm) } }, _ => Self::new(l1_batch_env, system_env, storage_view), diff --git a/core/lib/protobuf_config/src/experimental.rs b/core/lib/protobuf_config/src/experimental.rs index cb959e229047..e63c4b0d929e 100644 --- a/core/lib/protobuf_config/src/experimental.rs +++ b/core/lib/protobuf_config/src/experimental.rs @@ -57,6 +57,7 @@ impl proto::FastVmMode { FastVmMode::Old => Self::Old, FastVmMode::New => Self::New, FastVmMode::Shadow => Self::Shadow, + FastVmMode::ShadowLenient => Self::ShadowLenient, } } @@ -65,6 +66,7 @@ impl proto::FastVmMode { Self::Old => FastVmMode::Old, Self::New => FastVmMode::New, Self::Shadow => FastVmMode::Shadow, + Self::ShadowLenient => FastVmMode::ShadowLenient, } } } diff --git a/core/lib/protobuf_config/src/proto/config/experimental.proto b/core/lib/protobuf_config/src/proto/config/experimental.proto index 1682b2c9a834..f8a72d3a2c6a 100644 --- a/core/lib/protobuf_config/src/proto/config/experimental.proto +++ b/core/lib/protobuf_config/src/proto/config/experimental.proto @@ -23,6 +23,7 @@ enum FastVmMode { OLD = 0; NEW = 1; SHADOW = 2; + SHADOW_LENIENT = 3; } // Experimental VM configuration diff --git a/core/node/state_keeper/src/batch_executor/main_executor.rs b/core/node/state_keeper/src/batch_executor/main_executor.rs index db4daeb77444..a8df949915c6 100644 --- a/core/node/state_keeper/src/batch_executor/main_executor.rs +++ b/core/node/state_keeper/src/batch_executor/main_executor.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{path::PathBuf, sync::Arc}; use anyhow::Context as _; use once_cell::sync::OnceCell; @@ -35,6 +35,7 @@ pub struct MainBatchExecutor { /// regardless of its configuration, this flag should be set to `true`. optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, + dumps_directory: Option, } impl MainBatchExecutor { @@ -43,6 +44,7 @@ impl MainBatchExecutor { save_call_traces, optional_bytecode_compression, fast_vm_mode: FastVmMode::Old, + dumps_directory: None, } } @@ -54,6 +56,11 @@ impl MainBatchExecutor { } self.fast_vm_mode = fast_vm_mode; } + + pub fn set_dumps_directory(&mut self, dir: PathBuf) { + tracing::info!("Set VM dumps directory: {}", dir.display()); + self.dumps_directory = Some(dir); + } } impl BatchExecutor for MainBatchExecutor { @@ -70,6 +77,7 @@ impl BatchExecutor for MainBatchExecutor { save_call_traces: self.save_call_traces, optional_bytecode_compression: self.optional_bytecode_compression, fast_vm_mode: self.fast_vm_mode, + dumps_directory: self.dumps_directory.clone(), commands: commands_receiver, }; @@ -97,6 +105,7 @@ struct CommandReceiver { save_call_traces: bool, optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, + dumps_directory: Option, commands: mpsc::Receiver, } @@ -117,6 +126,12 @@ impl CommandReceiver { self.fast_vm_mode, ); + if let VmInstance::ShadowedVmFast(vm) = &mut vm { + if let Some(dir) = self.dumps_directory.take() { + vm.set_dumps_directory(dir); + } + } + while let Some(cmd) = self.commands.blocking_recv() { match cmd { Command::ExecuteTx(tx, resp) => { diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 8268b121847c..4e5651fde48b 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -7959,6 +7959,8 @@ dependencies = [ "itertools 0.10.5", "once_cell", "pretty_assertions", + "serde", + "serde_json", "thiserror", "tracing", "vise", From a07eddcf064fa6211d8b45a24d99a198d8fa66ab Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 21 Aug 2024 15:08:21 +0300 Subject: [PATCH 03/24] Fix contract hashes diff --- core/lib/multivm/src/versions/shadow.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index 7f7aa2086630..b215b0e2671c 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,6 +1,6 @@ use std::{ cell::RefCell, - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashMap}, fmt, fs, io, path::PathBuf, time::SystemTime, @@ -460,8 +460,8 @@ impl DivergenceErrors { ); self.check_match( "final_state.used_contract_hashes", - &main.used_contract_hashes.iter().collect::>(), - &shadow.used_contract_hashes.iter().collect::>(), + &main.used_contract_hashes.iter().collect::>(), + &shadow.used_contract_hashes.iter().collect::>(), ); let main_deduplicated_logs = Self::gather_logs(&main.deduplicated_storage_logs); From f42faaf1566990deaee5d44a40350ba45bb397cf Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 21 Aug 2024 16:00:30 +0300 Subject: [PATCH 04/24] Set VM dumps dir in VM playground --- core/lib/multivm/src/versions/shadow.rs | 47 +++++++++++++-------- core/node/vm_runner/src/impls/playground.rs | 2 + 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index b215b0e2671c..63879d86cd19 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -2,10 +2,11 @@ use std::{ cell::RefCell, collections::{BTreeMap, BTreeSet, HashMap}, fmt, fs, io, - path::PathBuf, + path::{Path, PathBuf}, time::SystemTime, }; +use anyhow::Context as _; use serde::Serialize; use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, H256}; @@ -21,6 +22,7 @@ use crate::{ }; #[derive(Debug, Serialize)] +#[serde(rename_all = "snake_case")] enum BlockOrTransaction { Block(L2BlockEnv), Transaction(Box), @@ -72,7 +74,7 @@ impl fmt::Debug for VmWithReporting { impl VmWithReporting { fn report(self, err: anyhow::Error) { let mut dump = self.partial_dump; - let batch_number = dump.l1_batch_env.number.0; + let batch_number = dump.l1_batch_env.number; tracing::error!("VM execution diverged on batch #{batch_number}!"); let read_keys = self.storage.borrow().cache().read_storage_keys(); @@ -83,31 +85,40 @@ impl VmWithReporting { .collect(); if let Some(dumps_directory) = self.dumps_directory { - let timestamp = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .expect("bogus clock"); - let timestamp = timestamp.as_millis(); - let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); - let dump_filename = dumps_directory.join(dump_filename); - tracing::info!("Dumping VM state to file `{}`", dump_filename.display()); - - let writer = fs::File::create(&dump_filename).expect("failed creating dump file"); - let writer = io::BufWriter::new(writer); - serde_json::to_writer(writer, &dump).expect("failed dumping VM state to file"); - } else { - let json = serde_json::to_string(&dump).expect("failed dumping VM state to string"); - tracing::error!("VM state: {json}"); + if let Err(err) = Self::dump_to_file(&dumps_directory, &dump) { + tracing::warn!("Failed dumping VM state to file: {err:#}"); + } } + let json = serde_json::to_string(&dump).expect("failed dumping VM state to string"); + tracing::error!("VM state: {json}"); + if self.panic_on_divergence { panic!("{err:?}"); } else { - tracing::error!("{err:?}"); + tracing::error!("{err:#}"); tracing::warn!( - "New VM is dropped; following VM actions will be executed on the main VM only" + "New VM is dropped; following VM actions will be executed only on the main VM" ); } } + + fn dump_to_file(dumps_directory: &Path, dump: &VmStateDump) -> anyhow::Result<()> { + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("bogus clock"); + let timestamp = timestamp.as_millis(); + let batch_number = dump.l1_batch_env.number.0; + let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); + let dump_filename = dumps_directory.join(dump_filename); + tracing::info!("Dumping VM state to file `{}`", dump_filename.display()); + + fs::create_dir_all(dumps_directory).context("failed creating dumps directory")?; + let writer = fs::File::create(&dump_filename).context("failed creating dump file")?; + let writer = io::BufWriter::new(writer); + serde_json::to_writer(writer, &dump).context("failed dumping VM state to file")?; + Ok(()) + } } #[derive(Debug)] diff --git a/core/node/vm_runner/src/impls/playground.rs b/core/node/vm_runner/src/impls/playground.rs index 4fb140431df6..72a8e91ac3b3 100644 --- a/core/node/vm_runner/src/impls/playground.rs +++ b/core/node/vm_runner/src/impls/playground.rs @@ -75,6 +75,8 @@ impl VmPlayground { let mut batch_executor = MainBatchExecutor::new(false, false); batch_executor.set_fast_vm_mode(vm_mode); + let vm_dumps_dir = Path::new(&rocksdb_path).join("__vm_dumps"); + batch_executor.set_dumps_directory(vm_dumps_dir); let io = VmPlaygroundIo { cursor_file_path, From 743353c162e6a42b60fe4cb6d9d8f18e3ebd77e8 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 21 Aug 2024 17:27:39 +0300 Subject: [PATCH 05/24] Add other storage data to VM dump --- core/lib/multivm/src/versions/shadow.rs | 68 +++++++++++++++++++------ 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index 63879d86cd19..0c532219861b 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,6 +1,6 @@ use std::{ cell::RefCell, - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet}, fmt, fs, io, path::{Path, PathBuf}, time::SystemTime, @@ -8,7 +8,8 @@ use std::{ use anyhow::Context as _; use serde::Serialize; -use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, H256}; +use zksync_types::{web3, StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, H256}; +use zksync_utils::u256_to_h256; use crate::{ interface::{ @@ -30,10 +31,15 @@ enum BlockOrTransaction { #[derive(Debug, Serialize)] struct VmStateDump { + // VM inputs l1_batch_env: L1BatchEnv, system_env: SystemEnv, blocks_and_transactions: Vec, + // Storage information (read slots and factory deps, `is_initial_write` oracle) read_storage_keys: HashMap, + initial_writes: HashSet, + repeated_writes: HashSet, + factory_deps: HashMap, } impl VmStateDump { @@ -43,6 +49,9 @@ impl VmStateDump { system_env, blocks_and_transactions: vec![], read_storage_keys: HashMap::new(), + initial_writes: HashSet::new(), + repeated_writes: HashSet::new(), + factory_deps: HashMap::new(), } } @@ -71,18 +80,37 @@ impl fmt::Debug for VmWithReporting { } } -impl VmWithReporting { - fn report(self, err: anyhow::Error) { +impl VmWithReporting { + fn report(self, main_vm: &impl VmInterface, err: anyhow::Error) { let mut dump = self.partial_dump; let batch_number = dump.l1_batch_env.number; tracing::error!("VM execution diverged on batch #{batch_number}!"); - let read_keys = self.storage.borrow().cache().read_storage_keys(); - dump.read_storage_keys = read_keys + let storage_cache = self.storage.borrow().cache(); + dump.read_storage_keys = storage_cache + .read_storage_keys() .into_iter() .filter(|(_, value)| !value.is_zero()) .map(|(key, value)| (key.hashed_key(), value)) .collect(); + for (key, is_initial) in storage_cache.initial_writes() { + if is_initial { + dump.initial_writes.insert(key.hashed_key()); + } else { + dump.repeated_writes.insert(key.hashed_key()); + } + } + + let used_contract_hashes = main_vm.get_current_execution_state().used_contract_hashes; + let mut storage = self.storage.borrow_mut(); + dump.factory_deps = used_contract_hashes + .into_iter() + .filter_map(|hash| { + let hash = u256_to_h256(hash); + Some((hash, web3::Bytes(storage.load_factory_dep(hash)?))) + }) + .collect(); + drop(storage); if let Some(dumps_directory) = self.dumps_directory { if let Err(err) = Self::dump_to_file(&dumps_directory, &dump) { @@ -198,7 +226,10 @@ where errors.check_results_match(&main_result, &shadow_result); if let Err(err) = errors.into_result() { let ctx = format!("executing VM with mode {execution_mode:?}"); - self.shadow.take().unwrap().report(err.context(ctx)); + self.shadow + .take() + .unwrap() + .report(&self.main, err.context(ctx)); } } main_result @@ -216,10 +247,11 @@ where errors.check_results_match(&main_result, &shadow_result); if let Err(err) = errors.into_result() { + let ctx = format!("executing VM with mode {execution_mode:?}"); self.shadow .take() .unwrap() - .report(err.context(format!("executing VM with mode {execution_mode:?}"))); + .report(&self.main, err.context(ctx)); } } main_result @@ -232,7 +264,7 @@ where let result = DivergenceErrors::single("get_bootloader_memory", &main_memory, &shadow_memory); if let Err(err) = result { - self.shadow.take().unwrap().report(err); + self.shadow.take().unwrap().report(&self.main, err); } } main_memory @@ -248,7 +280,7 @@ where &shadow_bytecodes, ); if let Err(err) = result { - self.shadow.take().unwrap().report(err); + self.shadow.take().unwrap().report(&self.main, err); } } main_bytecodes @@ -272,7 +304,7 @@ where let result = DivergenceErrors::single("get_current_execution_state", &main_state, &shadow_state); if let Err(err) = result { - self.shadow.take().unwrap().report(err); + self.shadow.take().unwrap().report(&self.main, err); } } main_state @@ -301,7 +333,10 @@ where let ctx = format!( "executing transaction {tx_hash:?}, with_compression={with_compression:?}" ); - self.shadow.take().unwrap().report(err.context(ctx)); + self.shadow + .take() + .unwrap() + .report(&self.main, err.context(ctx)); } } main_result @@ -334,7 +369,10 @@ where let ctx = format!( "inspecting transaction {tx_hash:?}, with_compression={with_compression:?}" ); - self.shadow.take().unwrap().report(err.context(ctx)); + self.shadow + .take() + .unwrap() + .report(&self.main, err.context(ctx)); } } main_result @@ -350,7 +388,7 @@ where let shadow_gas = shadow.vm.gas_remaining(); let result = DivergenceErrors::single("gas_remaining", &main_gas, &shadow_gas); if let Err(err) = result { - self.shadow.take().unwrap().report(err); + self.shadow.take().unwrap().report(&self.main, err); } } main_gas @@ -386,7 +424,7 @@ where ); if let Err(err) = errors.into_result() { - self.shadow.take().unwrap().report(err); + self.shadow.take().unwrap().report(&self.main, err); } } main_batch From 47e31cc676dbf8caaca686cdd067c5d9d49bdcae Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 09:28:51 +0300 Subject: [PATCH 06/24] Sketch GCS-based dump handler ...and brush up dumps --- Cargo.lock | 2 +- core/lib/multivm/Cargo.toml | 1 - core/lib/multivm/src/dump.rs | 127 ++++++++++++++++++ core/lib/multivm/src/lib.rs | 1 + core/lib/multivm/src/versions/shadow.rs | 119 +++------------- core/lib/object_store/src/file.rs | 1 + core/lib/object_store/src/objects.rs | 1 - core/lib/object_store/src/raw.rs | 2 + .../layers/vm_runner/playground.rs | 4 + .../src/batch_executor/main_executor.rs | 52 +++++-- .../state_keeper/src/batch_executor/mod.rs | 2 +- core/node/vm_runner/Cargo.toml | 1 + core/node/vm_runner/src/impls/playground.rs | 36 ++++- core/node/vm_runner/src/tests/playground.rs | 1 + prover/Cargo.lock | 1 - 15 files changed, 231 insertions(+), 120 deletions(-) create mode 100644 core/lib/multivm/src/dump.rs diff --git a/Cargo.lock b/Cargo.lock index 6f3c84e0153e..35b2bc4919c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8938,7 +8938,6 @@ dependencies = [ "once_cell", "pretty_assertions", "serde", - "serde_json", "thiserror", "tokio", "tracing", @@ -9789,6 +9788,7 @@ dependencies = [ "once_cell", "rand 0.8.5", "serde", + "serde_json", "tempfile", "test-casing", "tokio", diff --git a/core/lib/multivm/Cargo.toml b/core/lib/multivm/Cargo.toml index f0b611c9a65c..b50f35475162 100644 --- a/core/lib/multivm/Cargo.toml +++ b/core/lib/multivm/Cargo.toml @@ -39,7 +39,6 @@ thiserror.workspace = true tracing.workspace = true vise.workspace = true serde.workspace = true -serde_json.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["time"] } diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs new file mode 100644 index 000000000000..e21091eb1f84 --- /dev/null +++ b/core/lib/multivm/src/dump.rs @@ -0,0 +1,127 @@ +use std::{collections::HashMap, sync::Arc}; + +use serde::{Deserialize, Serialize}; +use zksync_types::{web3, L1BatchNumber, Transaction, H256}; +use zksync_utils::u256_to_h256; +use zksync_vm_interface::{ + storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, + L1BatchEnv, L2BlockEnv, SystemEnv, VmInterface, +}; + +/// Handler for [`VmDump`]. +pub type VmDumpHandler = Arc; + +/// Part of the VM dump representing the storage oracle. +#[derive(Debug, Default, Serialize, Deserialize)] +pub(crate) struct VmStorageDump { + read_storage_keys: HashMap, + repeated_writes: HashMap, + factory_deps: HashMap, +} + +impl VmStorageDump { + /// Storage must be the one used by the VM. + pub(crate) fn new( + storage: &StoragePtr>, + vm: &impl VmInterface, + ) -> Self { + let mut storage = storage.borrow_mut(); + let storage_cache = storage.cache(); + let read_storage_keys: HashMap<_, _> = storage_cache + .read_storage_keys() + .into_iter() + .filter_map(|(key, value)| { + let enum_index = storage.get_enumeration_index(&key)?; + Some((key.hashed_key(), (value, enum_index))) + }) + .collect(); + let repeated_writes = storage_cache + .initial_writes() + .into_iter() + .filter_map(|(key, is_initial)| { + let hashed_key = key.hashed_key(); + if !is_initial && !read_storage_keys.contains_key(&hashed_key) { + let enum_index = storage.get_enumeration_index(&key)?; + Some((hashed_key, enum_index)) + } else { + None + } + }) + .collect(); + + let used_contract_hashes = vm.get_current_execution_state().used_contract_hashes; + let factory_deps = used_contract_hashes + .into_iter() + .filter_map(|hash| { + let hash = u256_to_h256(hash); + Some((hash, web3::Bytes(storage.load_factory_dep(hash)?))) + }) + .collect(); + Self { + read_storage_keys, + repeated_writes, + factory_deps, + } + } + + #[allow(dead_code)] // FIXME + pub(crate) fn into_storage(self) -> InMemoryStorage { + let mut storage = InMemoryStorage::default(); + for (key, (value, enum_index)) in self.read_storage_keys { + storage.set_value_hashed_enum(key, enum_index, value); + } + for (key, enum_index) in self.repeated_writes { + // The value shouldn't be read by the VM, so it doesn't matter. + storage.set_value_hashed_enum(key, enum_index, H256::zero()); + } + for (hash, bytecode) in self.factory_deps { + storage.store_factory_dep(hash, bytecode.0); + } + storage + } +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum VmAction { + Block(L2BlockEnv), + Transaction(Box), +} + +/// VM dump allowing to re-run the VM on the same inputs. Opaque, but can be (de)serialized. +#[derive(Debug, Serialize, Deserialize)] +pub struct VmDump { + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + actions: Vec, + #[serde(flatten)] + storage: VmStorageDump, +} + +impl VmDump { + pub(crate) fn new(l1_batch_env: L1BatchEnv, system_env: SystemEnv) -> Self { + Self { + l1_batch_env, + system_env, + actions: vec![], + storage: VmStorageDump::default(), + } + } + + pub fn l1_batch_number(&self) -> L1BatchNumber { + self.l1_batch_env.number + } + + pub(crate) fn push_transaction(&mut self, tx: Transaction) { + let tx = VmAction::Transaction(Box::new(tx)); + self.actions.push(tx); + } + + pub(crate) fn push_block(&mut self, block: L2BlockEnv) { + self.actions.push(VmAction::Block(block)); + } + + pub(crate) fn set_storage(&mut self, storage: VmStorageDump) { + self.storage = storage; + } +} diff --git a/core/lib/multivm/src/lib.rs b/core/lib/multivm/src/lib.rs index 77851a1df002..0a6bfc949356 100644 --- a/core/lib/multivm/src/lib.rs +++ b/core/lib/multivm/src/lib.rs @@ -19,6 +19,7 @@ pub use crate::{ vm_instance::VmInstance, }; +pub mod dump; mod glue; pub mod tracers; pub mod utils; diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index 0c532219861b..1fed67625531 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,17 +1,14 @@ use std::{ cell::RefCell, - collections::{BTreeMap, BTreeSet, HashMap, HashSet}, - fmt, fs, io, - path::{Path, PathBuf}, - time::SystemTime, + collections::{BTreeMap, BTreeSet}, + fmt, + sync::Arc, }; -use anyhow::Context as _; -use serde::Serialize; -use zksync_types::{web3, StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, H256}; -use zksync_utils::u256_to_h256; +use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; use crate::{ + dump::{VmDump, VmDumpHandler, VmStorageDump}, interface::{ storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, @@ -22,50 +19,11 @@ use crate::{ vm_fast, }; -#[derive(Debug, Serialize)] -#[serde(rename_all = "snake_case")] -enum BlockOrTransaction { - Block(L2BlockEnv), - Transaction(Box), -} - -#[derive(Debug, Serialize)] -struct VmStateDump { - // VM inputs - l1_batch_env: L1BatchEnv, - system_env: SystemEnv, - blocks_and_transactions: Vec, - // Storage information (read slots and factory deps, `is_initial_write` oracle) - read_storage_keys: HashMap, - initial_writes: HashSet, - repeated_writes: HashSet, - factory_deps: HashMap, -} - -impl VmStateDump { - fn new(l1_batch_env: L1BatchEnv, system_env: SystemEnv) -> Self { - Self { - l1_batch_env, - system_env, - blocks_and_transactions: vec![], - read_storage_keys: HashMap::new(), - initial_writes: HashSet::new(), - repeated_writes: HashSet::new(), - factory_deps: HashMap::new(), - } - } - - fn push_transaction(&mut self, tx: Transaction) { - let tx = BlockOrTransaction::Transaction(Box::new(tx)); - self.blocks_and_transactions.push(tx); - } -} - struct VmWithReporting { vm: vm_fast::Vm>, storage: StoragePtr>, - partial_dump: VmStateDump, - dumps_directory: Option, + partial_dump: VmDump, + dump_handler: VmDumpHandler, panic_on_divergence: bool, } @@ -74,7 +32,6 @@ impl fmt::Debug for VmWithReporting { formatter .debug_struct("VmWithReporting") .field("vm", &self.vm) - .field("dumps_directory", &self.dumps_directory) .field("panic_on_divergence", &self.panic_on_divergence) .finish_non_exhaustive() } @@ -83,43 +40,10 @@ impl fmt::Debug for VmWithReporting { impl VmWithReporting { fn report(self, main_vm: &impl VmInterface, err: anyhow::Error) { let mut dump = self.partial_dump; - let batch_number = dump.l1_batch_env.number; + let batch_number = dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - - let storage_cache = self.storage.borrow().cache(); - dump.read_storage_keys = storage_cache - .read_storage_keys() - .into_iter() - .filter(|(_, value)| !value.is_zero()) - .map(|(key, value)| (key.hashed_key(), value)) - .collect(); - for (key, is_initial) in storage_cache.initial_writes() { - if is_initial { - dump.initial_writes.insert(key.hashed_key()); - } else { - dump.repeated_writes.insert(key.hashed_key()); - } - } - - let used_contract_hashes = main_vm.get_current_execution_state().used_contract_hashes; - let mut storage = self.storage.borrow_mut(); - dump.factory_deps = used_contract_hashes - .into_iter() - .filter_map(|hash| { - let hash = u256_to_h256(hash); - Some((hash, web3::Bytes(storage.load_factory_dep(hash)?))) - }) - .collect(); - drop(storage); - - if let Some(dumps_directory) = self.dumps_directory { - if let Err(err) = Self::dump_to_file(&dumps_directory, &dump) { - tracing::warn!("Failed dumping VM state to file: {err:#}"); - } - } - - let json = serde_json::to_string(&dump).expect("failed dumping VM state to string"); - tracing::error!("VM state: {json}"); + dump.set_storage(VmStorageDump::new(&self.storage, main_vm)); + (self.dump_handler)(dump); if self.panic_on_divergence { panic!("{err:?}"); @@ -131,13 +55,9 @@ impl VmWithReporting { } } + /* fn dump_to_file(dumps_directory: &Path, dump: &VmStateDump) -> anyhow::Result<()> { - let timestamp = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .expect("bogus clock"); - let timestamp = timestamp.as_millis(); - let batch_number = dump.l1_batch_env.number.0; - let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); + let dump_filename = dumps_directory.join(dump_filename); tracing::info!("Dumping VM state to file `{}`", dump_filename.display()); @@ -146,7 +66,7 @@ impl VmWithReporting { let writer = io::BufWriter::new(writer); serde_json::to_writer(writer, &dump).context("failed dumping VM state to file")?; Ok(()) - } + }*/ } #[derive(Debug)] @@ -160,9 +80,9 @@ where S: ReadStorage, T: VmInterface, { - pub fn set_dumps_directory(&mut self, dir: PathBuf) { + pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { if let Some(shadow) = self.shadow.get_mut() { - shadow.dumps_directory = Some(dir); + shadow.dump_handler = handler; } } @@ -192,8 +112,8 @@ where let shadow = VmWithReporting { vm: shadow, storage, - partial_dump: VmStateDump::new(batch_env, system_env), - dumps_directory: None, + partial_dump: VmDump::new(batch_env, system_env), + dump_handler: Arc::new(drop), // We don't want to log the dump (it's too large), so there's no trivial way to handle it panic_on_divergence: true, }; Self { @@ -289,10 +209,7 @@ where fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { self.main.start_new_l2_block(l2_block_env); if let Some(shadow) = self.shadow.get_mut() { - shadow - .partial_dump - .blocks_and_transactions - .push(BlockOrTransaction::Block(l2_block_env)); + shadow.partial_dump.push_block(l2_block_env); shadow.vm.start_new_l2_block(l2_block_env); } } diff --git a/core/lib/object_store/src/file.rs b/core/lib/object_store/src/file.rs index decba534d23e..308cd65427fb 100644 --- a/core/lib/object_store/src/file.rs +++ b/core/lib/object_store/src/file.rs @@ -43,6 +43,7 @@ impl FileBackedObjectStore { Bucket::ProofsFri, Bucket::StorageSnapshot, Bucket::TeeVerifierInput, + Bucket::VmDumps, ] { let bucket_path = format!("{base_dir}/{bucket}"); fs::create_dir_all(&bucket_path).await?; diff --git a/core/lib/object_store/src/objects.rs b/core/lib/object_store/src/objects.rs index f5bb3706d9d4..ff5fae2a81f0 100644 --- a/core/lib/object_store/src/objects.rs +++ b/core/lib/object_store/src/objects.rs @@ -95,7 +95,6 @@ where type Key<'a> = SnapshotStorageLogsStorageKey; fn encode_key(key: Self::Key<'_>) -> String { - // FIXME: should keys be separated by version? format!( "snapshot_l1_batch_{}_storage_logs_part_{:0>4}.proto.gzip", key.l1_batch_number, key.chunk_id diff --git a/core/lib/object_store/src/raw.rs b/core/lib/object_store/src/raw.rs index 3c5a89f160a5..740e8d76e246 100644 --- a/core/lib/object_store/src/raw.rs +++ b/core/lib/object_store/src/raw.rs @@ -20,6 +20,7 @@ pub enum Bucket { StorageSnapshot, DataAvailability, TeeVerifierInput, + VmDumps, } impl Bucket { @@ -39,6 +40,7 @@ impl Bucket { Self::StorageSnapshot => "storage_logs_snapshots", Self::DataAvailability => "data_availability", Self::TeeVerifierInput => "tee_verifier_inputs", + Self::VmDumps => "vm_dumps", } } } diff --git a/core/node/node_framework/src/implementations/layers/vm_runner/playground.rs b/core/node/node_framework/src/implementations/layers/vm_runner/playground.rs index 810d538ba978..c5457d5e19a4 100644 --- a/core/node/node_framework/src/implementations/layers/vm_runner/playground.rs +++ b/core/node/node_framework/src/implementations/layers/vm_runner/playground.rs @@ -10,6 +10,7 @@ use zksync_vm_runner::{ use crate::{ implementations::resources::{ healthcheck::AppHealthCheckResource, + object_store::ObjectStoreResource, pools::{MasterPool, PoolResource}, }, StopReceiver, Task, TaskId, WiringError, WiringLayer, @@ -34,6 +35,7 @@ impl VmPlaygroundLayer { #[context(crate = crate)] pub struct Input { pub master_pool: PoolResource, + pub dumps_object_store: Option, #[context(default)] pub app_health: AppHealthCheckResource, } @@ -61,6 +63,7 @@ impl WiringLayer for VmPlaygroundLayer { async fn wire(self, input: Self::Input) -> Result { let Input { master_pool, + dumps_object_store, app_health, } = input; @@ -73,6 +76,7 @@ impl WiringLayer for VmPlaygroundLayer { let (playground, tasks) = VmPlayground::new( connection_pool, + dumps_object_store.map(|resource| resource.0), self.config.fast_vm_mode, self.config.db_path, self.zksync_network_id, diff --git a/core/node/state_keeper/src/batch_executor/main_executor.rs b/core/node/state_keeper/src/batch_executor/main_executor.rs index a8df949915c6..31d920abdc3d 100644 --- a/core/node/state_keeper/src/batch_executor/main_executor.rs +++ b/core/node/state_keeper/src/batch_executor/main_executor.rs @@ -1,9 +1,10 @@ -use std::{path::PathBuf, sync::Arc}; +use std::{fmt, sync::Arc}; use anyhow::Context as _; use once_cell::sync::OnceCell; use tokio::sync::mpsc; use zksync_multivm::{ + dump::{VmDump, VmDumpHandler}, interface::{ storage::{ReadStorage, StorageView}, Call, CompressedBytecodeInfo, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, @@ -24,7 +25,7 @@ use crate::{ /// The default implementation of [`BatchExecutor`]. /// Creates a "real" batch executor which maintains the VM (as opposed to the test builder which doesn't use the VM). -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct MainBatchExecutor { save_call_traces: bool, /// Whether batch executor would allow transactions with bytecode that cannot be compressed. @@ -35,7 +36,21 @@ pub struct MainBatchExecutor { /// regardless of its configuration, this flag should be set to `true`. optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - dumps_directory: Option, + dump_handler: Option, +} + +impl fmt::Debug for MainBatchExecutor { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("MainBatchExecutor") + .field("save_call_traces", &self.save_call_traces) + .field( + "optional_bytecode_compression", + &self.optional_bytecode_compression, + ) + .field("fast_vm_mode", &self.fast_vm_mode) + .finish_non_exhaustive() + } } impl MainBatchExecutor { @@ -44,7 +59,7 @@ impl MainBatchExecutor { save_call_traces, optional_bytecode_compression, fast_vm_mode: FastVmMode::Old, - dumps_directory: None, + dump_handler: None, } } @@ -57,9 +72,9 @@ impl MainBatchExecutor { self.fast_vm_mode = fast_vm_mode; } - pub fn set_dumps_directory(&mut self, dir: PathBuf) { - tracing::info!("Set VM dumps directory: {}", dir.display()); - self.dumps_directory = Some(dir); + pub fn set_dump_handler(&mut self, handler: Arc) { + tracing::info!("Set VM dumps handler"); + self.dump_handler = Some(handler); } } @@ -77,7 +92,7 @@ impl BatchExecutor for MainBatchExecutor { save_call_traces: self.save_call_traces, optional_bytecode_compression: self.optional_bytecode_compression, fast_vm_mode: self.fast_vm_mode, - dumps_directory: self.dumps_directory.clone(), + dump_handler: self.dump_handler.clone(), commands: commands_receiver, }; @@ -100,15 +115,28 @@ struct TransactionOutput { /// /// One `CommandReceiver` can execute exactly one batch, so once the batch is sealed, a new `CommandReceiver` object must /// be constructed. -#[derive(Debug)] struct CommandReceiver { save_call_traces: bool, optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - dumps_directory: Option, + dump_handler: Option, commands: mpsc::Receiver, } +impl fmt::Debug for CommandReceiver { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("CommandReceiver") + .field("save_call_traces", &self.save_call_traces) + .field( + "optional_bytecode_compression", + &self.optional_bytecode_compression, + ) + .field("fast_vm_mode", &self.fast_vm_mode) + .finish_non_exhaustive() + } +} + impl CommandReceiver { pub(super) fn run( mut self, @@ -127,8 +155,8 @@ impl CommandReceiver { ); if let VmInstance::ShadowedVmFast(vm) = &mut vm { - if let Some(dir) = self.dumps_directory.take() { - vm.set_dumps_directory(dir); + if let Some(handler) = self.dump_handler.take() { + vm.set_dump_handler(handler); } } diff --git a/core/node/state_keeper/src/batch_executor/mod.rs b/core/node/state_keeper/src/batch_executor/mod.rs index 235a8f581c82..217417c80421 100644 --- a/core/node/state_keeper/src/batch_executor/mod.rs +++ b/core/node/state_keeper/src/batch_executor/mod.rs @@ -56,7 +56,7 @@ impl TxExecutionResult { /// by communicating with the externally initialized thread. /// /// This type is generic over the storage type accepted to create the VM instance, mostly for testing purposes. -pub trait BatchExecutor: 'static + Send + Sync + fmt::Debug { +pub trait BatchExecutor: 'static + Send + fmt::Debug { fn init_batch( &mut self, storage: S, diff --git a/core/node/vm_runner/Cargo.toml b/core/node/vm_runner/Cargo.toml index cc6313fa5727..574a1f52c8f5 100644 --- a/core/node/vm_runner/Cargo.toml +++ b/core/node/vm_runner/Cargo.toml @@ -25,6 +25,7 @@ zksync_vm_utils.workspace = true zksync_health_check.workspace = true serde.workspace = true +serde_json.workspace = true tokio = { workspace = true, features = ["time"] } anyhow.workspace = true async-trait.workspace = true diff --git a/core/node/vm_runner/src/impls/playground.rs b/core/node/vm_runner/src/impls/playground.rs index 72a8e91ac3b3..2dccba8b5f9b 100644 --- a/core/node/vm_runner/src/impls/playground.rs +++ b/core/node/vm_runner/src/impls/playground.rs @@ -2,6 +2,7 @@ use std::{ io, path::{Path, PathBuf}, sync::Arc, + time::SystemTime, }; use anyhow::Context as _; @@ -13,6 +14,8 @@ use tokio::{ }; use zksync_dal::{Connection, ConnectionPool, Core, CoreDal}; use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthCheck}; +use zksync_multivm::dump::VmDump; +use zksync_object_store::{Bucket, ObjectStore}; use zksync_state::RocksdbStorage; use zksync_state_keeper::{MainBatchExecutor, StateKeeperOutputHandler, UpdatesManager}; use zksync_types::{vm::FastVmMode, L1BatchNumber, L2ChainId}; @@ -53,6 +56,7 @@ impl VmPlayground { /// Creates a new playground. pub async fn new( pool: ConnectionPool, + dumps_object_store: Option>, vm_mode: FastVmMode, rocksdb_path: String, chain_id: L2ChainId, @@ -75,8 +79,19 @@ impl VmPlayground { let mut batch_executor = MainBatchExecutor::new(false, false); batch_executor.set_fast_vm_mode(vm_mode); - let vm_dumps_dir = Path::new(&rocksdb_path).join("__vm_dumps"); - batch_executor.set_dumps_directory(vm_dumps_dir); + let handle = tokio::runtime::Handle::current(); + if let Some(store) = dumps_object_store { + tracing::info!("Using object store for VM dumps: {store:?}"); + + batch_executor.set_dump_handler(Arc::new(move |dump| { + if let Err(err) = handle.block_on(Self::dump_vm_state(&*store, &dump)) { + let l1_batch_number = dump.l1_batch_number(); + tracing::error!( + "Saving VM dump for L1 batch #{l1_batch_number} failed: {err:#}" + ); + } + })); + } let io = VmPlaygroundIo { cursor_file_path, @@ -113,6 +128,23 @@ impl VmPlayground { )) } + async fn dump_vm_state(object_store: &dyn ObjectStore, dump: &VmDump) -> anyhow::Result<()> { + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("bogus clock"); + let timestamp = timestamp.as_millis(); + let batch_number = dump.l1_batch_number(); + let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); + + tracing::info!("Dumping diverged VM state to `{dump_filename}`"); + let dump = serde_json::to_string(&dump).context("failed serializing VM dump")?; + object_store + .put_raw(Bucket::VmDumps, &dump_filename, dump.into_bytes()) + .await + .context("failed putting VM dump to object store")?; + Ok(()) + } + /// Returns a health check for this component. pub fn health_check(&self) -> ReactiveHealthCheck { self.io.health_updater.subscribe() diff --git a/core/node/vm_runner/src/tests/playground.rs b/core/node/vm_runner/src/tests/playground.rs index c4111f737418..573bdf17d55c 100644 --- a/core/node/vm_runner/src/tests/playground.rs +++ b/core/node/vm_runner/src/tests/playground.rs @@ -35,6 +35,7 @@ async fn run_playground( let (playground, playground_tasks) = VmPlayground::new( pool.clone(), + None, FastVmMode::Shadow, rocksdb_dir.path().to_str().unwrap().to_owned(), genesis_params.config().l2_chain_id, diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 4e5651fde48b..aef896fff158 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -7960,7 +7960,6 @@ dependencies = [ "once_cell", "pretty_assertions", "serde", - "serde_json", "thiserror", "tracing", "vise", From b0ecc34c586a603b84da3151ba93f359d92a2215 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 09:46:39 +0300 Subject: [PATCH 07/24] Generalize shadowing --- core/lib/multivm/src/versions/shadow.rs | 72 ++++------ .../vm_fast/tests/tester/vm_tester.rs | 4 +- core/lib/multivm/src/versions/vm_fast/vm.rs | 133 ++++++++++-------- core/lib/multivm/src/vm_instance.rs | 6 +- core/tests/vm-benchmark/harness/src/lib.rs | 2 +- 5 files changed, 111 insertions(+), 106 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index 1fed67625531..708347910eb4 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -19,15 +19,15 @@ use crate::{ vm_fast, }; -struct VmWithReporting { - vm: vm_fast::Vm>, - storage: StoragePtr>, +struct VmWithReporting { + vm: Shadow, + main_vm_storage: StoragePtr>, partial_dump: VmDump, dump_handler: VmDumpHandler, panic_on_divergence: bool, } -impl fmt::Debug for VmWithReporting { +impl fmt::Debug for VmWithReporting { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter .debug_struct("VmWithReporting") @@ -37,12 +37,12 @@ impl fmt::Debug for VmWithReporting { } } -impl VmWithReporting { +impl VmWithReporting { fn report(self, main_vm: &impl VmInterface, err: anyhow::Error) { let mut dump = self.partial_dump; let batch_number = dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - dump.set_storage(VmStorageDump::new(&self.storage, main_vm)); + dump.set_storage(VmStorageDump::new(&self.main_vm_storage, main_vm)); (self.dump_handler)(dump); if self.panic_on_divergence { @@ -54,31 +54,19 @@ impl VmWithReporting { ); } } - - /* - fn dump_to_file(dumps_directory: &Path, dump: &VmStateDump) -> anyhow::Result<()> { - - let dump_filename = dumps_directory.join(dump_filename); - tracing::info!("Dumping VM state to file `{}`", dump_filename.display()); - - fs::create_dir_all(dumps_directory).context("failed creating dumps directory")?; - let writer = fs::File::create(&dump_filename).context("failed creating dump file")?; - let writer = io::BufWriter::new(writer); - serde_json::to_writer(writer, &dump).context("failed dumping VM state to file")?; - Ok(()) - }*/ } #[derive(Debug)] -pub struct ShadowVm { - main: T, - shadow: RefCell>>, +pub struct ShadowVm>> { + main: Main, + shadow: RefCell>>, } -impl ShadowVm +impl ShadowVm where S: ReadStorage, - T: VmInterface, + Main: VmInterface, + Shadow: VmInterface, { pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { if let Some(shadow) = self.shadow.get_mut() { @@ -93,25 +81,22 @@ where } } -impl VmFactory> for ShadowVm +impl VmFactory> for ShadowVm where S: ReadStorage, - T: VmFactory>, + Main: VmFactory>, + Shadow: VmFactory>, { fn new( batch_env: L1BatchEnv, system_env: SystemEnv, storage: StoragePtr>, ) -> Self { - let main = T::new(batch_env.clone(), system_env.clone(), storage.clone()); - let shadow = vm_fast::Vm::new( - batch_env.clone(), - system_env.clone(), - ImmutableStorageView::new(storage.clone()), - ); + let main = Main::new(batch_env.clone(), system_env.clone(), storage.clone()); + let shadow = Shadow::new(batch_env.clone(), system_env.clone(), storage.clone()); let shadow = VmWithReporting { vm: shadow, - storage, + main_vm_storage: storage, partial_dump: VmDump::new(batch_env, system_env), dump_handler: Arc::new(drop), // We don't want to log the dump (it's too large), so there's no trivial way to handle it panic_on_divergence: true, @@ -123,12 +108,14 @@ where } } -impl VmInterface for ShadowVm +/// **Important.** This doesn't properly handle tracers; they are not passed to the shadow VM! +impl VmInterface for ShadowVm where S: ReadStorage, - T: VmInterface, + Main: VmInterface, + Shadow: VmInterface, { - type TracerDispatcher = T::TracerDispatcher; + type TracerDispatcher = Main::TracerDispatcher; fn push_transaction(&mut self, tx: Transaction) { if let Some(shadow) = self.shadow.get_mut() { @@ -162,7 +149,9 @@ where ) -> VmExecutionResultAndLogs { let main_result = self.main.inspect(dispatcher, execution_mode); if let Some(shadow) = self.shadow.get_mut() { - let shadow_result = shadow.vm.inspect((), execution_mode); + let shadow_result = shadow + .vm + .inspect(Shadow::TracerDispatcher::default(), execution_mode); let mut errors = DivergenceErrors::default(); errors.check_results_match(&main_result, &shadow_result); @@ -276,10 +265,11 @@ where ); if let Some(shadow) = self.shadow.get_mut() { shadow.partial_dump.push_transaction(tx.clone()); - let shadow_result = - shadow - .vm - .inspect_transaction_with_bytecode_compression((), tx, with_compression); + let shadow_result = shadow.vm.inspect_transaction_with_bytecode_compression( + Shadow::TracerDispatcher::default(), + tx, + with_compression, + ); let mut errors = DivergenceErrors::default(); errors.check_results_match(&main_result.1, &shadow_result.1); if let Err(err) = errors.into_result() { diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs index efab73aed1df..1059bef15553 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs @@ -98,7 +98,7 @@ impl VmTester { }; } - let vm = Vm::new(l1_batch, self.vm.system_env.clone(), storage); + let vm = Vm::custom(l1_batch, self.vm.system_env.clone(), storage); if self.test_contract.is_some() { self.deploy_test_contract(); @@ -230,7 +230,7 @@ impl VmTesterBuilder { } let fee_account = l1_batch_env.fee_account; - let vm = Vm::new(l1_batch_env, self.system_env, storage_ptr.clone()); + let vm = Vm::custom(l1_batch_env, self.system_env, storage_ptr.clone()); VmTester { vm, diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index a9b2fcd605c9..bb982a5ff09d 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -19,6 +19,10 @@ use zksync_types::{ L2_BASE_TOKEN_ADDRESS, U256, }; use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256}; +use zksync_vm_interface::{ + storage::{ImmutableStorageView, StoragePtr, StorageView}, + VmFactory, +}; use super::{ bootloader_state::{BootloaderState, BootloaderStateSnapshot}, @@ -66,6 +70,63 @@ pub struct Vm { } impl Vm { + pub fn custom(batch_env: L1BatchEnv, system_env: SystemEnv, storage: S) -> Self { + let default_aa_code_hash = system_env + .base_system_smart_contracts + .default_aa + .hash + .into(); + + let program_cache = HashMap::from([convert_system_contract_code( + &system_env.base_system_smart_contracts.default_aa, + false, + )]); + + let (_, bootloader) = + convert_system_contract_code(&system_env.base_system_smart_contracts.bootloader, true); + let bootloader_memory = bootloader_initial_memory(&batch_env); + + let mut inner = VirtualMachine::new( + BOOTLOADER_ADDRESS, + bootloader, + H160::zero(), + vec![], + system_env.bootloader_gas_limit, + Settings { + default_aa_code_hash, + // this will change after 1.5 + evm_interpreter_code_hash: default_aa_code_hash, + hook_address: get_vm_hook_position(VM_VERSION) * 32, + }, + ); + + inner.state.current_frame.sp = 0; + + // The bootloader writes results to high addresses in its heap, so it makes sense to preallocate it. + inner.state.current_frame.heap_size = u32::MAX; + inner.state.current_frame.aux_heap_size = u32::MAX; + inner.state.current_frame.exception_handler = INITIAL_FRAME_FORMAL_EH_LOCATION; + + let mut me = Self { + world: World::new(storage, program_cache), + inner, + suspended_at: 0, + gas_for_account_validation: system_env.default_validation_computational_gas_limit, + bootloader_state: BootloaderState::new( + system_env.execution_mode, + bootloader_memory.clone(), + batch_env.first_l2_block, + ), + system_env, + batch_env, + snapshot: None, + }; + + me.write_to_bootloader_heap(bootloader_memory); + + me + } + fn run( &mut self, execution_mode: VmExecutionMode, @@ -345,67 +406,6 @@ impl Vm { pub(crate) fn decommitted_hashes(&self) -> impl Iterator + '_ { self.inner.world_diff.decommitted_hashes() } -} - -// We don't implement `VmFactory` trait because, unlike old VMs, the new VM doesn't require storage to be writable; -// it maintains its own storage cache and a write buffer. -impl Vm { - pub fn new(batch_env: L1BatchEnv, system_env: SystemEnv, storage: S) -> Self { - let default_aa_code_hash = system_env - .base_system_smart_contracts - .default_aa - .hash - .into(); - - let program_cache = HashMap::from([convert_system_contract_code( - &system_env.base_system_smart_contracts.default_aa, - false, - )]); - - let (_, bootloader) = - convert_system_contract_code(&system_env.base_system_smart_contracts.bootloader, true); - let bootloader_memory = bootloader_initial_memory(&batch_env); - - let mut inner = VirtualMachine::new( - BOOTLOADER_ADDRESS, - bootloader, - H160::zero(), - vec![], - system_env.bootloader_gas_limit, - Settings { - default_aa_code_hash, - // this will change after 1.5 - evm_interpreter_code_hash: default_aa_code_hash, - hook_address: get_vm_hook_position(VM_VERSION) * 32, - }, - ); - - inner.state.current_frame.sp = 0; - - // The bootloader writes results to high addresses in its heap, so it makes sense to preallocate it. - inner.state.current_frame.heap_size = u32::MAX; - inner.state.current_frame.aux_heap_size = u32::MAX; - inner.state.current_frame.exception_handler = INITIAL_FRAME_FORMAL_EH_LOCATION; - - let mut me = Self { - world: World::new(storage, program_cache), - inner, - suspended_at: 0, - gas_for_account_validation: system_env.default_validation_computational_gas_limit, - bootloader_state: BootloaderState::new( - system_env.execution_mode, - bootloader_memory.clone(), - batch_env.first_l2_block, - ), - system_env, - batch_env, - snapshot: None, - }; - - me.write_to_bootloader_heap(bootloader_memory); - - me - } fn delete_history_if_appropriate(&mut self) { if self.snapshot.is_none() && self.inner.state.previous_frames.is_empty() { @@ -414,6 +414,17 @@ impl Vm { } } +impl VmFactory> for Vm> { + fn new( + batch_env: L1BatchEnv, + system_env: SystemEnv, + storage: StoragePtr>, + ) -> Self { + let storage = ImmutableStorageView::new(storage); + Self::custom(batch_env, system_env, storage) + } +} + impl VmInterface for Vm { type TracerDispatcher = (); diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index e2cb6d9a1e2c..f39262f7091b 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -261,7 +261,11 @@ impl VmInstance { FastVmMode::Old => Self::new(l1_batch_env, system_env, storage_view), FastVmMode::New => { let storage = ImmutableStorageView::new(storage_view); - Self::VmFast(crate::vm_fast::Vm::new(l1_batch_env, system_env, storage)) + Self::VmFast(crate::vm_fast::Vm::custom( + l1_batch_env, + system_env, + storage, + )) } FastVmMode::Shadow | FastVmMode::ShadowLenient => { let mut vm = ShadowVm::new(l1_batch_env, system_env, storage_view); diff --git a/core/tests/vm-benchmark/harness/src/lib.rs b/core/tests/vm-benchmark/harness/src/lib.rs index 6460d25a8e8d..ed5ac3d62581 100644 --- a/core/tests/vm-benchmark/harness/src/lib.rs +++ b/core/tests/vm-benchmark/harness/src/lib.rs @@ -125,7 +125,7 @@ impl BenchmarkingVmFactory for Fast { system_env: SystemEnv, storage: &'static InMemoryStorage, ) -> Self::Instance { - vm_fast::Vm::new(batch_env, system_env, storage) + vm_fast::Vm::custom(batch_env, system_env, storage) } } From 1eef7e2cbbd05e4599e658b036d79a776576f04b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 11:47:35 +0300 Subject: [PATCH 08/24] Sketch shadowing tests --- Cargo.lock | 1 + core/lib/multivm/Cargo.toml | 1 + core/lib/multivm/src/dump.rs | 26 +- core/lib/multivm/src/versions/mod.rs | 2 + .../src/versions/{shadow.rs => shadow/mod.rs} | 52 +++- core/lib/multivm/src/versions/shadow/tests.rs | 229 ++++++++++++++++++ core/lib/multivm/src/versions/testonly.rs | 51 ++++ .../src/versions/vm_fast/tests/l2_blocks.rs | 6 +- .../src/versions/vm_fast/tests/tester/mod.rs | 2 +- .../vm_fast/tests/tester/vm_tester.rs | 69 +----- core/lib/vm_interface/src/vm.rs | 1 + 11 files changed, 357 insertions(+), 83 deletions(-) rename core/lib/multivm/src/versions/{shadow.rs => shadow/mod.rs} (93%) create mode 100644 core/lib/multivm/src/versions/shadow/tests.rs create mode 100644 core/lib/multivm/src/versions/testonly.rs diff --git a/Cargo.lock b/Cargo.lock index 35b2bc4919c8..6919b0bc953c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8927,6 +8927,7 @@ name = "zksync_multivm" version = "0.1.0" dependencies = [ "anyhow", + "assert_matches", "circuit_sequencer_api 0.133.0", "circuit_sequencer_api 0.140.0", "circuit_sequencer_api 0.141.1", diff --git a/core/lib/multivm/Cargo.toml b/core/lib/multivm/Cargo.toml index b50f35475162..bdfd4b723bdf 100644 --- a/core/lib/multivm/Cargo.toml +++ b/core/lib/multivm/Cargo.toml @@ -45,3 +45,4 @@ tokio = { workspace = true, features = ["time"] } zksync_test_account.workspace = true ethabi.workspace = true zksync_eth_signer.workspace = true +assert_matches.workspace = true diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index e21091eb1f84..0e7524ab76ec 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -12,11 +12,12 @@ use zksync_vm_interface::{ pub type VmDumpHandler = Arc; /// Part of the VM dump representing the storage oracle. -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[cfg_attr(test, derive(PartialEq))] pub(crate) struct VmStorageDump { - read_storage_keys: HashMap, - repeated_writes: HashMap, - factory_deps: HashMap, + pub(crate) read_storage_keys: HashMap, + pub(crate) repeated_writes: HashMap, + pub(crate) factory_deps: HashMap, } impl VmStorageDump { @@ -49,6 +50,7 @@ impl VmStorageDump { }) .collect(); + // FIXME: may panic let used_contract_hashes = vm.get_current_execution_state().used_contract_hashes; let factory_deps = used_contract_hashes .into_iter() @@ -81,21 +83,23 @@ impl VmStorageDump { } } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(test, derive(PartialEq))] #[serde(rename_all = "snake_case")] -enum VmAction { +pub(crate) enum VmAction { Block(L2BlockEnv), Transaction(Box), } /// VM dump allowing to re-run the VM on the same inputs. Opaque, but can be (de)serialized. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(test, derive(PartialEq))] pub struct VmDump { - l1_batch_env: L1BatchEnv, - system_env: SystemEnv, - actions: Vec, + pub(crate) l1_batch_env: L1BatchEnv, + pub(crate) system_env: SystemEnv, + pub(crate) actions: Vec, #[serde(flatten)] - storage: VmStorageDump, + pub(crate) storage: VmStorageDump, } impl VmDump { diff --git a/core/lib/multivm/src/versions/mod.rs b/core/lib/multivm/src/versions/mod.rs index 81358a482f1a..7d4703ea77ae 100644 --- a/core/lib/multivm/src/versions/mod.rs +++ b/core/lib/multivm/src/versions/mod.rs @@ -1,5 +1,7 @@ pub mod shadow; mod shared; +#[cfg(test)] +mod testonly; pub mod vm_1_3_2; pub mod vm_1_4_1; pub mod vm_1_4_2; diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow/mod.rs similarity index 93% rename from core/lib/multivm/src/versions/shadow.rs rename to core/lib/multivm/src/versions/shadow/mod.rs index 708347910eb4..84101d31059c 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -19,6 +19,9 @@ use crate::{ vm_fast, }; +#[cfg(test)] +mod tests; + struct VmWithReporting { vm: Shadow, main_vm_storage: StoragePtr>, @@ -38,12 +41,16 @@ impl fmt::Debug for VmWithReporting VmWithReporting { - fn report(self, main_vm: &impl VmInterface, err: anyhow::Error) { - let mut dump = self.partial_dump; - let batch_number = dump.l1_batch_number(); + fn finalize_dump(&mut self, main_vm: &impl VmInterface) { + self.partial_dump + .set_storage(VmStorageDump::new(&self.main_vm_storage, main_vm)); + } + + fn report(mut self, main_vm: &impl VmInterface, err: anyhow::Error) { + let batch_number = self.partial_dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - dump.set_storage(VmStorageDump::new(&self.main_vm_storage, main_vm)); - (self.dump_handler)(dump); + self.finalize_dump(main_vm); + (self.dump_handler)(self.partial_dump); if self.panic_on_divergence { panic!("{err:?}"); @@ -79,21 +86,32 @@ where shadow.panic_on_divergence = panic; } } + + #[cfg(test)] + fn dump_state(self) -> VmDump { + let mut shadow = self.shadow.into_inner().expect("VM execution diverged"); + shadow.finalize_dump(&self.main); + shadow.partial_dump + } } -impl VmFactory> for ShadowVm +impl ShadowVm where S: ReadStorage, Main: VmFactory>, - Shadow: VmFactory>, + Shadow: VmInterface, { - fn new( + pub fn with_custom_shadow( batch_env: L1BatchEnv, system_env: SystemEnv, storage: StoragePtr>, - ) -> Self { + shadow_storage: StoragePtr, + ) -> Self + where + Shadow: VmFactory, + { let main = Main::new(batch_env.clone(), system_env.clone(), storage.clone()); - let shadow = Shadow::new(batch_env.clone(), system_env.clone(), storage.clone()); + let shadow = Shadow::new(batch_env.clone(), system_env.clone(), shadow_storage); let shadow = VmWithReporting { vm: shadow, main_vm_storage: storage, @@ -108,6 +126,20 @@ where } } +impl VmFactory> for ShadowVm +where + S: ReadStorage, + Main: VmFactory>, +{ + fn new( + batch_env: L1BatchEnv, + system_env: SystemEnv, + storage: StoragePtr>, + ) -> Self { + Self::with_custom_shadow(batch_env, system_env, storage.clone(), storage) + } +} + /// **Important.** This doesn't properly handle tracers; they are not passed to the shadow VM! impl VmInterface for ShadowVm where diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs new file mode 100644 index 000000000000..0d53fbbfbd8d --- /dev/null +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -0,0 +1,229 @@ +//! Shadow VM tests. + +use assert_matches::assert_matches; +use zksync_contracts::{get_loadnext_contract, test_contracts::LoadnextContractExecutionParams}; +use zksync_test_account::{Account, TxType}; +use zksync_types::{ + block::L2BlockHasher, fee::Fee, Execute, L1BatchNumber, L2BlockNumber, ProtocolVersionId, H256, + U256, +}; +use zksync_utils::bytecode::hash_bytecode; + +use super::*; +use crate::{ + dump::VmAction, + interface::{storage::InMemoryStorage, ExecutionResult}, + utils::get_max_gas_per_pubdata_byte, + versions::testonly::{default_l1_batch, default_system_env, make_account_rich}, + vm_latest, + vm_latest::HistoryEnabled, +}; + +type ReferenceVm = vm_latest::Vm, HistoryEnabled>; + +fn hash_block(block_env: L2BlockEnv, tx_hashes: &[H256]) -> H256 { + let mut hasher = L2BlockHasher::new( + L2BlockNumber(block_env.number), + block_env.timestamp, + block_env.prev_block_hash, + ); + for &tx_hash in tx_hashes { + hasher.push_tx_hash(tx_hash); + } + hasher.finalize(ProtocolVersionId::latest()) +} + +fn tx_fee(gas_limit: u32) -> Fee { + Fee { + gas_limit: U256::from(gas_limit), + max_fee_per_gas: U256::from(250_000_000), + max_priority_fee_per_gas: U256::from(0), + gas_per_pubdata_limit: U256::from(get_max_gas_per_pubdata_byte( + ProtocolVersionId::latest().into(), + )), + } +} + +#[derive(Debug)] +struct Harness { + alice: Account, + bob: Account, + current_block: L2BlockEnv, +} + +impl Harness { + fn new(l1_batch_env: &L1BatchEnv) -> Self { + Self { + alice: Account::random(), + bob: Account::random(), + current_block: l1_batch_env.first_l2_block, + } + } + + fn setup_storage(&self, storage: &mut InMemoryStorage) { + make_account_rich(storage, &self.alice); + make_account_rich(storage, &self.bob); + } + + fn new_block(&mut self, vm: &mut impl VmInterface, tx_hashes: &[H256]) { + self.current_block = L2BlockEnv { + number: self.current_block.number + 1, + timestamp: self.current_block.timestamp + 1, + prev_block_hash: hash_block(self.current_block, tx_hashes), + max_virtual_blocks_to_create: self.current_block.max_virtual_blocks_to_create, + }; + vm.start_new_l2_block(self.current_block); + } + + fn execute_on_vm(&mut self, vm: &mut impl VmInterface) { + let transfer_exec = Execute { + contract_address: self.bob.address(), + calldata: vec![], + value: 1_000_000_000.into(), + factory_deps: vec![], + }; + let transfer_to_bob = self + .alice + .get_l2_tx_for_execute(transfer_exec.clone(), None); + let (compression_result, exec_result) = + vm.execute_transaction_with_bytecode_compression(transfer_to_bob.clone(), true); + compression_result.unwrap(); + assert!(!exec_result.result.is_failed(), "{:#?}", exec_result); + + self.new_block(vm, &[transfer_to_bob.hash()]); + + let out_of_gas_transfer = self.bob.get_l2_tx_for_execute( + transfer_exec.clone(), + Some(tx_fee(200_000)), // high enough to pass validation + ); + let (compression_result, exec_result) = + vm.execute_transaction_with_bytecode_compression(out_of_gas_transfer.clone(), true); + compression_result.unwrap(); + assert_matches!(exec_result.result, ExecutionResult::Revert { .. }); + + self.new_block(vm, &[out_of_gas_transfer.hash()]); + + let deploy_tx = self.alice.get_deploy_tx( + &get_loadnext_contract().bytecode, + Some(&[ethabi::Token::Uint(100.into())]), + TxType::L2, + ); + let (compression_result, exec_result) = + vm.execute_transaction_with_bytecode_compression(deploy_tx.tx.clone(), true); + compression_result.unwrap(); + assert!(!exec_result.result.is_failed(), "{:#?}", exec_result); + + let load_test_tx = self.bob.get_loadnext_transaction( + deploy_tx.address, + LoadnextContractExecutionParams::default(), + TxType::L2, + ); + let (compression_result, exec_result) = + vm.execute_transaction_with_bytecode_compression(load_test_tx.clone(), true); + compression_result.unwrap(); + assert!(!exec_result.result.is_failed(), "{:#?}", exec_result); + + self.new_block(vm, &[deploy_tx.tx.hash(), load_test_tx.hash()]); + vm.finish_batch(); + } +} + +fn sanity_check_vm() -> (Vm, Harness) +where + Vm: VmInterface + VmFactory>, +{ + let system_env = default_system_env(); + let l1_batch_env = default_l1_batch(L1BatchNumber(1)); + let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); + let mut harness = Harness::new(&l1_batch_env); + harness.setup_storage(&mut storage); + + let storage = StorageView::new(storage).to_rc_ptr(); + let mut vm = Vm::new(l1_batch_env, system_env, storage); + harness.execute_on_vm(&mut vm); + (vm, harness) +} + +#[test] +fn sanity_check_harness() { + sanity_check_vm::(); +} + +#[test] +fn sanity_check_harness_on_new_vm() { + sanity_check_vm::>(); +} + +#[test] +fn sanity_check_shadow_vm() { + let system_env = default_system_env(); + let l1_batch_env = default_l1_batch(L1BatchNumber(1)); + let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); + let mut harness = Harness::new(&l1_batch_env); + harness.setup_storage(&mut storage); + + // We need separate storage views since they are mutated by the VM execution + let main_storage = StorageView::new(&storage).to_rc_ptr(); + let shadow_storage = StorageView::new(&storage).to_rc_ptr(); + let mut vm = ShadowVm::<_, ReferenceVm<_>, ReferenceVm<_>>::with_custom_shadow( + l1_batch_env, + system_env, + main_storage, + shadow_storage, + ); + harness.execute_on_vm(&mut vm); +} + +#[test] +fn shadow_vm_basics() { + let (vm, harness) = sanity_check_vm::>(); + let dump = vm.dump_state(); + assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); + assert_matches!( + dump.actions.as_slice(), + [ + VmAction::Transaction(_), + VmAction::Block(_), + VmAction::Transaction(_), + VmAction::Block(_), + VmAction::Transaction(_), + VmAction::Transaction(_), + VmAction::Block(_), + ] + ); + assert!(!dump.storage.read_storage_keys.is_empty()); + assert!(!dump.storage.factory_deps.is_empty()); + + dbg!(111); + + let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); + harness.setup_storage(&mut storage); + let storage = StorageView::new(storage).to_rc_ptr(); + + let dump_storage = dump.storage.clone().into_storage(); + let dump_storage = StorageView::new(dump_storage).to_rc_ptr(); + // Check that the VM executes identically when reading from the original storage and one restored from the dump. + let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm>::with_custom_shadow( + dump.l1_batch_env.clone(), + dump.system_env.clone(), + storage, + dump_storage, + ); + + for action in dump.actions.clone() { + match dbg!(action) { + VmAction::Transaction(tx) => { + let (compression_result, _) = + vm.execute_transaction_with_bytecode_compression(*tx, true); + compression_result.unwrap(); + } + VmAction::Block(block) => { + vm.start_new_l2_block(block); + } + } + } + vm.finish_batch(); + + let new_dump = vm.dump_state(); + pretty_assertions::assert_eq!(new_dump, dump); +} diff --git a/core/lib/multivm/src/versions/testonly.rs b/core/lib/multivm/src/versions/testonly.rs new file mode 100644 index 000000000000..245ff9403c3a --- /dev/null +++ b/core/lib/multivm/src/versions/testonly.rs @@ -0,0 +1,51 @@ +use zksync_contracts::BaseSystemContracts; +use zksync_test_account::Account; +use zksync_types::{ + block::L2BlockHasher, fee_model::BatchFeeInput, helpers::unix_timestamp_ms, + utils::storage_key_for_eth_balance, Address, L1BatchNumber, L2BlockNumber, L2ChainId, + ProtocolVersionId, U256, +}; +use zksync_utils::u256_to_h256; + +use crate::{ + interface::{storage::InMemoryStorage, L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode}, + vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, +}; + +pub(super) fn default_system_env() -> SystemEnv { + SystemEnv { + zk_porter_available: false, + version: ProtocolVersionId::latest(), + base_system_smart_contracts: BaseSystemContracts::playground(), + bootloader_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, + execution_mode: TxExecutionMode::VerifyExecute, + default_validation_computational_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, + chain_id: L2ChainId::from(270), + } +} + +pub(super) fn default_l1_batch(number: L1BatchNumber) -> L1BatchEnv { + let timestamp = unix_timestamp_ms(); + L1BatchEnv { + previous_batch_hash: None, + number, + timestamp, + fee_input: BatchFeeInput::l1_pegged( + 50_000_000_000, // 50 gwei + 250_000_000, // 0.25 gwei + ), + fee_account: Address::random(), + enforced_base_fee: None, + first_l2_block: L2BlockEnv { + number: 1, + timestamp, + prev_block_hash: L2BlockHasher::legacy_hash(L2BlockNumber(0)), + max_virtual_blocks_to_create: 100, + }, + } +} + +pub(super) fn make_account_rich(storage: &mut InMemoryStorage, account: &Account) { + let key = storage_key_for_eth_balance(&account.address); + storage.set_value(key, u256_to_h256(U256::from(10_u64.pow(19)))); +} diff --git a/core/lib/multivm/src/versions/vm_fast/tests/l2_blocks.rs b/core/lib/multivm/src/versions/vm_fast/tests/l2_blocks.rs index 6ff5ed426cba..53035315a612 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/l2_blocks.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/l2_blocks.rs @@ -18,10 +18,8 @@ use crate::{ storage::ReadStorage, ExecutionResult, Halt, L2BlockEnv, TxExecutionMode, VmExecutionMode, VmInterface, }, - vm_fast::{ - tests::tester::{default_l1_batch, VmTesterBuilder}, - vm::Vm, - }, + versions::testonly::default_l1_batch, + vm_fast::{tests::tester::VmTesterBuilder, vm::Vm}, vm_latest::{ constants::{TX_OPERATOR_L2_BLOCK_INFO_OFFSET, TX_OPERATOR_SLOTS_PER_L2_BLOCK_INFO}, utils::l2_blocks::get_l2_block_hash_key, diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/mod.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/mod.rs index 781069ddf499..212e569d5107 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/mod.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/mod.rs @@ -1,5 +1,5 @@ pub(crate) use transaction_test_info::{ExpectedError, TransactionTestInfo, TxModifier}; -pub(crate) use vm_tester::{default_l1_batch, get_empty_storage, VmTester, VmTesterBuilder}; +pub(crate) use vm_tester::{get_empty_storage, VmTester, VmTesterBuilder}; pub(crate) use zksync_test_account::{Account, DeployContractsTx, TxType}; mod transaction_test_info; diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs index 1059bef15553..f47ba0de344a 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs @@ -4,13 +4,8 @@ use vm2::WorldDiff; use zksync_contracts::BaseSystemContracts; use zksync_test_account::{Account, TxType}; use zksync_types::{ - block::L2BlockHasher, - fee_model::BatchFeeInput, - get_code_key, get_is_account_key, - helpers::unix_timestamp_ms, - utils::{deployed_address_create, storage_key_for_eth_balance}, - AccountTreeId, Address, L1BatchNumber, L2BlockNumber, L2ChainId, Nonce, ProtocolVersionId, - StorageKey, U256, + block::L2BlockHasher, get_code_key, get_is_account_key, utils::deployed_address_create, + AccountTreeId, Address, L1BatchNumber, L2BlockNumber, Nonce, StorageKey, }; use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; @@ -19,8 +14,11 @@ use crate::{ storage::{InMemoryStorage, StoragePtr}, L1BatchEnv, L2Block, L2BlockEnv, SystemEnv, TxExecutionMode, VmExecutionMode, VmInterface, }, - versions::vm_fast::{tests::utils::read_test_contract, vm::Vm}, - vm_latest::{constants::BATCH_COMPUTATIONAL_GAS_LIMIT, utils::l2_blocks::load_last_l2_block}, + versions::{ + testonly::{default_l1_batch, default_system_env, make_account_rich}, + vm_fast::{tests::utils::read_test_contract, vm::Vm}, + }, + vm_latest::utils::l2_blocks::load_last_l2_block, }; pub(crate) struct VmTester { @@ -62,10 +60,10 @@ impl VmTester { pub(crate) fn reset_state(&mut self, use_latest_l2_block: bool) { for account in self.rich_accounts.iter_mut() { account.nonce = Nonce(0); - make_account_rich(self.storage.clone(), account); + make_account_rich(&mut self.storage.borrow_mut(), account); } if let Some(deployer) = &self.deployer { - make_account_rich(self.storage.clone(), deployer); + make_account_rich(&mut self.storage.borrow_mut(), deployer); } if !self.custom_contracts.is_empty() { @@ -131,21 +129,12 @@ impl Clone for VmTesterBuilder { } } -#[allow(dead_code)] impl VmTesterBuilder { pub(crate) fn new() -> Self { Self { storage: None, l1_batch_env: None, - system_env: SystemEnv { - zk_porter_available: false, - version: ProtocolVersionId::latest(), - base_system_smart_contracts: BaseSystemContracts::playground(), - bootloader_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, - execution_mode: TxExecutionMode::VerifyExecute, - default_validation_computational_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, - chain_id: L2ChainId::from(270), - }, + system_env: default_system_env(), deployer: None, rich_accounts: vec![], custom_contracts: vec![], @@ -157,11 +146,6 @@ impl VmTesterBuilder { self } - pub(crate) fn with_system_env(mut self, system_env: SystemEnv) -> Self { - self.system_env = system_env; - self - } - pub(crate) fn with_storage(mut self, storage: InMemoryStorage) -> Self { self.storage = Some(storage); self @@ -223,10 +207,10 @@ impl VmTesterBuilder { insert_contracts(&mut raw_storage, &self.custom_contracts); let storage_ptr = Rc::new(RefCell::new(raw_storage)); for account in self.rich_accounts.iter() { - make_account_rich(storage_ptr.clone(), account); + make_account_rich(&mut storage_ptr.borrow_mut(), account); } if let Some(deployer) = &self.deployer { - make_account_rich(storage_ptr.clone(), deployer); + make_account_rich(&mut storage_ptr.borrow_mut(), deployer); } let fee_account = l1_batch_env.fee_account; @@ -244,35 +228,6 @@ impl VmTesterBuilder { } } -pub(crate) fn default_l1_batch(number: L1BatchNumber) -> L1BatchEnv { - let timestamp = unix_timestamp_ms(); - L1BatchEnv { - previous_batch_hash: None, - number, - timestamp, - fee_input: BatchFeeInput::l1_pegged( - 50_000_000_000, // 50 gwei - 250_000_000, // 0.25 gwei - ), - fee_account: Address::random(), - enforced_base_fee: None, - first_l2_block: L2BlockEnv { - number: 1, - timestamp, - prev_block_hash: L2BlockHasher::legacy_hash(L2BlockNumber(0)), - max_virtual_blocks_to_create: 100, - }, - } -} - -pub(crate) fn make_account_rich(storage: StoragePtr, account: &Account) { - let key = storage_key_for_eth_balance(&account.address); - storage - .as_ref() - .borrow_mut() - .set_value(key, u256_to_h256(U256::from(10u64.pow(19)))); -} - pub(crate) fn get_empty_storage() -> InMemoryStorage { InMemoryStorage::with_system_contracts(hash_bytecode) } diff --git a/core/lib/vm_interface/src/vm.rs b/core/lib/vm_interface/src/vm.rs index b8614a46c147..f384adab82e9 100644 --- a/core/lib/vm_interface/src/vm.rs +++ b/core/lib/vm_interface/src/vm.rs @@ -48,6 +48,7 @@ pub trait VmInterface { fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv); /// Get the current state of the virtual machine. + /// This method should be used only after the batch execution; otherwise, it can panic. fn get_current_execution_state(&self) -> CurrentExecutionState; /// Execute transaction with optional bytecode compression. From ca7a0b95d2b9132717d9da7fa5a3b3362af793ff Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 12:38:37 +0300 Subject: [PATCH 09/24] Fix dumping state at middle of batch execution --- core/lib/multivm/src/dump.rs | 8 +- core/lib/multivm/src/versions/shadow/mod.rs | 113 ++++++++++-------- core/lib/multivm/src/versions/shadow/tests.rs | 13 +- core/lib/multivm/src/vm_instance.rs | 4 +- 4 files changed, 73 insertions(+), 65 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 0e7524ab76ec..e60191537d4c 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,11 +1,11 @@ use std::{collections::HashMap, sync::Arc}; use serde::{Deserialize, Serialize}; -use zksync_types::{web3, L1BatchNumber, Transaction, H256}; +use zksync_types::{web3, L1BatchNumber, Transaction, H256, U256}; use zksync_utils::u256_to_h256; use zksync_vm_interface::{ storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, - L1BatchEnv, L2BlockEnv, SystemEnv, VmInterface, + L1BatchEnv, L2BlockEnv, SystemEnv, }; /// Handler for [`VmDump`]. @@ -24,7 +24,7 @@ impl VmStorageDump { /// Storage must be the one used by the VM. pub(crate) fn new( storage: &StoragePtr>, - vm: &impl VmInterface, + used_contract_hashes: Vec, ) -> Self { let mut storage = storage.borrow_mut(); let storage_cache = storage.cache(); @@ -50,8 +50,6 @@ impl VmStorageDump { }) .collect(); - // FIXME: may panic - let used_contract_hashes = vm.get_current_execution_state().used_contract_hashes; let factory_deps = used_contract_hashes .into_iter() .filter_map(|hash| { diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/multivm/src/versions/shadow/mod.rs index 84101d31059c..f4d872c1a39b 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -5,7 +5,7 @@ use std::{ sync::Arc, }; -use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; +use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, U256}; use crate::{ dump::{VmDump, VmDumpHandler, VmStorageDump}, @@ -16,7 +16,9 @@ use crate::{ VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, }, - vm_fast, + vm_fast, vm_latest, + vm_latest::HistoryEnabled, + HistoryMode, }; #[cfg(test)] @@ -41,15 +43,17 @@ impl fmt::Debug for VmWithReporting VmWithReporting { - fn finalize_dump(&mut self, main_vm: &impl VmInterface) { - self.partial_dump - .set_storage(VmStorageDump::new(&self.main_vm_storage, main_vm)); + fn finalize_dump(&mut self, used_contract_hashes: Vec) { + self.partial_dump.set_storage(VmStorageDump::new( + &self.main_vm_storage, + used_contract_hashes, + )); } - fn report(mut self, main_vm: &impl VmInterface, err: anyhow::Error) { + fn report(mut self, used_contract_hashes: Vec, err: anyhow::Error) { let batch_number = self.partial_dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - self.finalize_dump(main_vm); + self.finalize_dump(used_contract_hashes); (self.dump_handler)(self.partial_dump); if self.panic_on_divergence { @@ -64,15 +68,15 @@ impl VmWithReporting { } #[derive(Debug)] -pub struct ShadowVm>> { - main: Main, +pub struct ShadowVm>> { + main: vm_latest::Vm, H>, shadow: RefCell>>, } -impl ShadowVm +impl ShadowVm where S: ReadStorage, - Main: VmInterface, + H: HistoryMode, Shadow: VmInterface, { pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { @@ -87,18 +91,32 @@ where } } + /// Mutable ref is not necessary, but it automatically drops potential borrows. + fn report(&mut self, err: anyhow::Error) { + self.report_shared(err); + } + + /// The caller is responsible for dropping any `shadow` borrows beforehand. + fn report_shared(&self, err: anyhow::Error) { + self.shadow + .take() + .unwrap() + .report(self.main.get_used_contracts(), err); + } + #[cfg(test)] - fn dump_state(self) -> VmDump { - let mut shadow = self.shadow.into_inner().expect("VM execution diverged"); - shadow.finalize_dump(&self.main); - shadow.partial_dump + fn dump_state(&mut self) -> VmDump { + let mut borrow = self.shadow.borrow_mut(); + let borrow = borrow.as_mut().expect("VM execution diverged"); + borrow.finalize_dump(self.main.get_used_contracts()); + borrow.partial_dump.clone() } } -impl ShadowVm +impl ShadowVm where S: ReadStorage, - Main: VmFactory>, + H: HistoryMode, Shadow: VmInterface, { pub fn with_custom_shadow( @@ -110,7 +128,7 @@ where where Shadow: VmFactory, { - let main = Main::new(batch_env.clone(), system_env.clone(), storage.clone()); + let main = vm_latest::Vm::new(batch_env.clone(), system_env.clone(), storage.clone()); let shadow = Shadow::new(batch_env.clone(), system_env.clone(), shadow_storage); let shadow = VmWithReporting { vm: shadow, @@ -126,10 +144,10 @@ where } } -impl VmFactory> for ShadowVm +impl VmFactory> for ShadowVm where S: ReadStorage, - Main: VmFactory>, + H: HistoryMode, { fn new( batch_env: L1BatchEnv, @@ -141,13 +159,13 @@ where } /// **Important.** This doesn't properly handle tracers; they are not passed to the shadow VM! -impl VmInterface for ShadowVm +impl VmInterface for ShadowVm where S: ReadStorage, - Main: VmInterface, + H: HistoryMode, Shadow: VmInterface, { - type TracerDispatcher = Main::TracerDispatcher; + type TracerDispatcher = , H> as VmInterface>::TracerDispatcher; fn push_transaction(&mut self, tx: Transaction) { if let Some(shadow) = self.shadow.get_mut() { @@ -165,10 +183,7 @@ where errors.check_results_match(&main_result, &shadow_result); if let Err(err) = errors.into_result() { let ctx = format!("executing VM with mode {execution_mode:?}"); - self.shadow - .take() - .unwrap() - .report(&self.main, err.context(ctx)); + self.report(err.context(ctx)); } } main_result @@ -189,10 +204,7 @@ where if let Err(err) = errors.into_result() { let ctx = format!("executing VM with mode {execution_mode:?}"); - self.shadow - .take() - .unwrap() - .report(&self.main, err.context(ctx)); + self.report(err.context(ctx)); } } main_result @@ -200,12 +212,14 @@ where fn get_bootloader_memory(&self) -> BootloaderMemory { let main_memory = self.main.get_bootloader_memory(); - if let Some(shadow) = &*self.shadow.borrow() { + let borrow = self.shadow.borrow(); + if let Some(shadow) = &*borrow { let shadow_memory = shadow.vm.get_bootloader_memory(); let result = DivergenceErrors::single("get_bootloader_memory", &main_memory, &shadow_memory); if let Err(err) = result { - self.shadow.take().unwrap().report(&self.main, err); + drop(borrow); + self.report_shared(err); } } main_memory @@ -213,7 +227,8 @@ where fn get_last_tx_compressed_bytecodes(&self) -> Vec { let main_bytecodes = self.main.get_last_tx_compressed_bytecodes(); - if let Some(shadow) = &*self.shadow.borrow() { + let borrow = self.shadow.borrow(); + if let Some(shadow) = &*borrow { let shadow_bytecodes = shadow.vm.get_last_tx_compressed_bytecodes(); let result = DivergenceErrors::single( "get_last_tx_compressed_bytecodes", @@ -221,7 +236,8 @@ where &shadow_bytecodes, ); if let Err(err) = result { - self.shadow.take().unwrap().report(&self.main, err); + drop(borrow); + self.report_shared(err); } } main_bytecodes @@ -237,12 +253,14 @@ where fn get_current_execution_state(&self) -> CurrentExecutionState { let main_state = self.main.get_current_execution_state(); - if let Some(shadow) = &*self.shadow.borrow() { + let borrow = self.shadow.borrow(); + if let Some(shadow) = &*borrow { let shadow_state = shadow.vm.get_current_execution_state(); let result = DivergenceErrors::single("get_current_execution_state", &main_state, &shadow_state); if let Err(err) = result { - self.shadow.take().unwrap().report(&self.main, err); + drop(borrow); + self.report_shared(err); } } main_state @@ -271,10 +289,7 @@ where let ctx = format!( "executing transaction {tx_hash:?}, with_compression={with_compression:?}" ); - self.shadow - .take() - .unwrap() - .report(&self.main, err.context(ctx)); + self.report(err.context(ctx)); } } main_result @@ -308,10 +323,7 @@ where let ctx = format!( "inspecting transaction {tx_hash:?}, with_compression={with_compression:?}" ); - self.shadow - .take() - .unwrap() - .report(&self.main, err.context(ctx)); + self.report(err.context(ctx)); } } main_result @@ -323,11 +335,13 @@ where fn gas_remaining(&self) -> u32 { let main_gas = self.main.gas_remaining(); - if let Some(shadow) = &*self.shadow.borrow() { + let borrow = self.shadow.borrow(); + if let Some(shadow) = &*borrow { let shadow_gas = shadow.vm.gas_remaining(); let result = DivergenceErrors::single("gas_remaining", &main_gas, &shadow_gas); if let Err(err) = result { - self.shadow.take().unwrap().report(&self.main, err); + drop(borrow); + self.report_shared(err); } } main_gas @@ -363,7 +377,7 @@ where ); if let Err(err) = errors.into_result() { - self.shadow.take().unwrap().report(&self.main, err); + self.report(err); } } main_batch @@ -518,10 +532,9 @@ impl UniqueStorageLogs { } } -impl VmInterfaceHistoryEnabled for ShadowVm +impl VmInterfaceHistoryEnabled for ShadowVm where S: ReadStorage, - T: VmInterfaceHistoryEnabled, { fn make_snapshot(&mut self) { if let Some(shadow) = self.shadow.get_mut() { diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index 0d53fbbfbd8d..f239418e8e00 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -16,7 +16,7 @@ use crate::{ utils::get_max_gas_per_pubdata_byte, versions::testonly::{default_l1_batch, default_system_env, make_account_rich}, vm_latest, - vm_latest::HistoryEnabled, + vm_latest::HistoryDisabled, }; type ReferenceVm = vm_latest::Vm, HistoryEnabled>; @@ -165,7 +165,7 @@ fn sanity_check_shadow_vm() { // We need separate storage views since they are mutated by the VM execution let main_storage = StorageView::new(&storage).to_rc_ptr(); let shadow_storage = StorageView::new(&storage).to_rc_ptr(); - let mut vm = ShadowVm::<_, ReferenceVm<_>, ReferenceVm<_>>::with_custom_shadow( + let mut vm = ShadowVm::<_, HistoryDisabled, ReferenceVm<_>>::with_custom_shadow( l1_batch_env, system_env, main_storage, @@ -176,7 +176,7 @@ fn sanity_check_shadow_vm() { #[test] fn shadow_vm_basics() { - let (vm, harness) = sanity_check_vm::>(); + let (mut vm, harness) = sanity_check_vm::>(); let dump = vm.dump_state(); assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); assert_matches!( @@ -194,8 +194,6 @@ fn shadow_vm_basics() { assert!(!dump.storage.read_storage_keys.is_empty()); assert!(!dump.storage.factory_deps.is_empty()); - dbg!(111); - let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); harness.setup_storage(&mut storage); let storage = StorageView::new(storage).to_rc_ptr(); @@ -203,7 +201,7 @@ fn shadow_vm_basics() { let dump_storage = dump.storage.clone().into_storage(); let dump_storage = StorageView::new(dump_storage).to_rc_ptr(); // Check that the VM executes identically when reading from the original storage and one restored from the dump. - let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm>::with_custom_shadow( + let mut vm = ShadowVm::<_, HistoryDisabled, ReferenceVm>::with_custom_shadow( dump.l1_batch_env.clone(), dump.system_env.clone(), storage, @@ -211,7 +209,7 @@ fn shadow_vm_basics() { ); for action in dump.actions.clone() { - match dbg!(action) { + match action { VmAction::Transaction(tx) => { let (compression_result, _) = vm.execute_transaction_with_bytecode_compression(*tx, true); @@ -221,6 +219,7 @@ fn shadow_vm_basics() { vm.start_new_l2_block(block); } } + vm.dump_state(); // Check that a dump can be created at any point in batch execution } vm.finish_batch(); diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index f39262f7091b..67d72c0c0a3f 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -13,8 +13,6 @@ use crate::{ versions::shadow::ShadowVm, }; -pub type ShadowedFastVm = ShadowVm, H>>; - #[derive(Debug)] pub enum VmInstance { VmM5(crate::vm_m5::Vm, H>), @@ -27,7 +25,7 @@ pub enum VmInstance { Vm1_4_2(crate::vm_1_4_2::Vm, H>), Vm1_5_0(crate::vm_latest::Vm, H>), VmFast(crate::vm_fast::Vm>), - ShadowedVmFast(ShadowedFastVm), + ShadowedVmFast(ShadowVm), } macro_rules! dispatch_vm { From 3cfd1265445d8787fc8275adc637e561e7a61743 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 13:04:03 +0300 Subject: [PATCH 10/24] Move `ContractToDeploy` to test utils --- core/lib/multivm/src/versions/testonly.rs | 48 +++++++++++++++++-- .../src/versions/vm_fast/tests/code_oracle.rs | 10 ++-- .../versions/vm_fast/tests/nonce_holder.rs | 4 +- .../src/versions/vm_fast/tests/refunds.rs | 4 +- .../versions/vm_fast/tests/require_eip712.rs | 6 ++- .../src/versions/vm_fast/tests/storage.rs | 3 +- .../vm_fast/tests/tester/vm_tester.rs | 34 +++---------- .../vm_fast/tests/tracing_execution_error.rs | 6 ++- .../src/versions/vm_fast/tests/transfer.rs | 25 +++++----- 9 files changed, 82 insertions(+), 58 deletions(-) diff --git a/core/lib/multivm/src/versions/testonly.rs b/core/lib/multivm/src/versions/testonly.rs index 245ff9403c3a..cb4d61667a80 100644 --- a/core/lib/multivm/src/versions/testonly.rs +++ b/core/lib/multivm/src/versions/testonly.rs @@ -1,11 +1,11 @@ use zksync_contracts::BaseSystemContracts; use zksync_test_account::Account; use zksync_types::{ - block::L2BlockHasher, fee_model::BatchFeeInput, helpers::unix_timestamp_ms, - utils::storage_key_for_eth_balance, Address, L1BatchNumber, L2BlockNumber, L2ChainId, - ProtocolVersionId, U256, + block::L2BlockHasher, fee_model::BatchFeeInput, get_code_key, get_is_account_key, + helpers::unix_timestamp_ms, utils::storage_key_for_eth_balance, Address, L1BatchNumber, + L2BlockNumber, L2ChainId, ProtocolVersionId, U256, }; -use zksync_utils::u256_to_h256; +use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; use crate::{ interface::{storage::InMemoryStorage, L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode}, @@ -49,3 +49,43 @@ pub(super) fn make_account_rich(storage: &mut InMemoryStorage, account: &Account let key = storage_key_for_eth_balance(&account.address); storage.set_value(key, u256_to_h256(U256::from(10_u64.pow(19)))); } + +#[derive(Debug, Clone)] +pub(super) struct ContractToDeploy { + bytecode: Vec, + address: Address, + is_account: bool, +} + +impl ContractToDeploy { + pub fn new(bytecode: Vec, address: Address) -> Self { + Self { + bytecode, + address, + is_account: false, + } + } + + pub fn account(bytecode: Vec, address: Address) -> Self { + Self { + bytecode, + address, + is_account: true, + } + } + + /// Inserts the contracts into the test environment, bypassing the deployer system contract. + pub fn insert(contracts: &[Self], storage: &mut InMemoryStorage) { + for contract in contracts { + let deployer_code_key = get_code_key(&contract.address); + storage.set_value(deployer_code_key, hash_bytecode(&contract.bytecode)); + + if contract.is_account { + let is_account_key = get_is_account_key(&contract.address); + storage.set_value(is_account_key, u256_to_h256(1_u32.into())); + } + + storage.store_factory_dep(hash_bytecode(&contract.bytecode), contract.bytecode.clone()); + } + } +} diff --git a/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs b/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs index 24fda3beed4b..548b6111777d 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs @@ -6,6 +6,7 @@ use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256}; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{get_empty_storage, VmTesterBuilder}, utils::{load_precompiles_contract, read_precompiles_contract, read_test_contract}, @@ -38,10 +39,9 @@ fn test_code_oracle() { .with_empty_in_memory_storage() .with_execution_mode(TxExecutionMode::VerifyExecute) .with_random_rich_accounts(1) - .with_custom_contracts(vec![( + .with_custom_contracts(vec![ContractToDeploy::new( precompile_contract_bytecode, precompiles_contract_address, - false, )]) .with_storage(storage) .build(); @@ -131,10 +131,9 @@ fn test_code_oracle_big_bytecode() { .with_empty_in_memory_storage() .with_execution_mode(TxExecutionMode::VerifyExecute) .with_random_rich_accounts(1) - .with_custom_contracts(vec![( + .with_custom_contracts(vec![ContractToDeploy::new( precompile_contract_bytecode, precompiles_contract_address, - false, )]) .with_storage(storage) .build(); @@ -195,10 +194,9 @@ fn refunds_in_code_oracle() { let mut vm = VmTesterBuilder::new() .with_execution_mode(TxExecutionMode::VerifyExecute) .with_random_rich_accounts(1) - .with_custom_contracts(vec![( + .with_custom_contracts(vec![ContractToDeploy::new( precompile_contract_bytecode.clone(), precompiles_contract_address, - false, )]) .with_storage(storage.clone()) .build(); diff --git a/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs b/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs index b18676cf2ba6..e0f7eeaf9d59 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs @@ -5,6 +5,7 @@ use crate::{ ExecutionResult, Halt, TxExecutionMode, TxRevertReason, VmExecutionMode, VmInterface, VmRevertReason, }, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{Account, VmTesterBuilder}, utils::read_nonce_holder_tester, @@ -41,10 +42,9 @@ fn test_nonce_holder() { .with_empty_in_memory_storage() .with_execution_mode(TxExecutionMode::VerifyExecute) .with_deployer() - .with_custom_contracts(vec![( + .with_custom_contracts(vec![ContractToDeploy::account( read_nonce_holder_tester().to_vec(), account.address, - true, )]) .with_rich_accounts(vec![account.clone()]) .build(); diff --git a/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs b/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs index 21a3129a3a61..aae69e62d1cb 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs @@ -3,6 +3,7 @@ use zksync_types::{Address, Execute, U256}; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{DeployContractsTx, TxType, VmTesterBuilder}, utils::{read_expensive_contract, read_test_contract}, @@ -172,10 +173,9 @@ fn negative_pubdata_for_transaction() { .with_empty_in_memory_storage() .with_execution_mode(TxExecutionMode::VerifyExecute) .with_random_rich_accounts(1) - .with_custom_contracts(vec![( + .with_custom_contracts(vec![ContractToDeploy::new( expensive_contract_bytecode, expensive_contract_address, - false, )]) .build(); diff --git a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs index 352e709b7043..55cb8c50aab0 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs @@ -10,6 +10,7 @@ use zksync_utils::h256_to_u256; use crate::{ interface::{storage::ReadStorage, TxExecutionMode, VmExecutionMode, VmInterface}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{Account, VmTester, VmTesterBuilder}, utils::read_many_owners_custom_account_contract, @@ -48,7 +49,10 @@ async fn test_require_eip712() { let (bytecode, contract) = read_many_owners_custom_account_contract(); let mut vm = VmTesterBuilder::new() .with_empty_in_memory_storage() - .with_custom_contracts(vec![(bytecode, account_abstraction.address, true)]) + .with_custom_contracts(vec![ContractToDeploy::account( + bytecode, + account_abstraction.address, + )]) .with_execution_mode(TxExecutionMode::VerifyExecute) .with_rich_accounts(vec![account_abstraction.clone(), private_account.clone()]) .build(); diff --git a/core/lib/multivm/src/versions/vm_fast/tests/storage.rs b/core/lib/multivm/src/versions/vm_fast/tests/storage.rs index 733ce1f0618c..072814c39a29 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/storage.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/storage.rs @@ -4,6 +4,7 @@ use zksync_types::{Address, Execute, U256}; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceHistoryEnabled}, + versions::testonly::ContractToDeploy, vm_fast::tests::tester::VmTesterBuilder, }; @@ -21,7 +22,7 @@ fn test_storage(first_tx_calldata: Vec, second_tx_calldata: Vec) -> u32 .with_execution_mode(TxExecutionMode::VerifyExecute) .with_deployer() .with_random_rich_accounts(1) - .with_custom_contracts(vec![(bytecode, test_contract_address, false)]) + .with_custom_contracts(vec![ContractToDeploy::new(bytecode, test_contract_address)]) .build(); let account = &mut vm.rich_accounts[0]; diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs index f47ba0de344a..2fd39a887ec7 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs @@ -4,8 +4,8 @@ use vm2::WorldDiff; use zksync_contracts::BaseSystemContracts; use zksync_test_account::{Account, TxType}; use zksync_types::{ - block::L2BlockHasher, get_code_key, get_is_account_key, utils::deployed_address_create, - AccountTreeId, Address, L1BatchNumber, L2BlockNumber, Nonce, StorageKey, + block::L2BlockHasher, utils::deployed_address_create, AccountTreeId, Address, L1BatchNumber, + L2BlockNumber, Nonce, StorageKey, }; use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; @@ -15,7 +15,7 @@ use crate::{ L1BatchEnv, L2Block, L2BlockEnv, SystemEnv, TxExecutionMode, VmExecutionMode, VmInterface, }, versions::{ - testonly::{default_l1_batch, default_system_env, make_account_rich}, + testonly::{default_l1_batch, default_system_env, make_account_rich, ContractToDeploy}, vm_fast::{tests::utils::read_test_contract, vm::Vm}, }, vm_latest::utils::l2_blocks::load_last_l2_block, @@ -28,7 +28,7 @@ pub(crate) struct VmTester { pub(crate) test_contract: Option
, pub(crate) fee_account: Address, pub(crate) rich_accounts: Vec, - pub(crate) custom_contracts: Vec, + pub(crate) custom_contracts: Vec, } impl VmTester { @@ -105,15 +105,13 @@ impl VmTester { } } -pub(crate) type ContractsToDeploy = (Vec, Address, bool); - pub(crate) struct VmTesterBuilder { storage: Option, l1_batch_env: Option, system_env: SystemEnv, deployer: Option, rich_accounts: Vec, - custom_contracts: Vec, + custom_contracts: Vec, } impl Clone for VmTesterBuilder { @@ -193,7 +191,7 @@ impl VmTesterBuilder { self } - pub(crate) fn with_custom_contracts(mut self, contracts: Vec) -> Self { + pub(crate) fn with_custom_contracts(mut self, contracts: Vec) -> Self { self.custom_contracts = contracts; self } @@ -204,7 +202,7 @@ impl VmTesterBuilder { .unwrap_or_else(|| default_l1_batch(L1BatchNumber(1))); let mut raw_storage = self.storage.unwrap_or_else(get_empty_storage); - insert_contracts(&mut raw_storage, &self.custom_contracts); + ContractToDeploy::insert(&self.custom_contracts, &mut raw_storage); let storage_ptr = Rc::new(RefCell::new(raw_storage)); for account in self.rich_accounts.iter() { make_account_rich(&mut storage_ptr.borrow_mut(), account); @@ -231,21 +229,3 @@ impl VmTesterBuilder { pub(crate) fn get_empty_storage() -> InMemoryStorage { InMemoryStorage::with_system_contracts(hash_bytecode) } - -// Inserts the contracts into the test environment, bypassing the -// deployer system contract. Besides the reference to storage -// it accepts a `contracts` tuple of information about the contract -// and whether or not it is an account. -fn insert_contracts(raw_storage: &mut InMemoryStorage, contracts: &[ContractsToDeploy]) { - for (contract, address, is_account) in contracts { - let deployer_code_key = get_code_key(address); - raw_storage.set_value(deployer_code_key, hash_bytecode(contract)); - - if *is_account { - let is_account_key = get_is_account_key(address); - raw_storage.set_value(is_account_key, u256_to_h256(1_u32.into())); - } - - raw_storage.store_factory_dep(hash_bytecode(contract), contract.clone()); - } -} diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tracing_execution_error.rs b/core/lib/multivm/src/versions/vm_fast/tests/tracing_execution_error.rs index 75144839006e..bccb1e6ec16f 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tracing_execution_error.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tracing_execution_error.rs @@ -2,6 +2,7 @@ use zksync_types::{Execute, H160}; use crate::{ interface::{TxExecutionMode, TxRevertReason, VmRevertReason}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{ExpectedError, TransactionTestInfo, VmTesterBuilder}, utils::{get_execute_error_calldata, read_error_contract, BASE_SYSTEM_CONTRACTS}, @@ -14,7 +15,10 @@ fn test_tracing_of_execution_errors() { let mut vm = VmTesterBuilder::new() .with_empty_in_memory_storage() .with_base_system_smart_contracts(BASE_SYSTEM_CONTRACTS.clone()) - .with_custom_contracts(vec![(read_error_contract(), contract_address, false)]) + .with_custom_contracts(vec![ContractToDeploy::new( + read_error_contract(), + contract_address, + )]) .with_execution_mode(TxExecutionMode::VerifyExecute) .with_deployer() .with_random_rich_accounts(1) diff --git a/core/lib/multivm/src/versions/vm_fast/tests/transfer.rs b/core/lib/multivm/src/versions/vm_fast/tests/transfer.rs index 3b61b8ac7f1e..174f8e43b407 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/transfer.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/transfer.rs @@ -6,6 +6,7 @@ use zksync_utils::u256_to_h256; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{get_empty_storage, VmTesterBuilder}, utils::get_balance, @@ -21,7 +22,7 @@ fn test_send_or_transfer(test_option: TestOptions) { let test_bytecode = read_bytecode( "etc/contracts-test-data/artifacts-zk/contracts/transfer/transfer.sol/TransferTest.json", ); - let recipeint_bytecode = read_bytecode( + let recipient_bytecode = read_bytecode( "etc/contracts-test-data/artifacts-zk/contracts/transfer/transfer.sol/Recipient.json", ); let test_abi = load_contract( @@ -62,8 +63,8 @@ fn test_send_or_transfer(test_option: TestOptions) { .with_deployer() .with_random_rich_accounts(1) .with_custom_contracts(vec![ - (test_bytecode, test_contract_address, false), - (recipeint_bytecode, recipient_address, false), + ContractToDeploy::new(test_bytecode, test_contract_address), + ContractToDeploy::new(recipient_bytecode, recipient_address), ]) .build(); @@ -110,7 +111,7 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { let test_bytecode = read_bytecode( "etc/contracts-test-data/artifacts-zk/contracts/transfer/transfer.sol/TransferTest.json", ); - let reentrant_recipeint_bytecode = read_bytecode( + let reentrant_recipient_bytecode = read_bytecode( "etc/contracts-test-data/artifacts-zk/contracts/transfer/transfer.sol/ReentrantRecipient.json", ); let test_abi = load_contract( @@ -121,7 +122,7 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { ); let test_contract_address = Address::random(); - let reentrant_recipeint_address = Address::random(); + let reentrant_recipient_address = Address::random(); let (value, calldata) = match test_option { TestOptions::Send(value) => ( @@ -130,7 +131,7 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { .function("send") .unwrap() .encode_input(&[ - Token::Address(reentrant_recipeint_address), + Token::Address(reentrant_recipient_address), Token::Uint(value), ]) .unwrap(), @@ -141,7 +142,7 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { .function("transfer") .unwrap() .encode_input(&[ - Token::Address(reentrant_recipeint_address), + Token::Address(reentrant_recipient_address), Token::Uint(value), ]) .unwrap(), @@ -154,12 +155,8 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { .with_deployer() .with_random_rich_accounts(1) .with_custom_contracts(vec![ - (test_bytecode, test_contract_address, false), - ( - reentrant_recipeint_bytecode, - reentrant_recipeint_address, - false, - ), + ContractToDeploy::new(test_bytecode, test_contract_address), + ContractToDeploy::new(reentrant_recipient_bytecode, reentrant_recipient_address), ]) .build(); @@ -167,7 +164,7 @@ fn test_reentrancy_protection_send_or_transfer(test_option: TestOptions) { let account = &mut vm.rich_accounts[0]; let tx1 = account.get_l2_tx_for_execute( Execute { - contract_address: reentrant_recipeint_address, + contract_address: reentrant_recipient_address, calldata: reentrant_recipient_abi .function("setX") .unwrap() From 0e0671e3e50e41646151c12911b5c038ea14e77c Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 13:56:56 +0300 Subject: [PATCH 11/24] Brush up shadow VM tests --- core/lib/multivm/src/versions/shadow/tests.rs | 109 ++++++++++++++---- core/lib/multivm/src/versions/testonly.rs | 22 ++-- .../vm_fast/tests/tester/vm_tester.rs | 2 +- 3 files changed, 100 insertions(+), 33 deletions(-) diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index f239418e8e00..a10db3088ceb 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -1,11 +1,15 @@ //! Shadow VM tests. use assert_matches::assert_matches; -use zksync_contracts::{get_loadnext_contract, test_contracts::LoadnextContractExecutionParams}; +use ethabi::Contract; +use zksync_contracts::{ + get_loadnext_contract, load_contract, read_bytecode, + test_contracts::LoadnextContractExecutionParams, +}; use zksync_test_account::{Account, TxType}; use zksync_types::{ - block::L2BlockHasher, fee::Fee, Execute, L1BatchNumber, L2BlockNumber, ProtocolVersionId, H256, - U256, + block::L2BlockHasher, fee::Fee, AccountTreeId, Address, Execute, L1BatchNumber, L2BlockNumber, + ProtocolVersionId, H256, U256, }; use zksync_utils::bytecode::hash_bytecode; @@ -14,9 +18,10 @@ use crate::{ dump::VmAction, interface::{storage::InMemoryStorage, ExecutionResult}, utils::get_max_gas_per_pubdata_byte, - versions::testonly::{default_l1_batch, default_system_env, make_account_rich}, - vm_latest, - vm_latest::HistoryDisabled, + versions::testonly::{ + default_l1_batch, default_system_env, make_account_rich, ContractToDeploy, + }, + vm_latest::{self, HistoryDisabled}, }; type ReferenceVm = vm_latest::Vm, HistoryEnabled>; @@ -48,14 +53,25 @@ fn tx_fee(gas_limit: u32) -> Fee { struct Harness { alice: Account, bob: Account, + storage_contract: ContractToDeploy, + storage_contract_abi: Contract, current_block: L2BlockEnv, } impl Harness { + const STORAGE_CONTRACT_PATH: &'static str = + "etc/contracts-test-data/artifacts-zk/contracts/storage/storage.sol/StorageTester.json"; + const STORAGE_CONTRACT_ADDRESS: Address = Address::repeat_byte(23); + fn new(l1_batch_env: &L1BatchEnv) -> Self { Self { alice: Account::random(), bob: Account::random(), + storage_contract: ContractToDeploy::new( + read_bytecode(Self::STORAGE_CONTRACT_PATH), + Self::STORAGE_CONTRACT_ADDRESS, + ), + storage_contract_abi: load_contract(Self::STORAGE_CONTRACT_PATH), current_block: l1_batch_env.first_l2_block, } } @@ -63,6 +79,45 @@ impl Harness { fn setup_storage(&self, storage: &mut InMemoryStorage) { make_account_rich(storage, &self.alice); make_account_rich(storage, &self.bob); + + self.storage_contract.insert(storage); + let storage_contract_key = StorageKey::new( + AccountTreeId::new(Self::STORAGE_CONTRACT_ADDRESS), + H256::zero(), + ); + storage.set_value_hashed_enum( + storage_contract_key.hashed_key(), + 999, + H256::from_low_u64_be(42), + ); + } + + fn assert_dump(dump: &VmDump) { + assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); + assert_matches!( + dump.actions.as_slice(), + [ + VmAction::Transaction(_), + VmAction::Block(_), + VmAction::Transaction(_), + VmAction::Transaction(_), + VmAction::Block(_), + VmAction::Transaction(_), + VmAction::Transaction(_), + VmAction::Block(_), + ] + ); + assert!(!dump.storage.read_storage_keys.is_empty()); + assert!(!dump.storage.factory_deps.is_empty()); + + let storage_contract_key = StorageKey::new( + AccountTreeId::new(Self::STORAGE_CONTRACT_ADDRESS), + H256::zero(), + ) + .hashed_key(); + let (value, enum_index) = dump.storage.read_storage_keys[&storage_contract_key]; + assert_eq!(value, H256::from_low_u64_be(42)); + assert_eq!(enum_index, 999); } fn new_block(&mut self, vm: &mut impl VmInterface, tx_hashes: &[H256]) { @@ -101,7 +156,31 @@ impl Harness { compression_result.unwrap(); assert_matches!(exec_result.result, ExecutionResult::Revert { .. }); - self.new_block(vm, &[out_of_gas_transfer.hash()]); + let write_fn = self.storage_contract_abi.function("simpleWrite").unwrap(); + let simple_write_tx = self.alice.get_l2_tx_for_execute( + Execute { + contract_address: Self::STORAGE_CONTRACT_ADDRESS, + calldata: write_fn.encode_input(&[]).unwrap(), + value: 0.into(), + factory_deps: vec![], + }, + None, + ); + let (compression_result, exec_result) = + vm.execute_transaction_with_bytecode_compression(simple_write_tx.clone(), true); + compression_result.unwrap(); + assert!(!exec_result.result.is_failed(), "{:#?}", exec_result); + + let storage_contract_key = StorageKey::new( + AccountTreeId::new(Self::STORAGE_CONTRACT_ADDRESS), + H256::zero(), + ); + let storage_logs = &exec_result.logs.storage_logs; + assert!(storage_logs.iter().any(|log| { + log.log.key == storage_contract_key && log.previous_value == H256::from_low_u64_be(42) + })); + + self.new_block(vm, &[out_of_gas_transfer.hash(), simple_write_tx.hash()]); let deploy_tx = self.alice.get_deploy_tx( &get_loadnext_contract().bytecode, @@ -178,21 +257,7 @@ fn sanity_check_shadow_vm() { fn shadow_vm_basics() { let (mut vm, harness) = sanity_check_vm::>(); let dump = vm.dump_state(); - assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); - assert_matches!( - dump.actions.as_slice(), - [ - VmAction::Transaction(_), - VmAction::Block(_), - VmAction::Transaction(_), - VmAction::Block(_), - VmAction::Transaction(_), - VmAction::Transaction(_), - VmAction::Block(_), - ] - ); - assert!(!dump.storage.read_storage_keys.is_empty()); - assert!(!dump.storage.factory_deps.is_empty()); + Harness::assert_dump(&dump); let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); harness.setup_storage(&mut storage); diff --git a/core/lib/multivm/src/versions/testonly.rs b/core/lib/multivm/src/versions/testonly.rs index cb4d61667a80..51a4d0842d90 100644 --- a/core/lib/multivm/src/versions/testonly.rs +++ b/core/lib/multivm/src/versions/testonly.rs @@ -74,18 +74,20 @@ impl ContractToDeploy { } } + pub fn insert(&self, storage: &mut InMemoryStorage) { + let deployer_code_key = get_code_key(&self.address); + storage.set_value(deployer_code_key, hash_bytecode(&self.bytecode)); + if self.is_account { + let is_account_key = get_is_account_key(&self.address); + storage.set_value(is_account_key, u256_to_h256(1_u32.into())); + } + storage.store_factory_dep(hash_bytecode(&self.bytecode), self.bytecode.clone()); + } + /// Inserts the contracts into the test environment, bypassing the deployer system contract. - pub fn insert(contracts: &[Self], storage: &mut InMemoryStorage) { + pub fn insert_all(contracts: &[Self], storage: &mut InMemoryStorage) { for contract in contracts { - let deployer_code_key = get_code_key(&contract.address); - storage.set_value(deployer_code_key, hash_bytecode(&contract.bytecode)); - - if contract.is_account { - let is_account_key = get_is_account_key(&contract.address); - storage.set_value(is_account_key, u256_to_h256(1_u32.into())); - } - - storage.store_factory_dep(hash_bytecode(&contract.bytecode), contract.bytecode.clone()); + contract.insert(storage); } } } diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs index 2fd39a887ec7..9bf2dc34a328 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs @@ -202,7 +202,7 @@ impl VmTesterBuilder { .unwrap_or_else(|| default_l1_batch(L1BatchNumber(1))); let mut raw_storage = self.storage.unwrap_or_else(get_empty_storage); - ContractToDeploy::insert(&self.custom_contracts, &mut raw_storage); + ContractToDeploy::insert_all(&self.custom_contracts, &mut raw_storage); let storage_ptr = Rc::new(RefCell::new(raw_storage)); for account in self.rich_accounts.iter() { make_account_rich(&mut storage_ptr.borrow_mut(), account); From d8418a46a96426bb4f180f3af99ee4917655006a Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 14:07:38 +0300 Subject: [PATCH 12/24] Implement VM dump playback --- core/lib/multivm/src/dump.rs | 28 +++++++++++++++++-- core/lib/multivm/src/versions/shadow/tests.rs | 12 ++++++-- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index e60191537d4c..4fe4d284c912 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -5,7 +5,7 @@ use zksync_types::{web3, L1BatchNumber, Transaction, H256, U256}; use zksync_utils::u256_to_h256; use zksync_vm_interface::{ storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, - L1BatchEnv, L2BlockEnv, SystemEnv, + L1BatchEnv, L2BlockEnv, SystemEnv, VmFactory, }; /// Handler for [`VmDump`]. @@ -64,7 +64,6 @@ impl VmStorageDump { } } - #[allow(dead_code)] // FIXME pub(crate) fn into_storage(self) -> InMemoryStorage { let mut storage = InMemoryStorage::default(); for (key, (value, enum_index)) in self.read_storage_keys { @@ -126,4 +125,29 @@ impl VmDump { pub(crate) fn set_storage(&mut self, storage: VmStorageDump) { self.storage = storage; } + + /// Plays back this dump on the specified VM. This prints some debug data to stdout, so should only be used in tests. + pub fn play_back>>(self) -> Vm { + let storage = self.storage.into_storage(); + let storage = StorageView::new(storage).to_rc_ptr(); + let mut vm = Vm::new(self.l1_batch_env, self.system_env, storage); + for action in self.actions { + println!("Executing action: {action:?}"); + match action { + VmAction::Transaction(tx) => { + let tx_hash = tx.hash(); + let (compression_result, _) = + vm.execute_transaction_with_bytecode_compression(*tx, true); + if let Err(err) = compression_result { + panic!("Failed compressing bytecodes for transaction {tx_hash:?}: {err}"); + } + } + VmAction::Block(block) => { + vm.start_new_l2_block(block); + } + } + } + vm.finish_batch(); + vm + } } diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index a10db3088ceb..a6660e242e00 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -209,7 +209,7 @@ impl Harness { fn sanity_check_vm() -> (Vm, Harness) where - Vm: VmInterface + VmFactory>, + Vm: VmFactory>, { let system_env = default_system_env(); let l1_batch_env = default_l1_batch(L1BatchNumber(1)); @@ -259,13 +259,19 @@ fn shadow_vm_basics() { let dump = vm.dump_state(); Harness::assert_dump(&dump); + // Test standard playback functionality. + let replayed_dump = dump + .clone() + .play_back::>() + .dump_state(); + pretty_assertions::assert_eq!(replayed_dump, dump); + + // Check that the VM executes identically when reading from the original storage and one restored from the dump. let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); harness.setup_storage(&mut storage); let storage = StorageView::new(storage).to_rc_ptr(); - let dump_storage = dump.storage.clone().into_storage(); let dump_storage = StorageView::new(dump_storage).to_rc_ptr(); - // Check that the VM executes identically when reading from the original storage and one restored from the dump. let mut vm = ShadowVm::<_, HistoryDisabled, ReferenceVm>::with_custom_shadow( dump.l1_batch_env.clone(), dump.system_env.clone(), From ab4adc5d9e7ea93c84dacf57d080ffdf5612d64e Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 22 Aug 2024 15:35:31 +0300 Subject: [PATCH 13/24] Document `VmStorageDump` contents --- core/lib/multivm/src/dump.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 4fe4d284c912..72fa32f946e2 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -11,11 +11,18 @@ use zksync_vm_interface::{ /// Handler for [`VmDump`]. pub type VmDumpHandler = Arc; -/// Part of the VM dump representing the storage oracle. +/// Part of the VM dump representing the storage oracle for a particular VM run. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub(crate) struct VmStorageDump { + /// Only existing keys (ones with an assigned enum index) are recorded. pub(crate) read_storage_keys: HashMap, + /// Repeatedly written keys together with their enum indices. Only includes keys that were not read + /// (i.e., absent from `read_storage_keys`). Since `InMemoryStorage` returns `is_write_initial == true` + /// for all unknown keys, we don't need to record initial writes. + /// + /// Al least when the Fast VM is involved, this map always looks to be empty because the VM reads values + /// for all written keys (i.e., they all will be added to `read_storage_keys`). pub(crate) repeated_writes: HashMap, pub(crate) factory_deps: HashMap, } From 950bc840fc6cce1beb84fb2a21d1d87b603e0666 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 27 Aug 2024 15:52:14 +0300 Subject: [PATCH 14/24] Sketch `DumpingVm` --- core/lib/multivm/src/dump.rs | 258 ++++++++++++++---- core/lib/multivm/src/versions/shadow/mod.rs | 48 +--- core/lib/multivm/src/versions/shadow/tests.rs | 40 +-- 3 files changed, 230 insertions(+), 116 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 72fa32f946e2..c33cd4dbc5ee 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,16 +1,22 @@ use std::{collections::HashMap, sync::Arc}; use serde::{Deserialize, Serialize}; -use zksync_types::{web3, L1BatchNumber, Transaction, H256, U256}; +use zksync_types::{ + block::L2BlockExecutionData, web3, L1BatchNumber, L2BlockNumber, Transaction, H256, U256, +}; use zksync_utils::u256_to_h256; -use zksync_vm_interface::{ + +use crate::interface::{ storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, - L1BatchEnv, L2BlockEnv, SystemEnv, VmFactory, + BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, + FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, + VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, }; /// Handler for [`VmDump`]. pub type VmDumpHandler = Arc; +// FIXME: unite with storage snapshots /// Part of the VM dump representing the storage oracle for a particular VM run. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] @@ -87,74 +93,230 @@ impl VmStorageDump { } } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq))] -#[serde(rename_all = "snake_case")] -pub(crate) enum VmAction { - Block(L2BlockEnv), - Transaction(Box), -} - /// VM dump allowing to re-run the VM on the same inputs. Opaque, but can be (de)serialized. #[derive(Debug, Clone, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub struct VmDump { pub(crate) l1_batch_env: L1BatchEnv, pub(crate) system_env: SystemEnv, - pub(crate) actions: Vec, + pub(crate) l2_blocks: Vec, #[serde(flatten)] pub(crate) storage: VmStorageDump, } impl VmDump { - pub(crate) fn new(l1_batch_env: L1BatchEnv, system_env: SystemEnv) -> Self { - Self { - l1_batch_env, - system_env, - actions: vec![], - storage: VmStorageDump::default(), - } - } - pub fn l1_batch_number(&self) -> L1BatchNumber { self.l1_batch_env.number } - pub(crate) fn push_transaction(&mut self, tx: Transaction) { - let tx = VmAction::Transaction(Box::new(tx)); - self.actions.push(tx); - } - - pub(crate) fn push_block(&mut self, block: L2BlockEnv) { - self.actions.push(VmAction::Block(block)); - } - - pub(crate) fn set_storage(&mut self, storage: VmStorageDump) { - self.storage = storage; - } - /// Plays back this dump on the specified VM. This prints some debug data to stdout, so should only be used in tests. pub fn play_back>>(self) -> Vm { let storage = self.storage.into_storage(); let storage = StorageView::new(storage).to_rc_ptr(); let mut vm = Vm::new(self.l1_batch_env, self.system_env, storage); - for action in self.actions { - println!("Executing action: {action:?}"); - match action { - VmAction::Transaction(tx) => { - let tx_hash = tx.hash(); - let (compression_result, _) = - vm.execute_transaction_with_bytecode_compression(*tx, true); - if let Err(err) = compression_result { - panic!("Failed compressing bytecodes for transaction {tx_hash:?}: {err}"); - } - } - VmAction::Block(block) => { - vm.start_new_l2_block(block); + Self::play_back_blocks(self.l2_blocks, &mut vm); + vm + } + + pub(crate) fn play_back_blocks( + l2_blocks: Vec, + vm: &mut impl VmInterface, + ) { + for (i, l2_block) in l2_blocks.into_iter().enumerate() { + if i > 0 { + // First block is already set. + vm.start_new_l2_block(L2BlockEnv { + number: l2_block.number.0, + timestamp: l2_block.timestamp, + prev_block_hash: l2_block.prev_block_hash, + max_virtual_blocks_to_create: l2_block.virtual_blocks, + }); + } + + for tx in l2_block.txs { + let tx_hash = tx.hash(); + let (compression_result, _) = + vm.execute_transaction_with_bytecode_compression(tx, true); + if let Err(err) = compression_result { + panic!("Failed compressing bytecodes for transaction {tx_hash:?}: {err}"); } } } vm.finish_batch(); - vm + } +} + +#[derive(Debug, Clone, Copy)] +struct L2BlocksSnapshot { + block_count: usize, + tx_count_in_last_block: usize, +} + +#[derive(Debug)] +pub(crate) struct DumpingVm { + storage: StoragePtr>, + inner: Vm, + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + l2_blocks: Vec, + l2_blocks_snapshot: Option, +} + +impl DumpingVm { + fn last_block_mut(&mut self) -> &mut L2BlockExecutionData { + self.l2_blocks.last_mut().unwrap() + } + + fn record_transaction(&mut self, tx: Transaction) { + self.last_block_mut().txs.push(tx); + } + + pub fn dump_state(&self) -> VmDump { + VmDump { + l1_batch_env: self.l1_batch_env.clone(), + system_env: self.system_env.clone(), + l2_blocks: self.l2_blocks.clone(), + storage: VmStorageDump::new(&self.storage, vec![]), // FIXME + } + } +} + +impl VmInterface for DumpingVm { + type TracerDispatcher = Vm::TracerDispatcher; + + fn push_transaction(&mut self, tx: Transaction) { + self.record_transaction(tx.clone()); + self.inner.push_transaction(tx); + } + + fn execute(&mut self, execution_mode: VmExecutionMode) -> VmExecutionResultAndLogs { + self.inner.execute(execution_mode) + } + + fn inspect( + &mut self, + dispatcher: Self::TracerDispatcher, + execution_mode: VmExecutionMode, + ) -> VmExecutionResultAndLogs { + self.inner.inspect(dispatcher, execution_mode) + } + + fn get_bootloader_memory(&self) -> BootloaderMemory { + self.inner.get_bootloader_memory() + } + + fn get_last_tx_compressed_bytecodes(&self) -> Vec { + self.inner.get_last_tx_compressed_bytecodes() + } + + fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { + self.l2_blocks.push(L2BlockExecutionData { + number: L2BlockNumber(l2_block_env.number), + timestamp: l2_block_env.timestamp, + prev_block_hash: l2_block_env.prev_block_hash, + virtual_blocks: l2_block_env.max_virtual_blocks_to_create, + txs: vec![], + }); + self.inner.start_new_l2_block(l2_block_env); + } + + fn get_current_execution_state(&self) -> CurrentExecutionState { + self.inner.get_current_execution_state() + } + + fn execute_transaction_with_bytecode_compression( + &mut self, + tx: Transaction, + with_compression: bool, + ) -> ( + Result<(), BytecodeCompressionError>, + VmExecutionResultAndLogs, + ) { + self.record_transaction(tx.clone()); + self.inner + .execute_transaction_with_bytecode_compression(tx, with_compression) + } + + fn inspect_transaction_with_bytecode_compression( + &mut self, + tracer: Self::TracerDispatcher, + tx: Transaction, + with_compression: bool, + ) -> ( + Result<(), BytecodeCompressionError>, + VmExecutionResultAndLogs, + ) { + self.record_transaction(tx.clone()); + self.inner + .inspect_transaction_with_bytecode_compression(tracer, tx, with_compression) + } + + fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + self.inner.record_vm_memory_metrics() + } + + fn gas_remaining(&self) -> u32 { + self.inner.gas_remaining() + } + + fn finish_batch(&mut self) -> FinishedL1Batch { + self.inner.finish_batch() + } +} + +impl VmInterfaceHistoryEnabled for DumpingVm { + fn make_snapshot(&mut self) { + self.l2_blocks_snapshot = Some(L2BlocksSnapshot { + block_count: self.l2_blocks.len(), + tx_count_in_last_block: self.last_block_mut().txs.len(), + }); + self.inner.make_snapshot(); + } + + fn rollback_to_the_latest_snapshot(&mut self) { + self.inner.rollback_to_the_latest_snapshot(); + let snapshot = self + .l2_blocks_snapshot + .take() + .expect("rollback w/o snapshot"); + self.l2_blocks.truncate(snapshot.block_count); + assert_eq!( + self.l2_blocks.len(), + snapshot.block_count, + "L2 blocks were removed after creating a snapshot" + ); + self.last_block_mut() + .txs + .truncate(snapshot.tx_count_in_last_block); + } + + fn pop_snapshot_no_rollback(&mut self) { + self.inner.pop_snapshot_no_rollback(); + self.l2_blocks_snapshot = None; + } +} + +impl>> VmFactory> for DumpingVm { + fn new( + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + storage: StoragePtr>, + ) -> Self { + let inner = Vm::new(l1_batch_env.clone(), system_env.clone(), storage.clone()); + let first_block = L2BlockExecutionData { + number: L2BlockNumber(l1_batch_env.first_l2_block.number), + timestamp: l1_batch_env.first_l2_block.timestamp, + prev_block_hash: l1_batch_env.first_l2_block.prev_block_hash, + virtual_blocks: l1_batch_env.first_l2_block.max_virtual_blocks_to_create, + txs: vec![], + }; + Self { + l1_batch_env, + system_env, + l2_blocks: vec![first_block], + l2_blocks_snapshot: None, + storage, + inner, + } } } diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/multivm/src/versions/shadow/mod.rs index f4d872c1a39b..9c4441dc4e4a 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -5,10 +5,10 @@ use std::{ sync::Arc, }; -use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction, U256}; +use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; use crate::{ - dump::{VmDump, VmDumpHandler, VmStorageDump}, + dump::{DumpingVm, VmDump, VmDumpHandler}, interface::{ storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, @@ -24,15 +24,13 @@ use crate::{ #[cfg(test)] mod tests; -struct VmWithReporting { +struct VmWithReporting { vm: Shadow, - main_vm_storage: StoragePtr>, - partial_dump: VmDump, dump_handler: VmDumpHandler, panic_on_divergence: bool, } -impl fmt::Debug for VmWithReporting { +impl fmt::Debug for VmWithReporting { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter .debug_struct("VmWithReporting") @@ -42,19 +40,11 @@ impl fmt::Debug for VmWithReporting VmWithReporting { - fn finalize_dump(&mut self, used_contract_hashes: Vec) { - self.partial_dump.set_storage(VmStorageDump::new( - &self.main_vm_storage, - used_contract_hashes, - )); - } - - fn report(mut self, used_contract_hashes: Vec, err: anyhow::Error) { - let batch_number = self.partial_dump.l1_batch_number(); +impl VmWithReporting { + fn report(self, dump: VmDump, err: anyhow::Error) { + let batch_number = dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - self.finalize_dump(used_contract_hashes); - (self.dump_handler)(self.partial_dump); + (self.dump_handler)(dump); if self.panic_on_divergence { panic!("{err:?}"); @@ -69,8 +59,8 @@ impl VmWithReporting { #[derive(Debug)] pub struct ShadowVm>> { - main: vm_latest::Vm, H>, - shadow: RefCell>>, + main: DumpingVm, H>>, + shadow: RefCell>>, } impl ShadowVm @@ -101,15 +91,7 @@ where self.shadow .take() .unwrap() - .report(self.main.get_used_contracts(), err); - } - - #[cfg(test)] - fn dump_state(&mut self) -> VmDump { - let mut borrow = self.shadow.borrow_mut(); - let borrow = borrow.as_mut().expect("VM execution diverged"); - borrow.finalize_dump(self.main.get_used_contracts()); - borrow.partial_dump.clone() + .report(self.main.dump_state(), err); } } @@ -128,12 +110,10 @@ where where Shadow: VmFactory, { - let main = vm_latest::Vm::new(batch_env.clone(), system_env.clone(), storage.clone()); + let main = DumpingVm::new(batch_env.clone(), system_env.clone(), storage.clone()); let shadow = Shadow::new(batch_env.clone(), system_env.clone(), shadow_storage); let shadow = VmWithReporting { vm: shadow, - main_vm_storage: storage, - partial_dump: VmDump::new(batch_env, system_env), dump_handler: Arc::new(drop), // We don't want to log the dump (it's too large), so there's no trivial way to handle it panic_on_divergence: true, }; @@ -169,7 +149,6 @@ where fn push_transaction(&mut self, tx: Transaction) { if let Some(shadow) = self.shadow.get_mut() { - shadow.partial_dump.push_transaction(tx.clone()); shadow.vm.push_transaction(tx.clone()); } self.main.push_transaction(tx); @@ -246,7 +225,6 @@ where fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { self.main.start_new_l2_block(l2_block_env); if let Some(shadow) = self.shadow.get_mut() { - shadow.partial_dump.push_block(l2_block_env); shadow.vm.start_new_l2_block(l2_block_env); } } @@ -279,7 +257,6 @@ where .main .execute_transaction_with_bytecode_compression(tx.clone(), with_compression); if let Some(shadow) = self.shadow.get_mut() { - shadow.partial_dump.push_transaction(tx.clone()); let shadow_result = shadow .vm .execute_transaction_with_bytecode_compression(tx, with_compression); @@ -311,7 +288,6 @@ where with_compression, ); if let Some(shadow) = self.shadow.get_mut() { - shadow.partial_dump.push_transaction(tx.clone()); let shadow_result = shadow.vm.inspect_transaction_with_bytecode_compression( Shadow::TracerDispatcher::default(), tx, diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index a6660e242e00..fde55f42b6e3 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -15,7 +15,6 @@ use zksync_utils::bytecode::hash_bytecode; use super::*; use crate::{ - dump::VmAction, interface::{storage::InMemoryStorage, ExecutionResult}, utils::get_max_gas_per_pubdata_byte, versions::testonly::{ @@ -94,19 +93,9 @@ impl Harness { fn assert_dump(dump: &VmDump) { assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); - assert_matches!( - dump.actions.as_slice(), - [ - VmAction::Transaction(_), - VmAction::Block(_), - VmAction::Transaction(_), - VmAction::Transaction(_), - VmAction::Block(_), - VmAction::Transaction(_), - VmAction::Transaction(_), - VmAction::Block(_), - ] - ); + let tx_counts_per_block: Vec<_> = + dump.l2_blocks.iter().map(|block| block.txs.len()).collect(); + assert_eq!(tx_counts_per_block, [1, 2, 2, 0]); assert!(!dump.storage.read_storage_keys.is_empty()); assert!(!dump.storage.factory_deps.is_empty()); @@ -255,14 +244,15 @@ fn sanity_check_shadow_vm() { #[test] fn shadow_vm_basics() { - let (mut vm, harness) = sanity_check_vm::>(); - let dump = vm.dump_state(); + let (vm, harness) = sanity_check_vm::>(); + let dump = vm.main.dump_state(); Harness::assert_dump(&dump); // Test standard playback functionality. let replayed_dump = dump .clone() .play_back::>() + .main .dump_state(); pretty_assertions::assert_eq!(replayed_dump, dump); @@ -279,21 +269,7 @@ fn shadow_vm_basics() { dump_storage, ); - for action in dump.actions.clone() { - match action { - VmAction::Transaction(tx) => { - let (compression_result, _) = - vm.execute_transaction_with_bytecode_compression(*tx, true); - compression_result.unwrap(); - } - VmAction::Block(block) => { - vm.start_new_l2_block(block); - } - } - vm.dump_state(); // Check that a dump can be created at any point in batch execution - } - vm.finish_batch(); - - let new_dump = vm.dump_state(); + VmDump::play_back_blocks(dump.l2_blocks.clone(), &mut vm); + let new_dump = vm.main.dump_state(); pretty_assertions::assert_eq!(new_dump, dump); } From ad0b88d704e5502ca60a347713a655f764aa9069 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 27 Aug 2024 16:31:21 +0300 Subject: [PATCH 15/24] Fix decommitted bytecodes computation --- core/lib/multivm/src/dump.rs | 55 ++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index c33cd4dbc5ee..1accef5766a5 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -5,18 +5,41 @@ use zksync_types::{ block::L2BlockExecutionData, web3, L1BatchNumber, L2BlockNumber, Transaction, H256, U256, }; use zksync_utils::u256_to_h256; - -use crate::interface::{ - storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, - BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, - FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, - VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, +use zksync_vm_interface::storage::WriteStorage; + +use crate::{ + interface::{ + storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView}, + BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, + FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, + VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, + VmMemoryMetrics, + }, + vm_fast, vm_latest, HistoryMode, }; +/// VM that tracks decommitment of bytecodes during execution. This is required to create a [`VmDump`]. +pub(crate) trait VmTrackingContracts: VmInterface { + /// Returns hashes of all decommitted bytecodes. + fn used_contract_hashes(&self) -> Vec; +} + +impl VmTrackingContracts for vm_latest::Vm { + fn used_contract_hashes(&self) -> Vec { + self.get_used_contracts() + } +} + +impl VmTrackingContracts for vm_fast::Vm { + fn used_contract_hashes(&self) -> Vec { + self.decommitted_hashes().collect() + } +} + /// Handler for [`VmDump`]. pub type VmDumpHandler = Arc; -// FIXME: unite with storage snapshots +// FIXME: use storage snapshots (https://github.com/matter-labs/zksync-era/pull/2724) /// Part of the VM dump representing the storage oracle for a particular VM run. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] @@ -162,7 +185,7 @@ pub(crate) struct DumpingVm { l2_blocks_snapshot: Option, } -impl DumpingVm { +impl DumpingVm { fn last_block_mut(&mut self) -> &mut L2BlockExecutionData { self.l2_blocks.last_mut().unwrap() } @@ -176,12 +199,12 @@ impl DumpingVm { l1_batch_env: self.l1_batch_env.clone(), system_env: self.system_env.clone(), l2_blocks: self.l2_blocks.clone(), - storage: VmStorageDump::new(&self.storage, vec![]), // FIXME + storage: VmStorageDump::new(&self.storage, self.inner.used_contract_hashes()), } } } -impl VmInterface for DumpingVm { +impl VmInterface for DumpingVm { type TracerDispatcher = Vm::TracerDispatcher; fn push_transaction(&mut self, tx: Transaction) { @@ -264,7 +287,11 @@ impl VmInterface for DumpingVm { } } -impl VmInterfaceHistoryEnabled for DumpingVm { +impl VmInterfaceHistoryEnabled for DumpingVm +where + S: ReadStorage, + Vm: VmInterfaceHistoryEnabled + VmTrackingContracts, +{ fn make_snapshot(&mut self) { self.l2_blocks_snapshot = Some(L2BlocksSnapshot { block_count: self.l2_blocks.len(), @@ -296,7 +323,11 @@ impl VmInterfaceHistoryEnabled fo } } -impl>> VmFactory> for DumpingVm { +impl VmFactory> for DumpingVm +where + S: ReadStorage, + Vm: VmFactory> + VmTrackingContracts, +{ fn new( l1_batch_env: L1BatchEnv, system_env: SystemEnv, From b32ef56f510fbf178ef2d999ef921dfe71a994e7 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 27 Aug 2024 17:05:22 +0300 Subject: [PATCH 16/24] Wrap dump handler in newtype --- core/lib/multivm/src/dump.rs | 36 +++++++++++++++--- core/lib/multivm/src/versions/shadow/mod.rs | 5 +-- .../src/batch_executor/main_executor.rs | 37 +++---------------- core/node/vm_runner/src/impls/playground.rs | 7 ++-- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 1accef5766a5..70ba85b582d8 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, sync::Arc}; +use std::{collections::HashMap, fmt, sync::Arc}; use serde::{Deserialize, Serialize}; use zksync_types::{ @@ -36,9 +36,6 @@ impl VmTrackingContracts for vm_fast::Vm { } } -/// Handler for [`VmDump`]. -pub type VmDumpHandler = Arc; - // FIXME: use storage snapshots (https://github.com/matter-labs/zksync-era/pull/2724) /// Part of the VM dump representing the storage oracle for a particular VM run. #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -132,7 +129,7 @@ impl VmDump { self.l1_batch_env.number } - /// Plays back this dump on the specified VM. This prints some debug data to stdout, so should only be used in tests. + /// Plays back this dump on the specified VM. pub fn play_back>>(self) -> Vm { let storage = self.storage.into_storage(); let storage = StorageView::new(storage).to_rc_ptr(); @@ -169,12 +166,41 @@ impl VmDump { } } +/// Handler for [`VmDump`]. +#[derive(Clone)] +pub struct VmDumpHandler(Arc); + +impl fmt::Debug for VmDumpHandler { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.debug_tuple("VmDumpHandler").field(&"_").finish() + } +} + +/// Default no-op handler. +impl Default for VmDumpHandler { + fn default() -> Self { + Self(Arc::new(drop)) + } +} + +impl VmDumpHandler { + /// Creates a new handler from the provided closure. + pub fn new(f: impl Fn(VmDump) + Send + Sync + 'static) -> Self { + Self(Arc::new(f)) + } + + pub(crate) fn handle(&self, dump: VmDump) { + self.0(dump); + } +} + #[derive(Debug, Clone, Copy)] struct L2BlocksSnapshot { block_count: usize, tx_count_in_last_block: usize, } +/// VM wrapper that can create [`VmDump`]s during execution. #[derive(Debug)] pub(crate) struct DumpingVm { storage: StoragePtr>, diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/multivm/src/versions/shadow/mod.rs index 9c4441dc4e4a..b9c378af0519 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -2,7 +2,6 @@ use std::{ cell::RefCell, collections::{BTreeMap, BTreeSet}, fmt, - sync::Arc, }; use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; @@ -44,7 +43,7 @@ impl VmWithReporting { fn report(self, dump: VmDump, err: anyhow::Error) { let batch_number = dump.l1_batch_number(); tracing::error!("VM execution diverged on batch #{batch_number}!"); - (self.dump_handler)(dump); + self.dump_handler.handle(dump); if self.panic_on_divergence { panic!("{err:?}"); @@ -114,7 +113,7 @@ where let shadow = Shadow::new(batch_env.clone(), system_env.clone(), shadow_storage); let shadow = VmWithReporting { vm: shadow, - dump_handler: Arc::new(drop), // We don't want to log the dump (it's too large), so there's no trivial way to handle it + dump_handler: VmDumpHandler::default(), panic_on_divergence: true, }; Self { diff --git a/core/node/state_keeper/src/batch_executor/main_executor.rs b/core/node/state_keeper/src/batch_executor/main_executor.rs index 31d920abdc3d..57d18a9a7b80 100644 --- a/core/node/state_keeper/src/batch_executor/main_executor.rs +++ b/core/node/state_keeper/src/batch_executor/main_executor.rs @@ -1,10 +1,10 @@ -use std::{fmt, sync::Arc}; +use std::sync::Arc; use anyhow::Context as _; use once_cell::sync::OnceCell; use tokio::sync::mpsc; use zksync_multivm::{ - dump::{VmDump, VmDumpHandler}, + dump::VmDumpHandler, interface::{ storage::{ReadStorage, StorageView}, Call, CompressedBytecodeInfo, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, @@ -25,7 +25,7 @@ use crate::{ /// The default implementation of [`BatchExecutor`]. /// Creates a "real" batch executor which maintains the VM (as opposed to the test builder which doesn't use the VM). -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct MainBatchExecutor { save_call_traces: bool, /// Whether batch executor would allow transactions with bytecode that cannot be compressed. @@ -39,20 +39,6 @@ pub struct MainBatchExecutor { dump_handler: Option, } -impl fmt::Debug for MainBatchExecutor { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter - .debug_struct("MainBatchExecutor") - .field("save_call_traces", &self.save_call_traces) - .field( - "optional_bytecode_compression", - &self.optional_bytecode_compression, - ) - .field("fast_vm_mode", &self.fast_vm_mode) - .finish_non_exhaustive() - } -} - impl MainBatchExecutor { pub fn new(save_call_traces: bool, optional_bytecode_compression: bool) -> Self { Self { @@ -72,7 +58,7 @@ impl MainBatchExecutor { self.fast_vm_mode = fast_vm_mode; } - pub fn set_dump_handler(&mut self, handler: Arc) { + pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { tracing::info!("Set VM dumps handler"); self.dump_handler = Some(handler); } @@ -115,6 +101,7 @@ struct TransactionOutput { /// /// One `CommandReceiver` can execute exactly one batch, so once the batch is sealed, a new `CommandReceiver` object must /// be constructed. +#[derive(Debug)] struct CommandReceiver { save_call_traces: bool, optional_bytecode_compression: bool, @@ -123,20 +110,6 @@ struct CommandReceiver { commands: mpsc::Receiver, } -impl fmt::Debug for CommandReceiver { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter - .debug_struct("CommandReceiver") - .field("save_call_traces", &self.save_call_traces) - .field( - "optional_bytecode_compression", - &self.optional_bytecode_compression, - ) - .field("fast_vm_mode", &self.fast_vm_mode) - .finish_non_exhaustive() - } -} - impl CommandReceiver { pub(super) fn run( mut self, diff --git a/core/node/vm_runner/src/impls/playground.rs b/core/node/vm_runner/src/impls/playground.rs index efc31dfba970..18aab0632165 100644 --- a/core/node/vm_runner/src/impls/playground.rs +++ b/core/node/vm_runner/src/impls/playground.rs @@ -15,7 +15,7 @@ use tokio::{ }; use zksync_dal::{Connection, ConnectionPool, Core, CoreDal}; use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthCheck}; -use zksync_multivm::dump::VmDump; +use zksync_multivm::dump::{VmDump, VmDumpHandler}; use zksync_object_store::{Bucket, ObjectStore}; use zksync_state::RocksdbStorage; use zksync_state_keeper::{MainBatchExecutor, StateKeeperOutputHandler, UpdatesManager}; @@ -91,14 +91,15 @@ impl VmPlayground { if let Some(store) = dumps_object_store { tracing::info!("Using object store for VM dumps: {store:?}"); - batch_executor.set_dump_handler(Arc::new(move |dump| { + let handler = VmDumpHandler::new(move |dump| { if let Err(err) = handle.block_on(Self::dump_vm_state(&*store, &dump)) { let l1_batch_number = dump.l1_batch_number(); tracing::error!( "Saving VM dump for L1 batch #{l1_batch_number} failed: {err:#}" ); } - })); + }); + batch_executor.set_dump_handler(handler); } let io = VmPlaygroundIo { From 82133c9b5a11e721812969b027d83fa94f821146 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 27 Aug 2024 17:28:26 +0300 Subject: [PATCH 17/24] Generalize main VM in `ShadowVm` (again) --- core/lib/multivm/src/dump.rs | 2 +- core/lib/multivm/src/versions/shadow/mod.rs | 36 ++++++++++--------- core/lib/multivm/src/versions/shadow/tests.rs | 10 +++--- core/lib/multivm/src/vm_instance.rs | 2 +- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 70ba85b582d8..6a8303a87587 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -19,7 +19,7 @@ use crate::{ }; /// VM that tracks decommitment of bytecodes during execution. This is required to create a [`VmDump`]. -pub(crate) trait VmTrackingContracts: VmInterface { +pub trait VmTrackingContracts: VmInterface { /// Returns hashes of all decommitted bytecodes. fn used_contract_hashes(&self) -> Vec; } diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/multivm/src/versions/shadow/mod.rs index b9c378af0519..e8585604d216 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -7,7 +7,7 @@ use std::{ use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; use crate::{ - dump::{DumpingVm, VmDump, VmDumpHandler}, + dump::{DumpingVm, VmDump, VmDumpHandler, VmTrackingContracts}, interface::{ storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, @@ -15,9 +15,7 @@ use crate::{ VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, }, - vm_fast, vm_latest, - vm_latest::HistoryEnabled, - HistoryMode, + vm_fast, }; #[cfg(test)] @@ -56,16 +54,20 @@ impl VmWithReporting { } } +/// Shadowed VM that executes 2 VMs for each operation and compares their outputs. +/// +/// If a divergence is detected, the VM state is dumped using [a pluggable handler](Self::set_dump_handler()), +/// after which the VM either panics, or drops the shadowed VM, depending on [`Self::set_panic_on_divergence()`] setting. #[derive(Debug)] -pub struct ShadowVm>> { - main: DumpingVm, H>>, +pub struct ShadowVm>> { + main: DumpingVm, shadow: RefCell>>, } -impl ShadowVm +impl ShadowVm where S: ReadStorage, - H: HistoryMode, + Main: VmTrackingContracts, Shadow: VmInterface, { pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { @@ -94,10 +96,10 @@ where } } -impl ShadowVm +impl ShadowVm where S: ReadStorage, - H: HistoryMode, + Main: VmFactory> + VmTrackingContracts, Shadow: VmInterface, { pub fn with_custom_shadow( @@ -123,10 +125,10 @@ where } } -impl VmFactory> for ShadowVm +impl VmFactory> for ShadowVm where S: ReadStorage, - H: HistoryMode, + Main: VmFactory> + VmTrackingContracts, { fn new( batch_env: L1BatchEnv, @@ -138,13 +140,13 @@ where } /// **Important.** This doesn't properly handle tracers; they are not passed to the shadow VM! -impl VmInterface for ShadowVm +impl VmInterface for ShadowVm where S: ReadStorage, - H: HistoryMode, + Main: VmTrackingContracts, Shadow: VmInterface, { - type TracerDispatcher = , H> as VmInterface>::TracerDispatcher; + type TracerDispatcher =
::TracerDispatcher; fn push_transaction(&mut self, tx: Transaction) { if let Some(shadow) = self.shadow.get_mut() { @@ -507,9 +509,11 @@ impl UniqueStorageLogs { } } -impl VmInterfaceHistoryEnabled for ShadowVm +impl VmInterfaceHistoryEnabled for ShadowVm where S: ReadStorage, + Main: VmInterfaceHistoryEnabled + VmTrackingContracts, + Shadow: VmInterfaceHistoryEnabled, { fn make_snapshot(&mut self) { if let Some(shadow) = self.shadow.get_mut() { diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index fde55f42b6e3..d1f28aa97bb8 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -20,7 +20,7 @@ use crate::{ versions::testonly::{ default_l1_batch, default_system_env, make_account_rich, ContractToDeploy, }, - vm_latest::{self, HistoryDisabled}, + vm_latest::{self, HistoryEnabled}, }; type ReferenceVm = vm_latest::Vm, HistoryEnabled>; @@ -233,7 +233,7 @@ fn sanity_check_shadow_vm() { // We need separate storage views since they are mutated by the VM execution let main_storage = StorageView::new(&storage).to_rc_ptr(); let shadow_storage = StorageView::new(&storage).to_rc_ptr(); - let mut vm = ShadowVm::<_, HistoryDisabled, ReferenceVm<_>>::with_custom_shadow( + let mut vm = ShadowVm::<_, ReferenceVm<_>, ReferenceVm<_>>::with_custom_shadow( l1_batch_env, system_env, main_storage, @@ -244,14 +244,14 @@ fn sanity_check_shadow_vm() { #[test] fn shadow_vm_basics() { - let (vm, harness) = sanity_check_vm::>(); + let (vm, harness) = sanity_check_vm::>(); let dump = vm.main.dump_state(); Harness::assert_dump(&dump); // Test standard playback functionality. let replayed_dump = dump .clone() - .play_back::>() + .play_back::>() .main .dump_state(); pretty_assertions::assert_eq!(replayed_dump, dump); @@ -262,7 +262,7 @@ fn shadow_vm_basics() { let storage = StorageView::new(storage).to_rc_ptr(); let dump_storage = dump.storage.clone().into_storage(); let dump_storage = StorageView::new(dump_storage).to_rc_ptr(); - let mut vm = ShadowVm::<_, HistoryDisabled, ReferenceVm>::with_custom_shadow( + let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm>::with_custom_shadow( dump.l1_batch_env.clone(), dump.system_env.clone(), storage, diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 67d72c0c0a3f..64d133791d8b 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -25,7 +25,7 @@ pub enum VmInstance { Vm1_4_2(crate::vm_1_4_2::Vm, H>), Vm1_5_0(crate::vm_latest::Vm, H>), VmFast(crate::vm_fast::Vm>), - ShadowedVmFast(ShadowVm), + ShadowedVmFast(ShadowVm, H>>), } macro_rules! dispatch_vm { From 82eee6acb39987e7eadad8cad4f79c56c14796e4 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 28 Aug 2024 11:47:22 +0300 Subject: [PATCH 18/24] Replace dump handler with divergence handler --- core/lib/basic_types/src/vm.rs | 2 +- core/lib/multivm/src/dump.rs | 30 +--- core/lib/multivm/src/lib.rs | 6 +- core/lib/multivm/src/versions/shadow/mod.rs | 145 +++++++++++------- core/lib/multivm/src/vm_instance.rs | 3 +- .../src/batch_executor/main_executor.rs | 20 +-- core/node/vm_runner/src/impls/playground.rs | 30 ++-- 7 files changed, 127 insertions(+), 109 deletions(-) diff --git a/core/lib/basic_types/src/vm.rs b/core/lib/basic_types/src/vm.rs index a1782a3199b9..fdc88e506e15 100644 --- a/core/lib/basic_types/src/vm.rs +++ b/core/lib/basic_types/src/vm.rs @@ -39,5 +39,5 @@ pub enum FastVmMode { Shadow, /// Run both the new and old VM and compare their outputs for each transaction execution. /// The new VM will get dropped on divergence. - ShadowLenient, + ShadowLenient, // FIXME: remove } diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 6a8303a87587..16686692eb3e 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, fmt, sync::Arc}; +use std::collections::HashMap; use serde::{Deserialize, Serialize}; use zksync_types::{ @@ -166,34 +166,6 @@ impl VmDump { } } -/// Handler for [`VmDump`]. -#[derive(Clone)] -pub struct VmDumpHandler(Arc); - -impl fmt::Debug for VmDumpHandler { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.debug_tuple("VmDumpHandler").field(&"_").finish() - } -} - -/// Default no-op handler. -impl Default for VmDumpHandler { - fn default() -> Self { - Self(Arc::new(drop)) - } -} - -impl VmDumpHandler { - /// Creates a new handler from the provided closure. - pub fn new(f: impl Fn(VmDump) + Send + Sync + 'static) -> Self { - Self(Arc::new(f)) - } - - pub(crate) fn handle(&self, dump: VmDump) { - self.0(dump); - } -} - #[derive(Debug, Clone, Copy)] struct L2BlocksSnapshot { block_count: usize, diff --git a/core/lib/multivm/src/lib.rs b/core/lib/multivm/src/lib.rs index 0a6bfc949356..68158038810d 100644 --- a/core/lib/multivm/src/lib.rs +++ b/core/lib/multivm/src/lib.rs @@ -13,8 +13,8 @@ pub use crate::{ tracers::{MultiVMTracer, MultiVmTracerPointer}, }, versions::{ - vm_1_3_2, vm_1_4_1, vm_1_4_2, vm_boojum_integration, vm_fast, vm_latest, vm_m5, vm_m6, - vm_refunds_enhancement, vm_virtual_blocks, + shadow, vm_1_3_2, vm_1_4_1, vm_1_4_2, vm_boojum_integration, vm_fast, vm_latest, vm_m5, + vm_m6, vm_refunds_enhancement, vm_virtual_blocks, }, vm_instance::VmInstance, }; @@ -23,5 +23,5 @@ pub mod dump; mod glue; pub mod tracers; pub mod utils; -pub mod versions; +mod versions; mod vm_instance; diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/multivm/src/versions/shadow/mod.rs index e8585604d216..f109be0e8635 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -2,12 +2,13 @@ use std::{ cell::RefCell, collections::{BTreeMap, BTreeSet}, fmt, + sync::Arc, }; use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; use crate::{ - dump::{DumpingVm, VmDump, VmDumpHandler, VmTrackingContracts}, + dump::{DumpingVm, VmDump, VmTrackingContracts}, interface::{ storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, BootloaderMemory, BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, @@ -21,10 +22,40 @@ use crate::{ #[cfg(test)] mod tests; +/// Handler for [`VmDump`]. +#[derive(Clone)] +pub struct DivergenceHandler(Arc); + +impl fmt::Debug for DivergenceHandler { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.debug_tuple("VmDumpHandler").field(&"_").finish() + } +} + +/// Default handler that panics. +impl Default for DivergenceHandler { + fn default() -> Self { + Self(Arc::new(|err, _| { + // There's no easy way to output the VM dump; it's too large to be logged. + panic!("{err}"); + })) + } +} + +impl DivergenceHandler { + /// Creates a new handler from the provided closure. + pub fn new(f: impl Fn(DivergenceErrors, VmDump) + Send + Sync + 'static) -> Self { + Self(Arc::new(f)) + } + + fn handle(&self, err: DivergenceErrors, dump: VmDump) { + self.0(err, dump); + } +} + struct VmWithReporting { vm: Shadow, - dump_handler: VmDumpHandler, - panic_on_divergence: bool, + divergence_handler: DivergenceHandler, } impl fmt::Debug for VmWithReporting { @@ -32,25 +63,17 @@ impl fmt::Debug for VmWithReporting { formatter .debug_struct("VmWithReporting") .field("vm", &self.vm) - .field("panic_on_divergence", &self.panic_on_divergence) .finish_non_exhaustive() } } impl VmWithReporting { - fn report(self, dump: VmDump, err: anyhow::Error) { - let batch_number = dump.l1_batch_number(); - tracing::error!("VM execution diverged on batch #{batch_number}!"); - self.dump_handler.handle(dump); - - if self.panic_on_divergence { - panic!("{err:?}"); - } else { - tracing::error!("{err:#}"); - tracing::warn!( - "New VM is dropped; following VM actions will be executed only on the main VM" - ); - } + fn report(self, err: DivergenceErrors, dump: VmDump) { + tracing::error!("{err}"); + self.divergence_handler.handle(err, dump); + tracing::warn!( + "New VM is dropped; following VM actions will be executed only on the main VM" + ); } } @@ -70,29 +93,23 @@ where Main: VmTrackingContracts, Shadow: VmInterface, { - pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { + pub fn set_divergence_handler(&mut self, handler: DivergenceHandler) { if let Some(shadow) = self.shadow.get_mut() { - shadow.dump_handler = handler; - } - } - - pub(crate) fn set_panic_on_divergence(&mut self, panic: bool) { - if let Some(shadow) = self.shadow.get_mut() { - shadow.panic_on_divergence = panic; + shadow.divergence_handler = handler; } } /// Mutable ref is not necessary, but it automatically drops potential borrows. - fn report(&mut self, err: anyhow::Error) { + fn report(&mut self, err: DivergenceErrors) { self.report_shared(err); } /// The caller is responsible for dropping any `shadow` borrows beforehand. - fn report_shared(&self, err: anyhow::Error) { + fn report_shared(&self, err: DivergenceErrors) { self.shadow .take() .unwrap() - .report(self.main.dump_state(), err); + .report(err, self.main.dump_state()); } } @@ -115,8 +132,7 @@ where let shadow = Shadow::new(batch_env.clone(), system_env.clone(), shadow_storage); let shadow = VmWithReporting { vm: shadow, - dump_handler: VmDumpHandler::default(), - panic_on_divergence: true, + divergence_handler: DivergenceHandler::default(), }; Self { main, @@ -159,7 +175,7 @@ where let main_result = self.main.execute(execution_mode); if let Some(shadow) = self.shadow.get_mut() { let shadow_result = shadow.vm.execute(execution_mode); - let mut errors = DivergenceErrors::default(); + let mut errors = DivergenceErrors::new(); errors.check_results_match(&main_result, &shadow_result); if let Err(err) = errors.into_result() { let ctx = format!("executing VM with mode {execution_mode:?}"); @@ -179,7 +195,7 @@ where let shadow_result = shadow .vm .inspect(Shadow::TracerDispatcher::default(), execution_mode); - let mut errors = DivergenceErrors::default(); + let mut errors = DivergenceErrors::new(); errors.check_results_match(&main_result, &shadow_result); if let Err(err) = errors.into_result() { @@ -261,7 +277,7 @@ where let shadow_result = shadow .vm .execute_transaction_with_bytecode_compression(tx, with_compression); - let mut errors = DivergenceErrors::default(); + let mut errors = DivergenceErrors::new(); errors.check_results_match(&main_result.1, &shadow_result.1); if let Err(err) = errors.into_result() { let ctx = format!( @@ -294,7 +310,7 @@ where tx, with_compression, ); - let mut errors = DivergenceErrors::default(); + let mut errors = DivergenceErrors::new(); errors.check_results_match(&main_result.1, &shadow_result.1); if let Err(err) = errors.into_result() { let ctx = format!( @@ -328,7 +344,7 @@ where let main_batch = self.main.finish_batch(); if let Some(shadow) = self.shadow.get_mut() { let shadow_batch = shadow.vm.finish_batch(); - let mut errors = DivergenceErrors::default(); + let mut errors = DivergenceErrors::new(); errors.check_results_match( &main_batch.block_tip_execution_result, &shadow_batch.block_tip_execution_result, @@ -361,17 +377,45 @@ where } } -#[must_use = "Should be converted to a `Result`"] -#[derive(Debug, Default)] -pub struct DivergenceErrors(Vec); +#[derive(Debug)] +pub struct DivergenceErrors { + divergences: Vec, + context: Option, +} + +impl fmt::Display for DivergenceErrors { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(context) = &self.context { + write!( + formatter, + "VM execution diverged: {context}: [{}]", + self.divergences.join(", ") + ) + } else { + write!( + formatter, + "VM execution diverged: [{}]", + self.divergences.join(", ") + ) + } + } +} impl DivergenceErrors { - fn single( - context: &str, - main: &T, - shadow: &T, - ) -> anyhow::Result<()> { - let mut this = Self::default(); + fn new() -> Self { + Self { + divergences: vec![], + context: None, + } + } + + fn context(mut self, context: String) -> Self { + self.context = Some(context); + self + } + + fn single(context: &str, main: &T, shadow: &T) -> Result<(), Self> { + let mut this = Self::new(); this.check_match(context, main, shadow); this.into_result() } @@ -406,8 +450,8 @@ impl DivergenceErrors { fn check_match(&mut self, context: &str, main: &T, shadow: &T) { if main != shadow { let comparison = pretty_assertions::Comparison::new(main, shadow); - let err = anyhow::anyhow!("`{context}` mismatch: {comparison}"); - self.0.push(err); + let err = format!("`{context}` mismatch: {comparison}"); + self.divergences.push(err); } } @@ -459,14 +503,11 @@ impl DivergenceErrors { .collect() } - fn into_result(self) -> anyhow::Result<()> { - if self.0.is_empty() { + fn into_result(self) -> Result<(), Self> { + if self.divergences.is_empty() { Ok(()) } else { - Err(anyhow::anyhow!( - "divergence between old VM and new VM execution: [{:?}]", - self.0 - )) + Err(self) } } } diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 64d133791d8b..10e385983bb0 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -266,8 +266,7 @@ impl VmInstance { )) } FastVmMode::Shadow | FastVmMode::ShadowLenient => { - let mut vm = ShadowVm::new(l1_batch_env, system_env, storage_view); - vm.set_panic_on_divergence(matches!(mode, FastVmMode::Shadow)); + let vm = ShadowVm::new(l1_batch_env, system_env, storage_view); Self::ShadowedVmFast(vm) } }, diff --git a/core/node/state_keeper/src/batch_executor/main_executor.rs b/core/node/state_keeper/src/batch_executor/main_executor.rs index 57d18a9a7b80..11db52ba6421 100644 --- a/core/node/state_keeper/src/batch_executor/main_executor.rs +++ b/core/node/state_keeper/src/batch_executor/main_executor.rs @@ -4,12 +4,12 @@ use anyhow::Context as _; use once_cell::sync::OnceCell; use tokio::sync::mpsc; use zksync_multivm::{ - dump::VmDumpHandler, interface::{ storage::{ReadStorage, StorageView}, Call, CompressedBytecodeInfo, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionResultAndLogs, VmInterface, VmInterfaceHistoryEnabled, }, + shadow, tracers::CallTracer, vm_latest::HistoryEnabled, MultiVMTracer, VmInstance, @@ -36,7 +36,7 @@ pub struct MainBatchExecutor { /// regardless of its configuration, this flag should be set to `true`. optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - dump_handler: Option, + divergence_handler: Option, } impl MainBatchExecutor { @@ -45,7 +45,7 @@ impl MainBatchExecutor { save_call_traces, optional_bytecode_compression, fast_vm_mode: FastVmMode::Old, - dump_handler: None, + divergence_handler: None, } } @@ -58,9 +58,9 @@ impl MainBatchExecutor { self.fast_vm_mode = fast_vm_mode; } - pub fn set_dump_handler(&mut self, handler: VmDumpHandler) { - tracing::info!("Set VM dumps handler"); - self.dump_handler = Some(handler); + pub fn set_divergence_handler(&mut self, handler: shadow::DivergenceHandler) { + tracing::info!("Set VM divergence handler"); + self.divergence_handler = Some(handler); } } @@ -78,7 +78,7 @@ impl BatchExecutor for MainBatchExecutor { save_call_traces: self.save_call_traces, optional_bytecode_compression: self.optional_bytecode_compression, fast_vm_mode: self.fast_vm_mode, - dump_handler: self.dump_handler.clone(), + divergence_handler: self.divergence_handler.clone(), commands: commands_receiver, }; @@ -106,7 +106,7 @@ struct CommandReceiver { save_call_traces: bool, optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - dump_handler: Option, + divergence_handler: Option, commands: mpsc::Receiver, } @@ -128,8 +128,8 @@ impl CommandReceiver { ); if let VmInstance::ShadowedVmFast(vm) = &mut vm { - if let Some(handler) = self.dump_handler.take() { - vm.set_dump_handler(handler); + if let Some(handler) = self.divergence_handler.take() { + vm.set_divergence_handler(handler); } } diff --git a/core/node/vm_runner/src/impls/playground.rs b/core/node/vm_runner/src/impls/playground.rs index 18aab0632165..606060709b2b 100644 --- a/core/node/vm_runner/src/impls/playground.rs +++ b/core/node/vm_runner/src/impls/playground.rs @@ -1,9 +1,9 @@ use std::{ + hash::{DefaultHasher, Hash, Hasher}, io, num::NonZeroU32, path::{Path, PathBuf}, sync::Arc, - time::SystemTime, }; use anyhow::Context as _; @@ -15,7 +15,7 @@ use tokio::{ }; use zksync_dal::{Connection, ConnectionPool, Core, CoreDal}; use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthCheck}; -use zksync_multivm::dump::{VmDump, VmDumpHandler}; +use zksync_multivm::{dump::VmDump, shadow}; use zksync_object_store::{Bucket, ObjectStore}; use zksync_state::RocksdbStorage; use zksync_state_keeper::{MainBatchExecutor, StateKeeperOutputHandler, UpdatesManager}; @@ -91,15 +91,17 @@ impl VmPlayground { if let Some(store) = dumps_object_store { tracing::info!("Using object store for VM dumps: {store:?}"); - let handler = VmDumpHandler::new(move |dump| { - if let Err(err) = handle.block_on(Self::dump_vm_state(&*store, &dump)) { + let handler = shadow::DivergenceHandler::new(move |err, dump| { + let err_message = err.to_string(); + if let Err(err) = handle.block_on(Self::dump_vm_state(&*store, &err_message, &dump)) + { let l1_batch_number = dump.l1_batch_number(); tracing::error!( "Saving VM dump for L1 batch #{l1_batch_number} failed: {err:#}" ); } }); - batch_executor.set_dump_handler(handler); + batch_executor.set_divergence_handler(handler); } let io = VmPlaygroundIo { @@ -138,13 +140,17 @@ impl VmPlayground { )) } - async fn dump_vm_state(object_store: &dyn ObjectStore, dump: &VmDump) -> anyhow::Result<()> { - let timestamp = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .expect("bogus clock"); - let timestamp = timestamp.as_millis(); - let batch_number = dump.l1_batch_number(); - let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{timestamp}.json"); + async fn dump_vm_state( + object_store: &dyn ObjectStore, + err_message: &str, + dump: &VmDump, + ) -> anyhow::Result<()> { + // Deduplicate VM dumps by the error hash so that we don't create a lot of dumps for the same error. + let mut hasher = DefaultHasher::new(); + err_message.hash(&mut hasher); + let err_hash = hasher.finish(); + let batch_number = dump.l1_batch_number().0; + let dump_filename = format!("shadow_vm_dump_batch{batch_number:08}_{err_hash:x}.json"); tracing::info!("Dumping diverged VM state to `{dump_filename}`"); let dump = serde_json::to_string(&dump).context("failed serializing VM dump")?; From 1b2cf548b8aa869be52d7498b89b8f42d2d65274 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 28 Aug 2024 11:49:43 +0300 Subject: [PATCH 19/24] Remove `ShadowLenient` VM mode --- core/lib/basic_types/src/vm.rs | 5 +---- core/lib/multivm/src/vm_instance.rs | 2 +- core/lib/protobuf_config/src/experimental.rs | 2 -- core/lib/protobuf_config/src/proto/config/experimental.proto | 1 - 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/core/lib/basic_types/src/vm.rs b/core/lib/basic_types/src/vm.rs index fdc88e506e15..c753bbfc8183 100644 --- a/core/lib/basic_types/src/vm.rs +++ b/core/lib/basic_types/src/vm.rs @@ -32,12 +32,9 @@ pub enum FastVmMode { /// Run only the old VM. #[default] Old, - /// Run only the new Vm. + /// Run only the new VM. New, /// Run both the new and old VM and compare their outputs for each transaction execution. /// The VM will panic on divergence. Shadow, - /// Run both the new and old VM and compare their outputs for each transaction execution. - /// The new VM will get dropped on divergence. - ShadowLenient, // FIXME: remove } diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 10e385983bb0..b97ab642a486 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -265,7 +265,7 @@ impl VmInstance { storage, )) } - FastVmMode::Shadow | FastVmMode::ShadowLenient => { + FastVmMode::Shadow => { let vm = ShadowVm::new(l1_batch_env, system_env, storage_view); Self::ShadowedVmFast(vm) } diff --git a/core/lib/protobuf_config/src/experimental.rs b/core/lib/protobuf_config/src/experimental.rs index 27f49ffabad0..7b71dec80344 100644 --- a/core/lib/protobuf_config/src/experimental.rs +++ b/core/lib/protobuf_config/src/experimental.rs @@ -57,7 +57,6 @@ impl proto::FastVmMode { FastVmMode::Old => Self::Old, FastVmMode::New => Self::New, FastVmMode::Shadow => Self::Shadow, - FastVmMode::ShadowLenient => Self::ShadowLenient, } } @@ -66,7 +65,6 @@ impl proto::FastVmMode { Self::Old => FastVmMode::Old, Self::New => FastVmMode::New, Self::Shadow => FastVmMode::Shadow, - Self::ShadowLenient => FastVmMode::ShadowLenient, } } } diff --git a/core/lib/protobuf_config/src/proto/config/experimental.proto b/core/lib/protobuf_config/src/proto/config/experimental.proto index bcac34c119b0..55fb81b56325 100644 --- a/core/lib/protobuf_config/src/proto/config/experimental.proto +++ b/core/lib/protobuf_config/src/proto/config/experimental.proto @@ -23,7 +23,6 @@ enum FastVmMode { OLD = 0; NEW = 1; SHADOW = 2; - SHADOW_LENIENT = 3; } // Experimental VM configuration From b3c84cbbea24ff25a2de3ce0c7d6e33f44a92450 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 2 Sep 2024 14:06:10 +0300 Subject: [PATCH 20/24] Use `StorageSnapshot` in VM dumps --- core/lib/multivm/src/dump.rs | 121 ++++++------------ core/lib/multivm/src/versions/shadow/tests.rs | 23 ++-- core/lib/vm_interface/src/storage/snapshot.rs | 32 ++++- 3 files changed, 81 insertions(+), 95 deletions(-) diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 1bca5a94ba20..93a1965ce277 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,14 +1,12 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use zksync_types::{ - block::L2BlockExecutionData, web3, L1BatchNumber, L2BlockNumber, Transaction, H256, U256, -}; +use zksync_types::{block::L2BlockExecutionData, L1BatchNumber, L2BlockNumber, Transaction, U256}; use zksync_utils::u256_to_h256; use crate::{ interface::{ - storage::{InMemoryStorage, ReadStorage, StoragePtr, StorageView, WriteStorage}, + storage::{ReadStorage, StoragePtr, StorageSnapshot, StorageView, WriteStorage}, BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled, VmMemoryMetrics, @@ -34,81 +32,44 @@ impl VmTrackingContracts for vm_fast::Vm { } } -// FIXME: use storage snapshots (https://github.com/matter-labs/zksync-era/pull/2724) -/// Part of the VM dump representing the storage oracle for a particular VM run. -#[derive(Debug, Clone, Default, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq))] -pub(crate) struct VmStorageDump { - /// Only existing keys (ones with an assigned enum index) are recorded. - pub(crate) read_storage_keys: HashMap, - /// Repeatedly written keys together with their enum indices. Only includes keys that were not read - /// (i.e., absent from `read_storage_keys`). Since `InMemoryStorage` returns `is_write_initial == true` - /// for all unknown keys, we don't need to record initial writes. - /// - /// Al least when the Fast VM is involved, this map always looks to be empty because the VM reads values - /// for all written keys (i.e., they all will be added to `read_storage_keys`). - pub(crate) repeated_writes: HashMap, - pub(crate) factory_deps: HashMap, -} +fn create_storage_snapshot( + storage: &StoragePtr>, + used_contract_hashes: Vec, +) -> StorageSnapshot { + let mut storage = storage.borrow_mut(); + let storage_cache = storage.cache(); + let mut storage_slots: HashMap<_, _> = storage_cache + .read_storage_keys() + .into_iter() + .map(|(key, value)| { + let enum_index = storage.get_enumeration_index(&key); + let value_and_index = enum_index.map(|idx| (value, idx)); + (key.hashed_key(), value_and_index) + }) + .collect(); -impl VmStorageDump { - /// Storage must be the one used by the VM. - pub(crate) fn new( - storage: &StoragePtr>, - used_contract_hashes: Vec, - ) -> Self { - let mut storage = storage.borrow_mut(); - let storage_cache = storage.cache(); - let read_storage_keys: HashMap<_, _> = storage_cache - .read_storage_keys() - .into_iter() - .filter_map(|(key, value)| { - let enum_index = storage.get_enumeration_index(&key)?; - Some((key.hashed_key(), (value, enum_index))) - }) - .collect(); - let repeated_writes = storage_cache - .initial_writes() - .into_iter() - .filter_map(|(key, is_initial)| { - let hashed_key = key.hashed_key(); - if !is_initial && !read_storage_keys.contains_key(&hashed_key) { - let enum_index = storage.get_enumeration_index(&key)?; - Some((hashed_key, enum_index)) - } else { - None - } - }) - .collect(); - - let factory_deps = used_contract_hashes - .into_iter() - .filter_map(|hash| { - let hash = u256_to_h256(hash); - Some((hash, web3::Bytes(storage.load_factory_dep(hash)?))) - }) - .collect(); - Self { - read_storage_keys, - repeated_writes, - factory_deps, + // Normally, all writes are internally read in order to calculate gas costs, so the code below + // is defensive programming. + for (key, _) in storage_cache.initial_writes() { + let hashed_key = key.hashed_key(); + if storage_slots.contains_key(&hashed_key) { + continue; } - } - pub(crate) fn into_storage(self) -> InMemoryStorage { - let mut storage = InMemoryStorage::default(); - for (key, (value, enum_index)) in self.read_storage_keys { - storage.set_value_hashed_enum(key, enum_index, value); - } - for (key, enum_index) in self.repeated_writes { - // The value shouldn't be read by the VM, so it doesn't matter. - storage.set_value_hashed_enum(key, enum_index, H256::zero()); - } - for (hash, bytecode) in self.factory_deps { - storage.store_factory_dep(hash, bytecode.0); - } - storage + let enum_index = storage.get_enumeration_index(&key); + let value_and_index = enum_index.map(|idx| (storage.read_value(&key), idx)); + storage_slots.insert(hashed_key, value_and_index); } + + let factory_deps = used_contract_hashes + .into_iter() + .filter_map(|hash| { + let hash = u256_to_h256(hash); + Some((hash, storage.load_factory_dep(hash)?)) + }) + .collect(); + + StorageSnapshot::new(storage_slots, factory_deps) } /// VM dump allowing to re-run the VM on the same inputs. Opaque, but can be (de)serialized. @@ -118,8 +79,7 @@ pub struct VmDump { pub(crate) l1_batch_env: L1BatchEnv, pub(crate) system_env: SystemEnv, pub(crate) l2_blocks: Vec, - #[serde(flatten)] - pub(crate) storage: VmStorageDump, + pub(crate) storage: StorageSnapshot, } impl VmDump { @@ -128,9 +88,8 @@ impl VmDump { } /// Plays back this dump on the specified VM. - pub fn play_back>>(self) -> Vm { - let storage = self.storage.into_storage(); - let storage = StorageView::new(storage).to_rc_ptr(); + pub fn play_back>>(self) -> Vm { + let storage = StorageView::new(self.storage).to_rc_ptr(); let mut vm = Vm::new(self.l1_batch_env, self.system_env, storage); Self::play_back_blocks(self.l2_blocks, &mut vm); vm @@ -195,7 +154,7 @@ impl DumpingVm { l1_batch_env: self.l1_batch_env.clone(), system_env: self.system_env.clone(), l2_blocks: self.l2_blocks.clone(), - storage: VmStorageDump::new(&self.storage, self.inner.used_contract_hashes()), + storage: create_storage_snapshot(&self.storage, self.inner.used_contract_hashes()), } } } diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs index 0200835cd2d9..caae39e1b7fa 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -91,22 +91,20 @@ impl Harness { ); } - fn assert_dump(dump: &VmDump) { + fn assert_dump(dump: &mut VmDump) { assert_eq!(dump.l1_batch_number(), L1BatchNumber(1)); let tx_counts_per_block: Vec<_> = dump.l2_blocks.iter().map(|block| block.txs.len()).collect(); assert_eq!(tx_counts_per_block, [1, 2, 2, 0]); - assert!(!dump.storage.read_storage_keys.is_empty()); - assert!(!dump.storage.factory_deps.is_empty()); let storage_contract_key = StorageKey::new( AccountTreeId::new(Self::STORAGE_CONTRACT_ADDRESS), H256::zero(), - ) - .hashed_key(); - let (value, enum_index) = dump.storage.read_storage_keys[&storage_contract_key]; + ); + let value = dump.storage.read_value(&storage_contract_key); assert_eq!(value, H256::from_low_u64_be(42)); - assert_eq!(enum_index, 999); + let enum_index = dump.storage.get_enumeration_index(&storage_contract_key); + assert_eq!(enum_index, Some(999)); } fn new_block(&mut self, vm: &mut impl VmInterface, tx_hashes: &[H256]) { @@ -245,13 +243,13 @@ fn sanity_check_shadow_vm() { #[test] fn shadow_vm_basics() { let (vm, harness) = sanity_check_vm::>(); - let dump = vm.main.dump_state(); - Harness::assert_dump(&dump); + let mut dump = vm.main.dump_state(); + Harness::assert_dump(&mut dump); // Test standard playback functionality. let replayed_dump = dump .clone() - .play_back::>() + .play_back::>>() .main .dump_state(); pretty_assertions::assert_eq!(replayed_dump, dump); @@ -260,9 +258,8 @@ fn shadow_vm_basics() { let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); harness.setup_storage(&mut storage); let storage = StorageView::new(storage).to_rc_ptr(); - let dump_storage = dump.storage.clone().into_storage(); - let dump_storage = StorageView::new(dump_storage).to_rc_ptr(); - let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm>::with_custom_shadow( + let dump_storage = StorageView::new(dump.storage.clone()).to_rc_ptr(); + let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm<_>>::with_custom_shadow( dump.l1_batch_env.clone(), dump.system_env.clone(), storage, diff --git a/core/lib/vm_interface/src/storage/snapshot.rs b/core/lib/vm_interface/src/storage/snapshot.rs index a0175ff478a3..78b57a31f13e 100644 --- a/core/lib/vm_interface/src/storage/snapshot.rs +++ b/core/lib/vm_interface/src/storage/snapshot.rs @@ -12,7 +12,7 @@ use super::ReadStorage; /// In contrast, `StorageSnapshot` cannot be modified once created and is intended to represent a complete or almost complete snapshot /// for a particular VM execution. It can serve as a preloaded cache for a certain [`ReadStorage`] implementation /// that significantly reduces the number of storage accesses. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct StorageSnapshot { // `Option` encompasses entire map value for more efficient serialization storage: HashMap>, @@ -60,6 +60,36 @@ impl StorageSnapshot { } } +/// When used as a storage, a snapshot is assumed to be *complete*; [`ReadStorage`] methods will panic when called +/// with storage slots not present in the snapshot. +impl ReadStorage for StorageSnapshot { + fn read_value(&mut self, key: &StorageKey) -> StorageValue { + let entry = self + .storage + .get(&key.hashed_key()) + .unwrap_or_else(|| panic!("attempted to read from unknown storage slot: {key:?}")); + entry.unwrap_or_default().0 + } + + fn is_write_initial(&mut self, key: &StorageKey) -> bool { + let entry = self.storage.get(&key.hashed_key()).unwrap_or_else(|| { + panic!("attempted to check initialness for unknown storage slot: {key:?}") + }); + entry.is_none() + } + + fn load_factory_dep(&mut self, hash: H256) -> Option> { + self.factory_deps.get(&hash).map(|bytes| bytes.0.clone()) + } + + fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { + let entry = self.storage.get(&key.hashed_key()).unwrap_or_else(|| { + panic!("attempted to get enum index for unknown storage slot: {key:?}") + }); + entry.map(|(_, idx)| idx) + } +} + /// [`StorageSnapshot`] wrapper implementing [`ReadStorage`] trait. Created using [`with_fallback()`](StorageSnapshot::with_fallback()). /// /// # Why fallback? From ec76fd4efdafd21be7b98bded4413ffdb3ae1316 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 3 Sep 2024 21:55:44 +0300 Subject: [PATCH 21/24] Move `DumpingVm` and `ShadowVm` to interface crate --- Cargo.lock | 2 +- core/lib/multivm/Cargo.toml | 3 +- core/lib/multivm/src/dump.rs | 271 ------------------ core/lib/multivm/src/lib.rs | 4 +- core/lib/multivm/src/versions/mod.rs | 3 +- .../src/versions/{shadow => }/tests.rs | 44 +-- core/lib/multivm/src/versions/vm_fast/vm.rs | 23 +- core/lib/multivm/src/versions/vm_latest/vm.rs | 14 +- core/lib/multivm/src/vm_instance.rs | 10 +- core/lib/vm_executor/src/batch/factory.rs | 8 +- core/lib/vm_executor/src/batch/mod.rs | 3 - core/lib/vm_interface/Cargo.toml | 1 + core/lib/vm_interface/src/lib.rs | 3 +- core/lib/vm_interface/src/utils/dump.rs | 249 ++++++++++++++++ core/lib/vm_interface/src/utils/mod.rs | 9 + .../src/utils/shadow.rs} | 30 +- core/lib/vm_interface/src/vm.rs | 8 +- core/node/vm_runner/src/impls/playground.rs | 9 +- prover/Cargo.lock | 3 +- 19 files changed, 358 insertions(+), 339 deletions(-) rename core/lib/multivm/src/versions/{shadow => }/tests.rs (89%) create mode 100644 core/lib/vm_interface/src/utils/dump.rs create mode 100644 core/lib/vm_interface/src/utils/mod.rs rename core/lib/{multivm/src/versions/shadow/mod.rs => vm_interface/src/utils/shadow.rs} (94%) diff --git a/Cargo.lock b/Cargo.lock index d5ef5f370587..1708b46f0cbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8967,7 +8967,6 @@ dependencies = [ "itertools 0.10.5", "once_cell", "pretty_assertions", - "serde", "thiserror", "tokio", "tracing", @@ -9805,6 +9804,7 @@ dependencies = [ "assert_matches", "async-trait", "hex", + "pretty_assertions", "serde", "serde_json", "thiserror", diff --git a/core/lib/multivm/Cargo.toml b/core/lib/multivm/Cargo.toml index c03afb9fa6db..f379511bd561 100644 --- a/core/lib/multivm/Cargo.toml +++ b/core/lib/multivm/Cargo.toml @@ -34,14 +34,13 @@ anyhow.workspace = true hex.workspace = true itertools.workspace = true once_cell.workspace = true -pretty_assertions.workspace = true thiserror.workspace = true tracing.workspace = true vise.workspace = true -serde.workspace = true [dev-dependencies] assert_matches.workspace = true +pretty_assertions.workspace = true tokio = { workspace = true, features = ["time"] } zksync_test_account.workspace = true ethabi.workspace = true diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs index 93a1965ce277..8b137891791f 100644 --- a/core/lib/multivm/src/dump.rs +++ b/core/lib/multivm/src/dump.rs @@ -1,272 +1 @@ -use std::collections::HashMap; -use serde::{Deserialize, Serialize}; -use zksync_types::{block::L2BlockExecutionData, L1BatchNumber, L2BlockNumber, Transaction, U256}; -use zksync_utils::u256_to_h256; - -use crate::{ - interface::{ - storage::{ReadStorage, StoragePtr, StorageSnapshot, StorageView, WriteStorage}, - BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, - VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceExt, - VmInterfaceHistoryEnabled, VmMemoryMetrics, - }, - vm_fast, vm_latest, HistoryMode, -}; - -/// VM that tracks decommitment of bytecodes during execution. This is required to create a [`VmDump`]. -pub trait VmTrackingContracts: VmInterface { - /// Returns hashes of all decommitted bytecodes. - fn used_contract_hashes(&self) -> Vec; -} - -impl VmTrackingContracts for vm_latest::Vm { - fn used_contract_hashes(&self) -> Vec { - self.get_used_contracts() - } -} - -impl VmTrackingContracts for vm_fast::Vm { - fn used_contract_hashes(&self) -> Vec { - self.decommitted_hashes().collect() - } -} - -fn create_storage_snapshot( - storage: &StoragePtr>, - used_contract_hashes: Vec, -) -> StorageSnapshot { - let mut storage = storage.borrow_mut(); - let storage_cache = storage.cache(); - let mut storage_slots: HashMap<_, _> = storage_cache - .read_storage_keys() - .into_iter() - .map(|(key, value)| { - let enum_index = storage.get_enumeration_index(&key); - let value_and_index = enum_index.map(|idx| (value, idx)); - (key.hashed_key(), value_and_index) - }) - .collect(); - - // Normally, all writes are internally read in order to calculate gas costs, so the code below - // is defensive programming. - for (key, _) in storage_cache.initial_writes() { - let hashed_key = key.hashed_key(); - if storage_slots.contains_key(&hashed_key) { - continue; - } - - let enum_index = storage.get_enumeration_index(&key); - let value_and_index = enum_index.map(|idx| (storage.read_value(&key), idx)); - storage_slots.insert(hashed_key, value_and_index); - } - - let factory_deps = used_contract_hashes - .into_iter() - .filter_map(|hash| { - let hash = u256_to_h256(hash); - Some((hash, storage.load_factory_dep(hash)?)) - }) - .collect(); - - StorageSnapshot::new(storage_slots, factory_deps) -} - -/// VM dump allowing to re-run the VM on the same inputs. Opaque, but can be (de)serialized. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq))] -pub struct VmDump { - pub(crate) l1_batch_env: L1BatchEnv, - pub(crate) system_env: SystemEnv, - pub(crate) l2_blocks: Vec, - pub(crate) storage: StorageSnapshot, -} - -impl VmDump { - pub fn l1_batch_number(&self) -> L1BatchNumber { - self.l1_batch_env.number - } - - /// Plays back this dump on the specified VM. - pub fn play_back>>(self) -> Vm { - let storage = StorageView::new(self.storage).to_rc_ptr(); - let mut vm = Vm::new(self.l1_batch_env, self.system_env, storage); - Self::play_back_blocks(self.l2_blocks, &mut vm); - vm - } - - pub(crate) fn play_back_blocks( - l2_blocks: Vec, - vm: &mut impl VmInterface, - ) { - for (i, l2_block) in l2_blocks.into_iter().enumerate() { - if i > 0 { - // First block is already set. - vm.start_new_l2_block(L2BlockEnv { - number: l2_block.number.0, - timestamp: l2_block.timestamp, - prev_block_hash: l2_block.prev_block_hash, - max_virtual_blocks_to_create: l2_block.virtual_blocks, - }); - } - - for tx in l2_block.txs { - let tx_hash = tx.hash(); - let (compression_result, _) = - vm.execute_transaction_with_bytecode_compression(tx, true); - if let Err(err) = compression_result { - panic!("Failed compressing bytecodes for transaction {tx_hash:?}: {err}"); - } - } - } - vm.finish_batch(); - } -} - -#[derive(Debug, Clone, Copy)] -struct L2BlocksSnapshot { - block_count: usize, - tx_count_in_last_block: usize, -} - -/// VM wrapper that can create [`VmDump`]s during execution. -#[derive(Debug)] -pub(crate) struct DumpingVm { - storage: StoragePtr>, - inner: Vm, - l1_batch_env: L1BatchEnv, - system_env: SystemEnv, - l2_blocks: Vec, - l2_blocks_snapshot: Option, -} - -impl DumpingVm { - fn last_block_mut(&mut self) -> &mut L2BlockExecutionData { - self.l2_blocks.last_mut().unwrap() - } - - fn record_transaction(&mut self, tx: Transaction) { - self.last_block_mut().txs.push(tx); - } - - pub fn dump_state(&self) -> VmDump { - VmDump { - l1_batch_env: self.l1_batch_env.clone(), - system_env: self.system_env.clone(), - l2_blocks: self.l2_blocks.clone(), - storage: create_storage_snapshot(&self.storage, self.inner.used_contract_hashes()), - } - } -} - -impl VmInterface for DumpingVm { - type TracerDispatcher = Vm::TracerDispatcher; - - fn push_transaction(&mut self, tx: Transaction) { - self.record_transaction(tx.clone()); - self.inner.push_transaction(tx); - } - - fn inspect( - &mut self, - dispatcher: Self::TracerDispatcher, - execution_mode: VmExecutionMode, - ) -> VmExecutionResultAndLogs { - self.inner.inspect(dispatcher, execution_mode) - } - - fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { - self.l2_blocks.push(L2BlockExecutionData { - number: L2BlockNumber(l2_block_env.number), - timestamp: l2_block_env.timestamp, - prev_block_hash: l2_block_env.prev_block_hash, - virtual_blocks: l2_block_env.max_virtual_blocks_to_create, - txs: vec![], - }); - self.inner.start_new_l2_block(l2_block_env); - } - - fn inspect_transaction_with_bytecode_compression( - &mut self, - tracer: Self::TracerDispatcher, - tx: Transaction, - with_compression: bool, - ) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) { - self.record_transaction(tx.clone()); - self.inner - .inspect_transaction_with_bytecode_compression(tracer, tx, with_compression) - } - - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.inner.record_vm_memory_metrics() - } - - fn finish_batch(&mut self) -> FinishedL1Batch { - self.inner.finish_batch() - } -} - -impl VmInterfaceHistoryEnabled for DumpingVm -where - S: ReadStorage, - Vm: VmInterfaceHistoryEnabled + VmTrackingContracts, -{ - fn make_snapshot(&mut self) { - self.l2_blocks_snapshot = Some(L2BlocksSnapshot { - block_count: self.l2_blocks.len(), - tx_count_in_last_block: self.last_block_mut().txs.len(), - }); - self.inner.make_snapshot(); - } - - fn rollback_to_the_latest_snapshot(&mut self) { - self.inner.rollback_to_the_latest_snapshot(); - let snapshot = self - .l2_blocks_snapshot - .take() - .expect("rollback w/o snapshot"); - self.l2_blocks.truncate(snapshot.block_count); - assert_eq!( - self.l2_blocks.len(), - snapshot.block_count, - "L2 blocks were removed after creating a snapshot" - ); - self.last_block_mut() - .txs - .truncate(snapshot.tx_count_in_last_block); - } - - fn pop_snapshot_no_rollback(&mut self) { - self.inner.pop_snapshot_no_rollback(); - self.l2_blocks_snapshot = None; - } -} - -impl VmFactory> for DumpingVm -where - S: ReadStorage, - Vm: VmFactory> + VmTrackingContracts, -{ - fn new( - l1_batch_env: L1BatchEnv, - system_env: SystemEnv, - storage: StoragePtr>, - ) -> Self { - let inner = Vm::new(l1_batch_env.clone(), system_env.clone(), storage.clone()); - let first_block = L2BlockExecutionData { - number: L2BlockNumber(l1_batch_env.first_l2_block.number), - timestamp: l1_batch_env.first_l2_block.timestamp, - prev_block_hash: l1_batch_env.first_l2_block.prev_block_hash, - virtual_blocks: l1_batch_env.first_l2_block.max_virtual_blocks_to_create, - txs: vec![], - }; - Self { - l1_batch_env, - system_env, - l2_blocks: vec![first_block], - l2_blocks_snapshot: None, - storage, - inner, - } - } -} diff --git a/core/lib/multivm/src/lib.rs b/core/lib/multivm/src/lib.rs index 68158038810d..641102bd0743 100644 --- a/core/lib/multivm/src/lib.rs +++ b/core/lib/multivm/src/lib.rs @@ -13,8 +13,8 @@ pub use crate::{ tracers::{MultiVMTracer, MultiVmTracerPointer}, }, versions::{ - shadow, vm_1_3_2, vm_1_4_1, vm_1_4_2, vm_boojum_integration, vm_fast, vm_latest, vm_m5, - vm_m6, vm_refunds_enhancement, vm_virtual_blocks, + vm_1_3_2, vm_1_4_1, vm_1_4_2, vm_boojum_integration, vm_fast, vm_latest, vm_m5, vm_m6, + vm_refunds_enhancement, vm_virtual_blocks, }, vm_instance::VmInstance, }; diff --git a/core/lib/multivm/src/versions/mod.rs b/core/lib/multivm/src/versions/mod.rs index 7d4703ea77ae..bcb246cece46 100644 --- a/core/lib/multivm/src/versions/mod.rs +++ b/core/lib/multivm/src/versions/mod.rs @@ -1,7 +1,8 @@ -pub mod shadow; mod shared; #[cfg(test)] mod testonly; +#[cfg(test)] +mod tests; pub mod vm_1_3_2; pub mod vm_1_4_1; pub mod vm_1_4_2; diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/tests.rs similarity index 89% rename from core/lib/multivm/src/versions/shadow/tests.rs rename to core/lib/multivm/src/versions/tests.rs index caae39e1b7fa..78584f32e528 100644 --- a/core/lib/multivm/src/versions/shadow/tests.rs +++ b/core/lib/multivm/src/versions/tests.rs @@ -1,4 +1,5 @@ -//! Shadow VM tests. +//! Shadow VM tests. Since there are no real VM implementations in the `vm_interface` crate where `ShadowVm` is defined, +//! these tests are placed here. use assert_matches::assert_matches; use ethabi::Contract; @@ -9,21 +10,26 @@ use zksync_contracts::{ use zksync_test_account::{Account, TxType}; use zksync_types::{ block::L2BlockHasher, fee::Fee, AccountTreeId, Address, Execute, L1BatchNumber, L2BlockNumber, - ProtocolVersionId, H256, U256, + ProtocolVersionId, StorageKey, H256, U256, }; use zksync_utils::bytecode::hash_bytecode; -use super::*; use crate::{ - interface::{storage::InMemoryStorage, ExecutionResult, VmInterfaceExt}, + interface::{ + storage::{InMemoryStorage, ReadStorage, StorageView}, + utils::{ShadowVm, VmDump}, + ExecutionResult, L1BatchEnv, L2BlockEnv, VmFactory, VmInterface, VmInterfaceExt, + }, utils::get_max_gas_per_pubdata_byte, versions::testonly::{ default_l1_batch, default_system_env, make_account_rich, ContractToDeploy, }, + vm_fast, vm_latest::{self, HistoryEnabled}, }; type ReferenceVm = vm_latest::Vm, HistoryEnabled>; +type ShadowedVmFast = crate::vm_instance::ShadowedVmFast; fn hash_block(block_env: L2BlockEnv, tx_hashes: &[H256]) -> H256 { let mut hasher = L2BlockHasher::new( @@ -242,31 +248,29 @@ fn sanity_check_shadow_vm() { #[test] fn shadow_vm_basics() { - let (vm, harness) = sanity_check_vm::>(); - let mut dump = vm.main.dump_state(); + let (vm, harness) = sanity_check_vm::(); + let mut dump = vm.dump_state(); Harness::assert_dump(&mut dump); // Test standard playback functionality. - let replayed_dump = dump - .clone() - .play_back::>>() - .main - .dump_state(); + let replayed_dump = dump.clone().play_back::>().dump_state(); pretty_assertions::assert_eq!(replayed_dump, dump); // Check that the VM executes identically when reading from the original storage and one restored from the dump. let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); harness.setup_storage(&mut storage); let storage = StorageView::new(storage).to_rc_ptr(); - let dump_storage = StorageView::new(dump.storage.clone()).to_rc_ptr(); - let mut vm = ShadowVm::<_, ReferenceVm, ReferenceVm<_>>::with_custom_shadow( - dump.l1_batch_env.clone(), - dump.system_env.clone(), - storage, - dump_storage, - ); - VmDump::play_back_blocks(dump.l2_blocks.clone(), &mut vm); - let new_dump = vm.main.dump_state(); + let vm = dump + .clone() + .play_back_custom(|l1_batch_env, system_env, dump_storage| { + ShadowVm::<_, ReferenceVm, ReferenceVm<_>>::with_custom_shadow( + l1_batch_env, + system_env, + storage, + dump_storage, + ) + }); + let new_dump = vm.dump_state(); pretty_assertions::assert_eq!(new_dump, dump); } diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index dcb8c9d9c19d..70e7e0063758 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -15,14 +15,10 @@ use zksync_types::{ BYTES_PER_ENUMERATION_INDEX, }, AccountTreeId, StorageKey, StorageLog, StorageLogKind, StorageLogWithPreviousValue, - BOOTLOADER_ADDRESS, H160, KNOWN_CODES_STORAGE_ADDRESS, L1_MESSENGER_ADDRESS, + BOOTLOADER_ADDRESS, H160, H256, KNOWN_CODES_STORAGE_ADDRESS, L1_MESSENGER_ADDRESS, L2_BASE_TOKEN_ADDRESS, U256, }; use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256}; -use zksync_vm_interface::{ - storage::{ImmutableStorageView, StoragePtr, StorageView}, - VmFactory, -}; use super::{ bootloader_state::{BootloaderState, BootloaderStateSnapshot}, @@ -34,11 +30,12 @@ use super::{ use crate::{ glue::GlueInto, interface::{ - storage::ReadStorage, BytecodeCompressionError, CompressedBytecodeInfo, - CurrentExecutionState, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv, - Refunds, SystemEnv, TxRevertReason, VmEvent, VmExecutionLogs, VmExecutionMode, - VmExecutionResultAndLogs, VmExecutionStatistics, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, VmRevertReason, + storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, + BytecodeCompressionError, CompressedBytecodeInfo, CurrentExecutionState, ExecutionResult, + FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv, Refunds, SystemEnv, TxRevertReason, VmEvent, + VmExecutionLogs, VmExecutionMode, VmExecutionResultAndLogs, VmExecutionStatistics, + VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, VmRevertReason, + VmTrackingContracts, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_fast::{ @@ -652,6 +649,12 @@ impl VmInterfaceHistoryEnabled for Vm { } } +impl VmTrackingContracts for Vm { + fn used_contract_hashes(&self) -> Vec { + self.decommitted_hashes().map(u256_to_h256).collect() + } +} + impl fmt::Debug for Vm { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Vm") diff --git a/core/lib/multivm/src/versions/vm_latest/vm.rs b/core/lib/multivm/src/versions/vm_latest/vm.rs index c0c13669c2ef..cd84be5be062 100644 --- a/core/lib/multivm/src/versions/vm_latest/vm.rs +++ b/core/lib/multivm/src/versions/vm_latest/vm.rs @@ -2,8 +2,9 @@ use circuit_sequencer_api_1_5_0::sort_storage_access::sort_storage_access_querie use zksync_types::{ l2_to_l1_log::{SystemL2ToL1Log, UserL2ToL1Log}, vm::VmVersion, - Transaction, + Transaction, H256, }; +use zksync_utils::u256_to_h256; use crate::{ glue::GlueInto, @@ -12,7 +13,7 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, + VmMemoryMetrics, VmTrackingContracts, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_latest::{ @@ -235,3 +236,12 @@ impl VmInterfaceHistoryEnabled for Vm { self.snapshots.pop(); } } + +impl VmTrackingContracts for Vm { + fn used_contract_hashes(&self) -> Vec { + self.get_used_contracts() + .into_iter() + .map(u256_to_h256) + .collect() + } +} diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 3ee81b99fa51..e3edb75dfab7 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -4,14 +4,20 @@ use crate::{ glue::history_mode::HistoryMode, interface::{ storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, + utils::ShadowVm, BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, }, tracers::TracerDispatcher, - versions::shadow::ShadowVm, }; +pub(crate) type ShadowedVmFast = ShadowVm< + S, + crate::vm_latest::Vm, H>, + crate::vm_fast::Vm>, +>; + #[derive(Debug)] pub enum VmInstance { VmM5(crate::vm_m5::Vm, H>), @@ -24,7 +30,7 @@ pub enum VmInstance { Vm1_4_2(crate::vm_1_4_2::Vm, H>), Vm1_5_0(crate::vm_latest::Vm, H>), VmFast(crate::vm_fast::Vm>), - ShadowedVmFast(ShadowVm, H>>), + ShadowedVmFast(ShadowedVmFast), } macro_rules! dispatch_vm { diff --git a/core/lib/vm_executor/src/batch/factory.rs b/core/lib/vm_executor/src/batch/factory.rs index 3a31e17f80c1..ed066165c258 100644 --- a/core/lib/vm_executor/src/batch/factory.rs +++ b/core/lib/vm_executor/src/batch/factory.rs @@ -7,10 +7,10 @@ use zksync_multivm::{ interface::{ executor::{BatchExecutor, BatchExecutorFactory}, storage::{ReadStorage, StorageView}, + utils::DivergenceHandler, BatchTransactionExecutionResult, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv, SystemEnv, VmInterface, VmInterfaceHistoryEnabled, }, - shadow, tracers::CallTracer, vm_latest::HistoryEnabled, MultiVMTracer, VmInstance, @@ -36,7 +36,7 @@ pub struct MainBatchExecutorFactory { /// regardless of its configuration, this flag should be set to `true`. optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - divergence_handler: Option, + divergence_handler: Option, } impl MainBatchExecutorFactory { @@ -58,7 +58,7 @@ impl MainBatchExecutorFactory { self.fast_vm_mode = fast_vm_mode; } - pub fn set_divergence_handler(&mut self, handler: shadow::DivergenceHandler) { + pub fn set_divergence_handler(&mut self, handler: DivergenceHandler) { tracing::info!("Set VM divergence handler"); self.divergence_handler = Some(handler); } @@ -100,7 +100,7 @@ struct CommandReceiver { save_call_traces: bool, optional_bytecode_compression: bool, fast_vm_mode: FastVmMode, - divergence_handler: Option, + divergence_handler: Option, commands: mpsc::Receiver, _storage: PhantomData, } diff --git a/core/lib/vm_executor/src/batch/mod.rs b/core/lib/vm_executor/src/batch/mod.rs index cef99cbce5fd..2407d2daba2c 100644 --- a/core/lib/vm_executor/src/batch/mod.rs +++ b/core/lib/vm_executor/src/batch/mod.rs @@ -2,9 +2,6 @@ //! //! This implementation is used by various ZKsync components, like the state keeper and components based on the VM runner. -// Public re-exports to allow crate users not to export the `multivm` crate directly. -pub use zksync_multivm::{dump, shadow}; - pub use self::{executor::MainBatchExecutor, factory::MainBatchExecutorFactory}; mod executor; diff --git a/core/lib/vm_interface/Cargo.toml b/core/lib/vm_interface/Cargo.toml index 694576dca3b0..8bff19ddc475 100644 --- a/core/lib/vm_interface/Cargo.toml +++ b/core/lib/vm_interface/Cargo.toml @@ -18,6 +18,7 @@ zksync_types.workspace = true anyhow.workspace = true async-trait.workspace = true hex.workspace = true +pretty_assertions.workspace = true serde.workspace = true thiserror.workspace = true tracing.workspace = true diff --git a/core/lib/vm_interface/src/lib.rs b/core/lib/vm_interface/src/lib.rs index 315eb2bb36a7..8fc1166282e0 100644 --- a/core/lib/vm_interface/src/lib.rs +++ b/core/lib/vm_interface/src/lib.rs @@ -36,10 +36,11 @@ pub use crate::{ }, tracer, }, - vm::{VmFactory, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled}, + vm::{VmFactory, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled, VmTrackingContracts}, }; pub mod executor; pub mod storage; mod types; +pub mod utils; mod vm; diff --git a/core/lib/vm_interface/src/utils/dump.rs b/core/lib/vm_interface/src/utils/dump.rs new file mode 100644 index 000000000000..f7dce38ee899 --- /dev/null +++ b/core/lib/vm_interface/src/utils/dump.rs @@ -0,0 +1,249 @@ +use std::collections::HashMap; + +use serde::{Deserialize, Serialize}; +use zksync_types::{block::L2BlockExecutionData, L1BatchNumber, L2BlockNumber, Transaction, H256}; + +use crate::{ + storage::{ReadStorage, StoragePtr, StorageSnapshot, StorageView}, + BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, + VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled, + VmMemoryMetrics, VmTrackingContracts, +}; + +fn create_storage_snapshot( + storage: &StoragePtr>, + used_contract_hashes: Vec, +) -> StorageSnapshot { + let mut storage = storage.borrow_mut(); + let storage_cache = storage.cache(); + let mut storage_slots: HashMap<_, _> = storage_cache + .read_storage_keys() + .into_iter() + .map(|(key, value)| { + let enum_index = storage.get_enumeration_index(&key); + let value_and_index = enum_index.map(|idx| (value, idx)); + (key.hashed_key(), value_and_index) + }) + .collect(); + + // Normally, all writes are internally read in order to calculate their gas costs, so the code below + // is defensive programming. + for (key, _) in storage_cache.initial_writes() { + let hashed_key = key.hashed_key(); + if storage_slots.contains_key(&hashed_key) { + continue; + } + + let enum_index = storage.get_enumeration_index(&key); + let value_and_index = enum_index.map(|idx| (storage.read_value(&key), idx)); + storage_slots.insert(hashed_key, value_and_index); + } + + let factory_deps = used_contract_hashes + .into_iter() + .filter_map(|hash| Some((hash, storage.load_factory_dep(hash)?))) + .collect(); + + StorageSnapshot::new(storage_slots, factory_deps) +} + +/// VM dump allowing to re-run the VM on the same inputs. Can be (de)serialized. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct VmDump { + pub l1_batch_env: L1BatchEnv, + pub system_env: SystemEnv, + pub l2_blocks: Vec, + pub storage: StorageSnapshot, +} + +impl VmDump { + pub fn l1_batch_number(&self) -> L1BatchNumber { + self.l1_batch_env.number + } + + /// Plays back this dump on the specified VM. + pub fn play_back>>(self) -> Vm { + self.play_back_custom(Vm::new) + } + + /// Plays back this dump on a VM created using the provided closure. + #[doc(hidden)] // too low-level + pub fn play_back_custom( + self, + create_vm: impl FnOnce(L1BatchEnv, SystemEnv, StoragePtr>) -> Vm, + ) -> Vm { + let storage = StorageView::new(self.storage).to_rc_ptr(); + let mut vm = create_vm(self.l1_batch_env, self.system_env, storage); + + for (i, l2_block) in self.l2_blocks.into_iter().enumerate() { + if i > 0 { + // First block is already set. + vm.start_new_l2_block(L2BlockEnv { + number: l2_block.number.0, + timestamp: l2_block.timestamp, + prev_block_hash: l2_block.prev_block_hash, + max_virtual_blocks_to_create: l2_block.virtual_blocks, + }); + } + + for tx in l2_block.txs { + let tx_hash = tx.hash(); + let (compression_result, _) = + vm.execute_transaction_with_bytecode_compression(tx, true); + if let Err(err) = compression_result { + panic!("Failed compressing bytecodes for transaction {tx_hash:?}: {err}"); + } + } + } + vm.finish_batch(); + vm + } +} + +#[derive(Debug, Clone, Copy)] +struct L2BlocksSnapshot { + block_count: usize, + tx_count_in_last_block: usize, +} + +/// VM wrapper that can create [`VmDump`]s during execution. +#[derive(Debug)] +pub(super) struct DumpingVm { + storage: StoragePtr>, + inner: Vm, + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + l2_blocks: Vec, + l2_blocks_snapshot: Option, +} + +impl DumpingVm { + fn last_block_mut(&mut self) -> &mut L2BlockExecutionData { + self.l2_blocks.last_mut().unwrap() + } + + fn record_transaction(&mut self, tx: Transaction) { + self.last_block_mut().txs.push(tx); + } + + pub fn dump_state(&self) -> VmDump { + VmDump { + l1_batch_env: self.l1_batch_env.clone(), + system_env: self.system_env.clone(), + l2_blocks: self.l2_blocks.clone(), + storage: create_storage_snapshot(&self.storage, self.inner.used_contract_hashes()), + } + } +} + +impl VmInterface for DumpingVm { + type TracerDispatcher = Vm::TracerDispatcher; + + fn push_transaction(&mut self, tx: Transaction) { + self.record_transaction(tx.clone()); + self.inner.push_transaction(tx); + } + + fn inspect( + &mut self, + dispatcher: Self::TracerDispatcher, + execution_mode: VmExecutionMode, + ) -> VmExecutionResultAndLogs { + self.inner.inspect(dispatcher, execution_mode) + } + + fn start_new_l2_block(&mut self, l2_block_env: L2BlockEnv) { + self.l2_blocks.push(L2BlockExecutionData { + number: L2BlockNumber(l2_block_env.number), + timestamp: l2_block_env.timestamp, + prev_block_hash: l2_block_env.prev_block_hash, + virtual_blocks: l2_block_env.max_virtual_blocks_to_create, + txs: vec![], + }); + self.inner.start_new_l2_block(l2_block_env); + } + + fn inspect_transaction_with_bytecode_compression( + &mut self, + tracer: Self::TracerDispatcher, + tx: Transaction, + with_compression: bool, + ) -> (BytecodeCompressionResult, VmExecutionResultAndLogs) { + self.record_transaction(tx.clone()); + self.inner + .inspect_transaction_with_bytecode_compression(tracer, tx, with_compression) + } + + fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + self.inner.record_vm_memory_metrics() + } + + fn finish_batch(&mut self) -> FinishedL1Batch { + self.inner.finish_batch() + } +} + +impl VmInterfaceHistoryEnabled for DumpingVm +where + S: ReadStorage, + Vm: VmInterfaceHistoryEnabled + VmTrackingContracts, +{ + fn make_snapshot(&mut self) { + self.l2_blocks_snapshot = Some(L2BlocksSnapshot { + block_count: self.l2_blocks.len(), + tx_count_in_last_block: self.last_block_mut().txs.len(), + }); + self.inner.make_snapshot(); + } + + fn rollback_to_the_latest_snapshot(&mut self) { + self.inner.rollback_to_the_latest_snapshot(); + let snapshot = self + .l2_blocks_snapshot + .take() + .expect("rollback w/o snapshot"); + self.l2_blocks.truncate(snapshot.block_count); + assert_eq!( + self.l2_blocks.len(), + snapshot.block_count, + "L2 blocks were removed after creating a snapshot" + ); + self.last_block_mut() + .txs + .truncate(snapshot.tx_count_in_last_block); + } + + fn pop_snapshot_no_rollback(&mut self) { + self.inner.pop_snapshot_no_rollback(); + self.l2_blocks_snapshot = None; + } +} + +impl VmFactory> for DumpingVm +where + S: ReadStorage, + Vm: VmFactory> + VmTrackingContracts, +{ + fn new( + l1_batch_env: L1BatchEnv, + system_env: SystemEnv, + storage: StoragePtr>, + ) -> Self { + let inner = Vm::new(l1_batch_env.clone(), system_env.clone(), storage.clone()); + let first_block = L2BlockExecutionData { + number: L2BlockNumber(l1_batch_env.first_l2_block.number), + timestamp: l1_batch_env.first_l2_block.timestamp, + prev_block_hash: l1_batch_env.first_l2_block.prev_block_hash, + virtual_blocks: l1_batch_env.first_l2_block.max_virtual_blocks_to_create, + txs: vec![], + }; + Self { + l1_batch_env, + system_env, + l2_blocks: vec![first_block], + l2_blocks_snapshot: None, + storage, + inner, + } + } +} diff --git a/core/lib/vm_interface/src/utils/mod.rs b/core/lib/vm_interface/src/utils/mod.rs new file mode 100644 index 000000000000..80a51c7b144f --- /dev/null +++ b/core/lib/vm_interface/src/utils/mod.rs @@ -0,0 +1,9 @@ +//! Miscellaneous VM utils. + +pub use self::{ + dump::VmDump, + shadow::{DivergenceErrors, DivergenceHandler, ShadowVm}, +}; + +mod dump; +mod shadow; diff --git a/core/lib/multivm/src/versions/shadow/mod.rs b/core/lib/vm_interface/src/utils/shadow.rs similarity index 94% rename from core/lib/multivm/src/versions/shadow/mod.rs rename to core/lib/vm_interface/src/utils/shadow.rs index 25a05010da3b..b381892206df 100644 --- a/core/lib/multivm/src/versions/shadow/mod.rs +++ b/core/lib/vm_interface/src/utils/shadow.rs @@ -7,20 +7,14 @@ use std::{ use zksync_types::{StorageKey, StorageLog, StorageLogWithPreviousValue, Transaction}; +use super::dump::{DumpingVm, VmDump}; use crate::{ - dump::{DumpingVm, VmDump, VmTrackingContracts}, - interface::{ - storage::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}, - BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, - SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, - VmInterfaceHistoryEnabled, VmMemoryMetrics, - }, - vm_fast, + storage::{ReadStorage, StoragePtr, StorageView}, + BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, + SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, + VmInterfaceHistoryEnabled, VmMemoryMetrics, VmTrackingContracts, }; -#[cfg(test)] -mod tests; - /// Handler for VM divergences. #[derive(Clone)] pub struct DivergenceHandler(Arc); @@ -74,9 +68,9 @@ impl VmWithReporting { /// Shadowed VM that executes 2 VMs for each operation and compares their outputs. /// /// If a divergence is detected, the VM state is dumped using [a pluggable handler](Self::set_dump_handler()), -/// after which the VM either panics, or drops the shadowed VM, depending on [`Self::set_panic_on_divergence()`] setting. +/// after which the VM drops the shadowed VM (since it's assumed that its state can contain arbitrary garbage at this point). #[derive(Debug)] -pub struct ShadowVm>> { +pub struct ShadowVm { main: DumpingVm, shadow: RefCell>>, } @@ -87,6 +81,7 @@ where Main: VmTrackingContracts, Shadow: VmInterface, { + /// Sets the divergence handler to be used by this VM. pub fn set_divergence_handler(&mut self, handler: DivergenceHandler) { if let Some(shadow) = self.shadow.get_mut() { shadow.divergence_handler = handler; @@ -105,6 +100,11 @@ where .unwrap() .report(err, self.main.dump_state()); } + + /// Dumps the current VM state. + pub fn dump_state(&self) -> VmDump { + self.main.dump_state() + } } impl ShadowVm @@ -113,6 +113,7 @@ where Main: VmFactory> + VmTrackingContracts, Shadow: VmInterface, { + /// Creates a VM with a custom shadow storage. pub fn with_custom_shadow( batch_env: L1BatchEnv, system_env: SystemEnv, @@ -135,10 +136,11 @@ where } } -impl VmFactory> for ShadowVm +impl VmFactory> for ShadowVm where S: ReadStorage, Main: VmFactory> + VmTrackingContracts, + Shadow: VmFactory>, { fn new( batch_env: L1BatchEnv, diff --git a/core/lib/vm_interface/src/vm.rs b/core/lib/vm_interface/src/vm.rs index b6be2c7581f7..09479beeadee 100644 --- a/core/lib/vm_interface/src/vm.rs +++ b/core/lib/vm_interface/src/vm.rs @@ -11,7 +11,7 @@ //! Generally speaking, in most cases, the tracer dispatcher is a wrapper around `Vec>`, //! where `VmTracer` is a trait implemented for a specific VM version. -use zksync_types::Transaction; +use zksync_types::{Transaction, H256}; use crate::{ storage::StoragePtr, BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, @@ -103,3 +103,9 @@ pub trait VmInterfaceHistoryEnabled: VmInterface { /// (i.e., the VM must not panic in this case). fn pop_snapshot_no_rollback(&mut self); } + +/// VM that tracks decommitment of bytecodes during execution. This is required to create a [`VmDump`]. +pub trait VmTrackingContracts: VmInterface { + /// Returns hashes of all decommitted bytecodes. + fn used_contract_hashes(&self) -> Vec; +} diff --git a/core/node/vm_runner/src/impls/playground.rs b/core/node/vm_runner/src/impls/playground.rs index a06291b37cfa..3aa40258280a 100644 --- a/core/node/vm_runner/src/impls/playground.rs +++ b/core/node/vm_runner/src/impls/playground.rs @@ -18,8 +18,11 @@ use zksync_health_check::{Health, HealthStatus, HealthUpdater, ReactiveHealthChe use zksync_object_store::{Bucket, ObjectStore}; use zksync_state::RocksdbStorage; use zksync_types::{vm::FastVmMode, L1BatchNumber, L2ChainId}; -use zksync_vm_executor::batch::{dump::VmDump, shadow, MainBatchExecutorFactory}; -use zksync_vm_interface::{L1BatchEnv, L2BlockEnv, SystemEnv}; +use zksync_vm_executor::batch::MainBatchExecutorFactory; +use zksync_vm_interface::{ + utils::{DivergenceHandler, VmDump}, + L1BatchEnv, L2BlockEnv, SystemEnv, +}; use crate::{ storage::{PostgresLoader, StorageLoader}, @@ -136,7 +139,7 @@ impl VmPlayground { if let Some(store) = dumps_object_store { tracing::info!("Using object store for VM dumps: {store:?}"); - let handler = shadow::DivergenceHandler::new(move |err, dump| { + let handler = DivergenceHandler::new(move |err, dump| { let err_message = err.to_string(); if let Err(err) = handle.block_on(Self::dump_vm_state(&*store, &err_message, &dump)) { diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 2218739b5558..298155954af2 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -7926,8 +7926,6 @@ dependencies = [ "hex", "itertools 0.10.5", "once_cell", - "pretty_assertions", - "serde", "thiserror", "tracing", "vise", @@ -8360,6 +8358,7 @@ dependencies = [ "anyhow", "async-trait", "hex", + "pretty_assertions", "serde", "thiserror", "tracing", From 2de00503a6ebdea43646a5bccd9b812c25449eac Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 4 Sep 2024 08:53:44 +0300 Subject: [PATCH 22/24] Fix formatting --- infrastructure/zk/src/prover_setup.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/infrastructure/zk/src/prover_setup.ts b/infrastructure/zk/src/prover_setup.ts index b5bd4c828aec..0ef3515cc750 100644 --- a/infrastructure/zk/src/prover_setup.ts +++ b/infrastructure/zk/src/prover_setup.ts @@ -30,7 +30,8 @@ export async function setupProver(proverType: ProverType) { } else { env.modify( 'FRI_PROVER_SETUP_DATA_PATH', - `${process.env.ZKSYNC_HOME}/etc/hyperchains/prover-keys/${process.env.ZKSYNC_ENV}/${proverType === ProverType.GPU ? 'gpu' : 'cpu' + `${process.env.ZKSYNC_HOME}/etc/hyperchains/prover-keys/${process.env.ZKSYNC_ENV}/${ + proverType === ProverType.GPU ? 'gpu' : 'cpu' }/`, process.env.ENV_FILE! ); @@ -97,7 +98,8 @@ async function setupProverKeys(proverType: ProverType) { env.modify( 'FRI_PROVER_SETUP_DATA_PATH', - `${process.env.ZKSYNC_HOME}/etc/hyperchains/prover-keys/${process.env.ZKSYNC_ENV}/${proverType === ProverType.GPU ? 'gpu' : 'cpu' + `${process.env.ZKSYNC_HOME}/etc/hyperchains/prover-keys/${process.env.ZKSYNC_ENV}/${ + proverType === ProverType.GPU ? 'gpu' : 'cpu' }/`, process.env.ENV_FILE! ); From acb11a3fddfa5acda2c2ab4f497b61d38039de0b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 20 Sep 2024 11:26:41 +0300 Subject: [PATCH 23/24] Remove obsolete module --- core/lib/multivm/src/dump.rs | 1 - core/lib/multivm/src/lib.rs | 1 - 2 files changed, 2 deletions(-) delete mode 100644 core/lib/multivm/src/dump.rs diff --git a/core/lib/multivm/src/dump.rs b/core/lib/multivm/src/dump.rs deleted file mode 100644 index 8b137891791f..000000000000 --- a/core/lib/multivm/src/dump.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/core/lib/multivm/src/lib.rs b/core/lib/multivm/src/lib.rs index 641102bd0743..be740d6b3780 100644 --- a/core/lib/multivm/src/lib.rs +++ b/core/lib/multivm/src/lib.rs @@ -19,7 +19,6 @@ pub use crate::{ vm_instance::VmInstance, }; -pub mod dump; mod glue; pub mod tracers; pub mod utils; From 097b8855debda78c4f08a0fdc83bd39acfa164f2 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 20 Sep 2024 11:37:36 +0300 Subject: [PATCH 24/24] Fix formatting --- docs/guides/external-node/00_quick_start.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/guides/external-node/00_quick_start.md b/docs/guides/external-node/00_quick_start.md index 776e8a56e497..5eb601e3d590 100644 --- a/docs/guides/external-node/00_quick_start.md +++ b/docs/guides/external-node/00_quick_start.md @@ -65,8 +65,8 @@ The HTTP JSON-RPC API can be accessed on port `3060` and WebSocket API can be ac > [!NOTE] > -> To stop historical DB growth, you can enable DB pruning by uncommenting `EN_PRUNING_ENABLED: true` in docker compose file, -> you can read more about pruning in +> To stop historical DB growth, you can enable DB pruning by uncommenting `EN_PRUNING_ENABLED: true` in docker compose +> file, you can read more about pruning in > [08_pruning.md](https://github.com/matter-labs/zksync-era/blob/main/docs/guides/external-node/08_pruning.md) - 32 GB of RAM and a relatively modern CPU